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.  This reverts 159685 which reverted 146616.

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

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

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

11 files changed, 166 insertions, 8 deletions

diff --git a/build/android/gtest_filter/net_unittests_disabled b/build/android/gtest_filter/net_unittests_disabled
index 3cad90b..1dcf114 100644
--- a/build/android/gtest_filter/net_unittests_disabled
+++ b/build/android/gtest_filter/net_unittests_disabled
@@ -30,6 +30,7 @@ HTTPSRequestTest.HTTPSMismatchedTest
 HTTPSRequestTest.HTTPSExpiredTest
 HTTPSRequestTest.HTTPSPreloadedHSTSTest
 HTTPSRequestTest.ClientAuthTest
+URLRequestTest.AcceptClockSkewCookieWithWrongDateTimezone
 URLRequestTest.DelayedCookieCallback
 URLRequestTest.DoNotSendCookies
 URLRequestTest.DoNotSaveCookies
diff --git a/chrome/browser/browsing_data/browsing_data_cookie_helper.cc b/chrome/browser/browsing_data/browsing_data_cookie_helper.cc
index 016464e..d5fd0b1 100644
--- a/chrome/browser/browsing_data/browsing_data_cookie_helper.cc
+++ b/chrome/browser/browsing_data/browsing_data_cookie_helper.cc
@@ -139,6 +139,11 @@ void CannedBrowsingDataCookieHelper::AddChangedCookie(
     const std::string& cookie_line,
     const net::CookieOptions& options) {
   base::Time creation_time = base::Time::Now();
+  base::Time server_time;
+  if (options.has_server_time())
+    server_time = options.server_time();
+  else
+    server_time = creation_time;
 
   net::ParsedCookie pc(cookie_line);
   if (!pc.IsValid())
@@ -161,7 +166,7 @@ void CannedBrowsingDataCookieHelper::AddChangedCookie(
       pc.MACAlgorithm() : std::string();
 
   base::Time cookie_expires =
-      net::CanonicalCookie::CanonExpiration(pc, creation_time);
+      net::CanonicalCookie::CanonExpiration(pc, creation_time, server_time);
 
   scoped_ptr<net::CanonicalCookie> cookie(
       new net::CanonicalCookie(url, pc.Name(), pc.Value(), cookie_domain,
diff --git a/chrome_frame/test/net/fake_external_tab.cc b/chrome_frame/test/net/fake_external_tab.cc
index 00e86e7..2fd8530 100644
--- a/chrome_frame/test/net/fake_external_tab.cc
+++ b/chrome_frame/test/net/fake_external_tab.cc
@@ -279,6 +279,7 @@ void FilterDisabledTests() {
 
     // Not supported in ChromeFrame as we use IE's network stack.
     "URLRequestTest.NetworkDelegateProxyError",
+    "URLRequestTest.AcceptClockSkewCookieWithWrongDateTimezone",
 
     // URLRequestAutomationJob needs to support NeedsAuth.
     // http://crbug.com/98446
diff --git a/net/cookies/canonical_cookie.cc b/net/cookies/canonical_cookie.cc
index 5ae65f6..306b8fc 100644
--- a/net/cookies/canonical_cookie.cc
+++ b/net/cookies/canonical_cookie.cc
@@ -139,7 +139,7 @@ CanonicalCookie::CanonicalCookie(const GURL& url, const ParsedCookie& pc)
       secure_(pc.IsSecure()),
       httponly_(pc.IsHttpOnly()) {
   if (pc.HasExpires())
-    expiry_date_ = CanonExpiration(pc, creation_date_);
+    expiry_date_ = CanonExpiration(pc, creation_date_, creation_date_);
 
   // Do the best we can with the domain.
   std::string cookie_domain;
@@ -181,7 +181,8 @@ std::string CanonicalCookie::CanonPath(const GURL& url,
 
 // static
 Time CanonicalCookie::CanonExpiration(const ParsedCookie& pc,
-                                      const Time& current) {
+                                      const Time& current,
+                                      const Time& server_time) {
   // First, try the Max-Age attribute.
   uint64 max_age = 0;
   if (pc.HasMaxAge() &&
@@ -195,8 +196,10 @@ Time CanonicalCookie::CanonExpiration(const ParsedCookie& pc,
   }
 
   // Try the Expires attribute.
-  if (pc.HasExpires())
-    return cookie_util::ParseCookieTime(pc.Expires());
+  if (pc.HasExpires()) {
+    // Adjust for clock skew between server and host.
+    return current + (cookie_util::ParseCookieTime(pc.Expires()) - server_time);
+  }
 
   // Invalid or no expiration, persistent cookie.
   return Time();
diff --git a/net/cookies/canonical_cookie.h b/net/cookies/canonical_cookie.h
index 1b1ca77..ba50dfc 100644
--- a/net/cookies/canonical_cookie.h
+++ b/net/cookies/canonical_cookie.h
@@ -117,7 +117,8 @@ class NET_EXPORT CanonicalCookie {
   static std::string GetCookieSourceFromURL(const GURL& url);
   static std::string CanonPath(const GURL& url, const ParsedCookie& pc);
   static base::Time CanonExpiration(const ParsedCookie& pc,
-                                    const base::Time& current);
+                                    const base::Time& current,
+                                    const base::Time& server_time);
 
  private:
   // The source member of a canonical cookie is the origin of the URL that tried
diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
index d1f3426..eab2e0e 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -1703,6 +1703,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);
@@ -1728,7 +1733,8 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions(
       pc.MACAlgorithm() : std::string();
 
   scoped_ptr<CanonicalCookie> cc;
-  Time cookie_expires = CanonicalCookie::CanonExpiration(pc, creation_time);
+  Time cookie_expires =
+      CanonicalCookie::CanonExpiration(pc, creation_time, server_time);
 
   cc.reset(new CanonicalCookie(url, pc.Name(), pc.Value(), cookie_domain,
                                cookie_path, mac_key, mac_algorithm,
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 575b0cd..51c389d 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) +
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 52972b8..c65c33a 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -566,6 +566,9 @@ void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) {
 
   FetchResponseCookies(&response_cookies_);
 
+  if (!GetResponseHeaders()->GetDateValue(&response_date_))
+    response_date_ = base::Time();
+
   // Now, loop over the response cookies, and attempt to persist each.
   SaveNextCookie();
 }
@@ -592,6 +595,7 @@ void URLRequestHttpJob::SaveNextCookie() {
       response_cookies_.size() > 0) {
     CookieOptions options;
     options.set_include_httponly();
+    options.set_server_time(response_date_);
 
     net::CookieStore::SetCookiesCallback callback(
         base::Bind(&URLRequestHttpJob::OnCookieSaved,
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index b263046..3457166 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -98,6 +98,7 @@ class URLRequestHttpJob : public URLRequestJob {
 
   std::vector<std::string> response_cookies_;
   size_t response_cookies_save_index_;
+  base::Time response_date_;
 
   // Auth states for proxy and origin server.
   AuthState proxy_auth_state_;
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 38b574a..87e065f 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1742,6 +1742,104 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) {
   }
 }
 
+// FixedDateNetworkDelegate swaps out the server's HTTP Date response header
+// value for the |fixed_date| argument given to the constructor.
+class FixedDateNetworkDelegate : public TestNetworkDelegate {
+ public:
+  explicit FixedDateNetworkDelegate(const std::string& fixed_date)
+      : fixed_date_(fixed_date) {}
+  virtual ~FixedDateNetworkDelegate() {}
+
+  // net::NetworkDelegate implementation
+  virtual int OnHeadersReceived(
+      net::URLRequest* request,
+      const net::CompletionCallback& callback,
+      const net::HttpResponseHeaders* original_response_headers,
+      scoped_refptr<net::HttpResponseHeaders>* override_response_headers)
+      OVERRIDE;
+
+ private:
+  std::string fixed_date_;
+
+  DISALLOW_COPY_AND_ASSIGN(FixedDateNetworkDelegate);
+};
+
+int FixedDateNetworkDelegate::OnHeadersReceived(
+    net::URLRequest* request,
+    const net::CompletionCallback& callback,
+    const net::HttpResponseHeaders* original_response_headers,
+    scoped_refptr<net::HttpResponseHeaders>* override_response_headers) {
+  net::HttpResponseHeaders* new_response_headers =
+      new net::HttpResponseHeaders(original_response_headers->raw_headers());
+
+  new_response_headers->RemoveHeader("Date");
+  new_response_headers->AddHeader("Date: " + fixed_date_);
+
+  *override_response_headers = new_response_headers;
+  return TestNetworkDelegate::OnHeadersReceived(request,
+                                                callback,
+                                                original_response_headers,
+                                                override_response_headers);
+}
+
+// Test that cookie expiration times are adjusted for server/client clock
+// skew and that we handle incorrect timezone specifier "UTC" in HTTP Date
+// headers by defaulting to GMT. (crbug.com/135131)
+TEST_F(URLRequestTest, AcceptClockSkewCookieWithWrongDateTimezone) {
+  LocalHttpTestServer test_server;
+  ASSERT_TRUE(test_server.Start());
+
+  // Set up an expired cookie.
+  {
+    TestNetworkDelegate network_delegate;
+    default_context_.set_network_delegate(&network_delegate);
+    TestDelegate d;
+    URLRequest req(test_server.GetURL(
+        "set-cookie?StillGood=1;expires=Mon,18-Apr-1977,22:50:13,GMT"),
+        &d,
+        &default_context_);
+    req.Start();
+    MessageLoop::current()->Run();
+  }
+  // Verify that the cookie is not set.
+  {
+    TestNetworkDelegate network_delegate;
+    default_context_.set_network_delegate(&network_delegate);
+    TestDelegate d;
+    URLRequest req(
+        test_server.GetURL("echoheader?Cookie"), &d, &default_context_);
+    req.Start();
+    MessageLoop::current()->Run();
+
+    EXPECT_TRUE(d.data_received().find("StillGood=1") == std::string::npos);
+  }
+  // Set up a cookie with clock skew and "UTC" HTTP Date timezone specifier.
+  {
+    FixedDateNetworkDelegate network_delegate("18-Apr-1977 22:49:13 UTC");
+    default_context_.set_network_delegate(&network_delegate);
+    TestDelegate d;
+    URLRequest req(test_server.GetURL(
+        "set-cookie?StillGood=1;expires=Mon,18-Apr-1977,22:50:13,GMT"),
+        &d,
+        &default_context_);
+    req.Start();
+    MessageLoop::current()->Run();
+  }
+  // Verify that the cookie is set.
+  {
+    TestNetworkDelegate network_delegate;
+    default_context_.set_network_delegate(&network_delegate);
+    TestDelegate d;
+    URLRequest req(
+        test_server.GetURL("echoheader?Cookie"), &d, &default_context_);
+    req.Start();
+    MessageLoop::current()->Run();
+
+    EXPECT_TRUE(d.data_received().find("StillGood=1") != std::string::npos);
+  }
+}
+
+
 // Check that it is impossible to change the referrer in the extra headers of
 // an URLRequest.
 TEST_F(URLRequestTest, DoNotOverrideReferrer) {