Bug 368964 - Cookie expires attribute should be absolute, not relative to server time. r=ehsan
authoramy <amchung@mozilla.com>
Fri, 19 Aug 2016 17:28:09 +0800
changeset 403454 d3df16b2621d61e06f391f5e1ac2b5d59ae1d030
parent 403453 535cfe5e5030d223f5db403922ee8fd499f6c37d
child 403455 8eda8e1c8c04e1a13c6c87f0790e2dad36debda7
push id26920
push userbmo:tchiovoloni@mozilla.com
push dateFri, 19 Aug 2016 21:00:16 +0000
reviewersehsan
bugs368964
milestone51.0a1
Bug 368964 - Cookie expires attribute should be absolute, not relative to server time. r=ehsan
netwerk/cookie/nsCookieService.cpp
netwerk/test/TestCookie.cpp
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -4108,51 +4108,52 @@ nsCookieService::GetExpiry(nsCookieAttri
 {
   /* Determine when the cookie should expire. This is done by taking the difference between
    * the server time and the time the server wants the cookie to expire, and adding that
    * difference to the client time. This localizes the client time regardless of whether or
    * not the TZ environment variable was set on the client.
    *
    * Note: We need to consider accounting for network lag here, per RFC.
    */
-  int64_t delta;
-
   // check for max-age attribute first; this overrides expires attribute
   if (!aCookieAttributes.maxage.IsEmpty()) {
     // obtain numeric value of maxageAttribute
     int64_t maxage;
     int32_t numInts = PR_sscanf(aCookieAttributes.maxage.get(), "%lld", &maxage);
 
     // default to session cookie if the conversion failed
     if (numInts != 1) {
       return true;
     }
 
-    delta = maxage;
+    // if this addition overflows, expiryTime will be less than currentTime
+    // and the cookie will be expired - that's okay.
+    aCookieAttributes.expiryTime = aCurrentTime + maxage;
 
   // check for expires attribute
   } else if (!aCookieAttributes.expires.IsEmpty()) {
     PRTime expires;
 
     // parse expiry time
     if (PR_ParseTimeString(aCookieAttributes.expires.get(), true, &expires) != PR_SUCCESS) {
       return true;
     }
 
-    delta = expires / int64_t(PR_USEC_PER_SEC) - aServerTime;
+    // If set-cookie used absolute time to set expiration, and it can't use
+    // client time to set expiration.
+    // Because if current time be set in the future, but the cookie expire
+    // time be set less than current time and more than server time.
+    // The cookie item have to be used to the expired cookie.
+    aCookieAttributes.expiryTime = expires / int64_t(PR_USEC_PER_SEC);
 
   // default to session cookie if no attributes found
   } else {
     return true;
   }
 
-  // if this addition overflows, expiryTime will be less than currentTime
-  // and the cookie will be expired - that's okay.
-  aCookieAttributes.expiryTime = aCurrentTime + delta;
-
   return false;
 }
 
 /******************************************************************************
  * nsCookieService impl:
  * private cookielist management functions
  ******************************************************************************/
 
--- a/netwerk/test/TestCookie.cpp
+++ b/netwerk/test/TestCookie.cpp
@@ -28,16 +28,46 @@ static NS_DEFINE_CID(kPrefServiceCID,   
 static const char kCookiesPermissions[] = "network.cookie.cookieBehavior";
 static const char kCookiesLifetimeEnabled[] = "network.cookie.lifetime.enabled";
 static const char kCookiesLifetimeDays[] = "network.cookie.lifetime.days";
 static const char kCookiesLifetimeCurrentSession[] = "network.cookie.lifetime.behavior";
 static const char kCookiesMaxPerHost[] = "network.cookie.maxPerHost";
 
 static char *sBuffer;
 
+#define OFFSET_ONE_WEEK int64_t(604800) * PR_USEC_PER_SEC
+#define OFFSET_ONE_DAY int64_t(86400) * PR_USEC_PER_SEC
+
+//Set server time or expiry time
+void
+SetTime(PRTime offsetTime,nsAutoCString& serverString,nsAutoCString& cookieString,bool expiry)
+{
+    char timeStringPreset[40];
+    PRTime CurrentTime = PR_Now();
+    PRTime SetCookieTime = CurrentTime + offsetTime;
+    PRTime SetExpiryTime;
+    if (expiry) {
+      SetExpiryTime = SetCookieTime - OFFSET_ONE_DAY;
+    } else {
+      SetExpiryTime = SetCookieTime + OFFSET_ONE_DAY;
+    }
+
+    // Set server time string
+    PRExplodedTime explodedTime;
+    PR_ExplodeTime(SetCookieTime , PR_GMTParameters, &explodedTime);
+    PR_FormatTimeUSEnglish(timeStringPreset, 40, "%c GMT", &explodedTime);
+    serverString.Assign(timeStringPreset);
+
+    // Set cookie string
+    PR_ExplodeTime(SetExpiryTime , PR_GMTParameters, &explodedTime);
+    PR_FormatTimeUSEnglish(timeStringPreset, 40, "%c GMT", &explodedTime);
+    cookieString.Replace(0, strlen("test=expiry; expires=") + strlen(timeStringPreset) + 1, "test=expiry; expires=");
+    cookieString.Append(timeStringPreset);
+}
+
 nsresult
 SetACookie(nsICookieService *aCookieService, const char *aSpec1, const char *aSpec2, const char* aCookieString, const char *aServerTime)
 {
     nsCOMPtr<nsIURI> uri1, uri2;
     NS_NewURI(getter_AddRefs(uri1), aSpec1);
     if (aSpec2)
         NS_NewURI(getter_AddRefs(uri2), aSpec2);
 
@@ -458,18 +488,39 @@ main(int32_t argc, char *argv[])
 
       SetACookie(cookieService, "http://foo.expireme.org/", nullptr, "test=expiry; domain=.expireme.org; max-age=60", nullptr);
       GetACookie(cookieService, "http://expireme.org/", nullptr, getter_Copies(cookie));
       rv[14] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry");
       SetACookie(cookieService, "http://bar.expireme.org/", nullptr, "test=differentvalue; domain=.expireme.org; max-age=0", nullptr);
       GetACookie(cookieService, "http://expireme.org/", nullptr, getter_Copies(cookie));
       rv[15] = CheckResult(cookie.get(), MUST_BE_NULL);
 
-      allTestsPassed = PrintResult(rv, 16) && allTestsPassed;
+      nsAutoCString ServerTime;
+      nsAutoCString CookieString;
 
+      SetTime(-OFFSET_ONE_WEEK, ServerTime, CookieString, true);
+      SetACookie(cookieService, "http://expireme.org/", nullptr, CookieString.get(), ServerTime.get());
+      GetACookie(cookieService, "http://expireme.org/", nullptr, getter_Copies(cookie));
+      rv[16] = CheckResult(cookie.get(), MUST_BE_NULL);
+      // Set server time earlier than client time for one year + one day, and expirty time earlier than server time for one day.
+      SetTime(-(OFFSET_ONE_DAY + OFFSET_ONE_WEEK), ServerTime, CookieString, false);
+      SetACookie(cookieService, "http://expireme.org/", nullptr, CookieString.get(), ServerTime.get());
+      GetACookie(cookieService, "http://expireme.org/", nullptr, getter_Copies(cookie));
+      rv[17] = CheckResult(cookie.get(), MUST_BE_NULL);
+      // Set server time later than client time for one year, and expiry time later than server time for one day.
+      SetTime(OFFSET_ONE_WEEK, ServerTime, CookieString, false);
+      SetACookie(cookieService, "http://expireme.org/", nullptr, CookieString.get(), ServerTime.get());
+      GetACookie(cookieService, "http://expireme.org/", nullptr, getter_Copies(cookie));
+      rv[18] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry");
+      // Set server time later than client time for one year + one day, and expiry time earlier than server time for one day.
+      SetTime((OFFSET_ONE_DAY + OFFSET_ONE_WEEK), ServerTime, CookieString, true);
+      SetACookie(cookieService, "http://expireme.org/", nullptr, CookieString.get(), ServerTime.get());
+      GetACookie(cookieService, "http://expireme.org/", nullptr, getter_Copies(cookie));
+      rv[19] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry");
+      allTestsPassed = PrintResult(rv, 20) && allTestsPassed;
 
       // *** multiple cookie tests
       sBuffer = PR_sprintf_append(sBuffer, "*** Beginning multiple cookie tests...\n");
 
       // test the setting of multiple cookies, and test the order of precedence
       // (a later cookie overwriting an earlier one, in the same header string)
       SetACookie(cookieService, "http://multiple.cookies/", nullptr, "test=multiple; domain=.multiple.cookies \n test=different \n test=same; domain=.multiple.cookies \n newtest=ciao \n newtest=foo; max-age=-6 \n newtest=reincarnated", nullptr);
       GetACookie(cookieService, "http://multiple.cookies/", nullptr, getter_Copies(cookie));