Bug 1430803 - Ignore SameSite cookie attribute when value is empty or unrecognised. r=valentin, a=RyanVM
authorFrancois Marier <francois@mozilla.com>
Thu, 05 Apr 2018 17:09:13 -0700
changeset 463144 57318d4d7ca9f00ad4fa688516b43b4a39cadcf0
parent 463143 5dc2bed05d2ac1525dc9a4a146cbff0b84611cce
child 463145 18f05dfc1f61a82feaa96b5dd9d896d9ce475d5a
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin, RyanVM
bugs1430803
milestone60.0
Bug 1430803 - Ignore SameSite cookie attribute when value is empty or unrecognised. r=valentin, a=RyanVM Make the parsing the the attribute spec-compliant: https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-02#section-5.3.7 MozReview-Commit-ID: 8YpkZvgryJb
netwerk/cookie/nsCookieService.cpp
netwerk/test/TestCookie.cpp
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -3883,16 +3883,17 @@ nsCookieService::ParseAttributes(nsDepen
   static const char kPath[]    = "path";
   static const char kDomain[]  = "domain";
   static const char kExpires[] = "expires";
   static const char kMaxage[]  = "max-age";
   static const char kSecure[]  = "secure";
   static const char kHttpOnly[]  = "httponly";
   static const char kSameSite[]       = "samesite";
   static const char kSameSiteLax[]    = "lax";
+  static const char kSameSiteStrict[]    = "strict";
 
   nsACString::const_char_iterator tempBegin, tempEnd;
   nsACString::const_char_iterator cookieStart, cookieEnd;
   aCookieHeader.BeginReading(cookieStart);
   aCookieHeader.EndReading(cookieEnd);
 
   aCookieAttributes.isSecure = false;
   aCookieAttributes.isHttpOnly = false;
@@ -3944,17 +3945,17 @@ nsCookieService::ParseAttributes(nsDepen
     // ignore any tokenValue for isHttpOnly (see bug 178993);
     // just set the boolean
     else if (tokenString.LowerCaseEqualsLiteral(kHttpOnly))
       aCookieAttributes.isHttpOnly = true;
 
     else if (tokenString.LowerCaseEqualsLiteral(kSameSite)) {
       if (tokenValue.LowerCaseEqualsLiteral(kSameSiteLax)) {
         aCookieAttributes.sameSite = nsICookie2::SAMESITE_LAX;
-      } else {
+      } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteStrict)) {
         aCookieAttributes.sameSite = nsICookie2::SAMESITE_STRICT;
       }
     }
   }
 
   // rebind aCookieHeader, in case we need to process another cookie
   aCookieHeader.Rebind(cookieStart, cookieEnd);
   return newCookie;
--- a/netwerk/test/TestCookie.cpp
+++ b/netwerk/test/TestCookie.cpp
@@ -771,16 +771,20 @@ TEST(TestCookie,TestCookieMain)
     // Clear the cookies
     EXPECT_TRUE(NS_SUCCEEDED(cookieMgr->RemoveAll()));
 
     // Set cookies with various incantations of the samesite attribute:
     // No same site attribute present
     SetACookie(cookieService, "http://samesite.test", nullptr, "unset=yes", nullptr);
     // samesite attribute present but with no value
     SetACookie(cookieService, "http://samesite.test", nullptr, "unspecified=yes; samesite", nullptr);
+    // samesite attribute present but with an empty value
+    SetACookie(cookieService, "http://samesite.test", nullptr, "empty=yes; samesite=", nullptr);
+    // samesite attribute present but with an invalid value
+    SetACookie(cookieService, "http://samesite.test", nullptr, "bogus=yes; samesite=bogus", nullptr);
     // samesite=strict
     SetACookie(cookieService, "http://samesite.test", nullptr, "strict=yes; samesite=strict", nullptr);
     // samesite=lax
     SetACookie(cookieService, "http://samesite.test", nullptr, "lax=yes; samesite=lax", nullptr);
 
     EXPECT_TRUE(NS_SUCCEEDED(cookieMgr->GetEnumerator(getter_AddRefs(enumerator))));
     i = 0;
 
@@ -795,23 +799,27 @@ TEST(TestCookie,TestCookieMain)
       if (!cookie2) break;
       nsAutoCString name;
       cookie2->GetName(name);
       int32_t sameSiteAttr;
       cookie2->GetSameSite(&sameSiteAttr);
       if (name.EqualsLiteral("unset")) {
         EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_UNSET);
       } else if (name.EqualsLiteral("unspecified")) {
-        EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_STRICT);
+        EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_UNSET);
+      } else if (name.EqualsLiteral("empty")) {
+        EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_UNSET);
+      } else if (name.EqualsLiteral("bogus")) {
+        EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_UNSET);
       } else if (name.EqualsLiteral("strict")) {
         EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_STRICT);
       } else if (name.EqualsLiteral("lax")) {
         EXPECT_TRUE(sameSiteAttr == nsICookie2::SAMESITE_LAX);
       }
     }
 
-    EXPECT_TRUE(i == 4);
+    EXPECT_TRUE(i == 6);
 
     // XXX the following are placeholders: add these tests please!
     // *** "noncompliant cookie" tests
     // *** IP address tests
     // *** speed tests
 }