Account for server vs host clock skew in cookie expiration times.

When setting a cookie's expiration time in the cookie store we need
to take into account any difference between the HTTP server and the
host machine's real time clock.

BUG=135131
TEST=net_unittests --gtest_filter=CookieMonster/CookieStoreTest/0.TestCookieDeletion



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

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

3 files changed, 54 insertions, 6 deletions

diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
index e44ba18..9851c9a 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -236,7 +236,9 @@ std::string CanonPath(const GURL& url, const ParsedCookie& pc) {
   return CanonPathWithString(url, path_string);
 }
 
-Time CanonExpiration(const ParsedCookie& pc, const Time& current) {
+Time CanonExpiration(const ParsedCookie& pc,
+                     const Time& current,
+                     const Time& server_time) {
   // First, try the Max-Age attribute.
   uint64 max_age = 0;
   if (pc.HasMaxAge() &&
@@ -250,8 +252,11 @@ Time CanonExpiration(const ParsedCookie& pc, const Time& current) {
   }
 
   // Try the Expires attribute.
-  if (pc.HasExpires())
-    return CookieMonster::ParseCookieTime(pc.Expires());
+  if (pc.HasExpires()) {
+    // Adjust for clock skew between server and host.
+    return current + (CookieMonster::ParseCookieTime(pc.Expires()) -
+                      server_time);
+  }
 
   // Invalid or no expiration, persistent cookie.
   return Time();
@@ -1888,6 +1893,11 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions(
     creation_time = CurrentTime();
     last_time_seen_ = creation_time;
   }
+  Time server_time;
+  if (options.has_server_time())
+    server_time = options.server_time();
+  else
+    server_time = creation_time;
 
   // Parse the cookie.
   ParsedCookie pc(cookie_line);
@@ -1913,7 +1923,7 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions(
       pc.MACAlgorithm() : std::string();
 
   scoped_ptr<CanonicalCookie> cc;
-  Time cookie_expires = CanonExpiration(pc, creation_time);
+  Time cookie_expires = CanonExpiration(pc, creation_time, server_time);
 
   cc.reset(new CanonicalCookie(url, pc.Name(), pc.Value(), cookie_domain,
                                cookie_path, mac_key, mac_algorithm,
@@ -2375,7 +2385,7 @@ CookieMonster::CanonicalCookie::CanonicalCookie(const GURL& url,
       secure_(pc.IsSecure()),
       httponly_(pc.IsHttpOnly()) {
   if (pc.HasExpires())
-    expiry_date_ = CanonExpiration(pc, creation_date_);
+    expiry_date_ = CanonExpiration(pc, creation_date_, creation_date_);
   else
     SetSessionCookieExpiryTime();
 
diff --git a/net/cookies/cookie_options.h b/net/cookies/cookie_options.h
index 8370702..ed5e2ef 100644
--- a/net/cookies/cookie_options.h
+++ b/net/cookies/cookie_options.h
@@ -15,15 +15,26 @@ class CookieOptions {
   // - reading operations will not return httponly cookies.
   // - writing operations will not write httponly cookies.
   CookieOptions()
-      : exclude_httponly_(true) {
+      : exclude_httponly_(true),
+        server_time_() {
   }
 
   void set_exclude_httponly() { exclude_httponly_ = true; }
   void set_include_httponly() { exclude_httponly_ = false; }
   bool exclude_httponly() const { return exclude_httponly_; }
 
+  // |server_time| indicates what the server sending us the Cookie thought the
+  // current time was when the cookie was produced.  This is used to adjust for
+  // clock skew between server and host.
+  void set_server_time(const base::Time& server_time) {
+    server_time_ = server_time;
+  }
+  bool has_server_time() const { return !server_time_.is_null(); }
+  base::Time server_time() const { return server_time_; }
+
  private:
   bool exclude_httponly_;
+  base::Time server_time_;
 };
 }  // namespace net
 
diff --git a/net/cookies/cookie_store_unittest.h b/net/cookies/cookie_store_unittest.h
index be34c4a..592d52a 100644
--- a/net/cookies/cookie_store_unittest.h
+++ b/net/cookies/cookie_store_unittest.h
@@ -147,6 +147,17 @@ class CookieStoreTest : public testing::Test {
     return callback.result();
   }
 
+  bool SetCookieWithServerTime(CookieStore* cs,
+                               const GURL& url,
+                               const std::string& cookie_line,
+                               const base::Time& server_time) {
+    CookieOptions options;
+    if (!CookieStoreTestTraits::supports_http_only)
+      options.set_include_httponly();
+    options.set_server_time(server_time);
+    return SetCookieWithOptions(cs, url, cookie_line, options);
+  }
+
   bool SetCookie(CookieStore* cs,
                  const GURL& url,
                  const std::string& cookie_line) {
@@ -702,6 +713,22 @@ TYPED_TEST_P(CookieStoreTest, TestCookieDeletion) {
                               std::string(kValidCookieLine) +
                               "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
   this->MatchCookieLines("A=B", this->GetCookies(cs, this->url_google_));
+  // Check that it is not deleted with significant enough clock skew.
+  base::Time server_time;
+  EXPECT_TRUE(base::Time::FromString("Sun, 17-Apr-1977 22:50:13 GMT",
+                                     &server_time));
+  EXPECT_TRUE(this->SetCookieWithServerTime(
+      cs, this->url_google_,
+      std::string(kValidCookieLine) +
+      "; expires=Mon, 18-Apr-1977 22:50:13 GMT",
+      server_time));
+  this->MatchCookieLines("A=B", this->GetCookies(cs, this->url_google_));
+
+  // Create a persistent cookie.
+  EXPECT_TRUE(this->SetCookie(cs, this->url_google_,
+                              std::string(kValidCookieLine) +
+                              "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
+  this->MatchCookieLines("A=B", this->GetCookies(cs, this->url_google_));
   // Delete it via Expires, with a unix epoch of 0.
   EXPECT_TRUE(this->SetCookie(cs, this->url_google_,
                               std::string(kValidCookieLine) +