Ignore an empty expiry attribute, as per RFC.

Original issue with LGTM is at https://codereview.chromium.org/12635007/ but was based on a branch. Creating this issue to commit to Trunk first.

TBR=rdsmith@chromium.org
BUG=178201


Review URL: https://chromiumcodereview.appspot.com/12648006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187930 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 63 insertions, 2 deletions

diff --git a/net/cookies/canonical_cookie.cc b/net/cookies/canonical_cookie.cc
index a24f2f2..228e3f3 100644
--- a/net/cookies/canonical_cookie.cc
+++ b/net/cookies/canonical_cookie.cc
@@ -193,9 +193,11 @@ Time CanonicalCookie::CanonExpiration(const ParsedCookie& pc,
   }
 
   // Try the Expires attribute.
-  if (pc.HasExpires()) {
+  if (pc.HasExpires() && !pc.Expires().empty()) {
     // Adjust for clock skew between server and host.
-    return current + (cookie_util::ParseCookieTime(pc.Expires()) - server_time);
+    base::Time parsed_expiry = cookie_util::ParseCookieTime(pc.Expires());
+    if (!parsed_expiry.is_null())
+      return parsed_expiry + (current - server_time);
   }
 
   // Invalid or no expiration, persistent cookie.
diff --git a/net/cookies/canonical_cookie_unittest.cc b/net/cookies/canonical_cookie_unittest.cc
index 2a2b353..d7c25d188 100644
--- a/net/cookies/canonical_cookie_unittest.cc
+++ b/net/cookies/canonical_cookie_unittest.cc
@@ -131,6 +131,39 @@ TEST(CanonicalCookieTest, Create) {
   EXPECT_FALSE(cookie->IsSecure());
 }
 
+TEST(CanonicalCookieTest, EmptyExpiry) {
+  GURL url("http://www7.ipdl.inpit.go.jp/Tokujitu/tjkta.ipdl?N0000=108");
+  base::Time creation_time = base::Time::Now();
+  CookieOptions options;
+
+  std::string cookie_line =
+      "ACSTM=20130308043820420042; path=/; domain=ipdl.inpit.go.jp; Expires=";
+  scoped_ptr<CanonicalCookie> cookie(CanonicalCookie::Create(
+      url, cookie_line, creation_time, options));
+  EXPECT_TRUE(cookie.get());
+  EXPECT_FALSE(cookie->IsPersistent());
+  EXPECT_FALSE(cookie->IsExpired(creation_time));
+  EXPECT_EQ(base::Time(), cookie->ExpiryDate());
+
+  // With a stale server time
+  options.set_server_time(creation_time - base::TimeDelta::FromHours(1));
+  cookie.reset(CanonicalCookie::Create(
+      url, cookie_line, creation_time, options));
+  EXPECT_TRUE(cookie.get());
+  EXPECT_FALSE(cookie->IsPersistent());
+  EXPECT_FALSE(cookie->IsExpired(creation_time));
+  EXPECT_EQ(base::Time(), cookie->ExpiryDate());
+
+  // With a future server time
+  options.set_server_time(creation_time + base::TimeDelta::FromHours(1));
+  cookie.reset(CanonicalCookie::Create(
+      url, cookie_line, creation_time, options));
+  EXPECT_TRUE(cookie.get());
+  EXPECT_FALSE(cookie->IsPersistent());
+  EXPECT_FALSE(cookie->IsExpired(creation_time));
+  EXPECT_EQ(base::Time(), cookie->ExpiryDate());
+}
+
 TEST(CanonicalCookieTest, IsEquivalent) {
   GURL url("http://www.example.com/");
   std::string cookie_name = "A";
diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
index 83e99cb..61e7f26 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -1635,6 +1635,8 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
     }
 
     InternalInsertCookie(key, cc->release(), true);
+  } else {
+    VLOG(kVlogSetCookies) << "SetCookie() not storing already expired cookie.";
   }
 
   // We assume that hopefully setting a cookie will be less common than
diff --git a/net/cookies/cookie_store_unittest.h b/net/cookies/cookie_store_unittest.h
index eed8b00..d445a3f 100644
--- a/net/cookies/cookie_store_unittest.h
+++ b/net/cookies/cookie_store_unittest.h
@@ -574,6 +574,29 @@ TYPED_TEST_P(CookieStoreTest, PathTest) {
   this->MatchCookieLines("A=C", this->GetCookies(cs, GURL(url + "/")));
 }
 
+TYPED_TEST_P(CookieStoreTest, EmptyExpires) {
+  scoped_refptr<CookieStore> cs(this->GetCookieStore());
+  CookieOptions options;
+  GURL url("http://www7.ipdl.inpit.go.jp/Tokujitu/tjkta.ipdl?N0000=108");
+  std::string set_cookie_line =
+      "ACSTM=20130308043820420042; path=/; domain=ipdl.inpit.go.jp; Expires=";
+  std::string cookie_line = "ACSTM=20130308043820420042";
+
+  this->SetCookieWithOptions( cs, url, set_cookie_line, options);
+  this->MatchCookieLines(cookie_line,
+                         this->GetCookiesWithOptions(cs, url, options));
+
+  options.set_server_time(base::Time::Now() - base::TimeDelta::FromHours(1));
+  this->SetCookieWithOptions( cs, url, set_cookie_line, options);
+  this->MatchCookieLines(cookie_line,
+                         this->GetCookiesWithOptions(cs, url, options));
+
+  options.set_server_time(base::Time::Now() + base::TimeDelta::FromHours(1));
+  this->SetCookieWithOptions( cs, url, set_cookie_line, options);
+  this->MatchCookieLines(cookie_line,
+                         this->GetCookiesWithOptions(cs, url, options));
+}
+
 TYPED_TEST_P(CookieStoreTest, HttpOnlyTest) {
   if (!TypeParam::supports_http_only)
     return;
@@ -862,6 +885,7 @@ REGISTER_TYPED_TEST_CASE_P(CookieStoreTest,
                            InvalidScheme,
                            InvalidScheme_Read,
                            PathTest,
+                           EmptyExpires,
                            HttpOnlyTest,
                            TestCookieDeletion,
                            TestDeleteAllCreatedBetween,