Backed out changeset 754426d70d28 (bug 1551729) for causing wpt failures. CLOSED TREE
authorMihai Alexandru Michis <malexandru@mozilla.com>
Tue, 21 May 2019 15:17:37 +0300
changeset 474713 086a7a5fa9eb35bd1e341a91aa8cb934241b5b3f
parent 474712 03d3b6d0eeca1bcf833bef4d519848220d5c78bd
child 474714 4039c6761d62239b4e55691f381cf2953969dcca
push id36044
push userrmaries@mozilla.com
push dateTue, 21 May 2019 15:45:34 +0000
treeherdermozilla-central@78571bb1f20e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1551729
milestone69.0a1
backs out754426d70d2870cc8cdb4814f547b4e476648ff9
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 754426d70d28 (bug 1551729) for causing wpt failures. CLOSED TREE
browser/components/originattributes/test/browser/browser_favicon_firstParty.js
browser/components/originattributes/test/browser/browser_favicon_userContextId.js
browser/components/privatebrowsing/test/browser/browser_privatebrowsing_favicon.js
browser/components/sessionstore/test/browser_sessionStoreContainer.js
dom/browser-element/mochitest/file_browserElement_CookiesNotThirdParty.html
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
netwerk/test/TestCookie.cpp
testing/web-platform/meta/cookies/http-state/chromium-tests.html.ini
testing/web-platform/meta/cookies/http-state/general-tests.html.ini
toolkit/components/thumbnails/test/thumbnails_background.sjs
--- a/browser/components/originattributes/test/browser/browser_favicon_firstParty.js
+++ b/browser/components/originattributes/test/browser/browser_favicon_firstParty.js
@@ -178,18 +178,18 @@ async function assignCookiesUnderFirstPa
   });
 
   BrowserTestUtils.removeTab(tabInfo.tab);
 }
 
 async function generateCookies(aThirdParty) {
   // we generate two different cookies for two first party domains.
   let cookies = [];
-  cookies.push(Math.random().toString());
-  cookies.push(Math.random().toString());
+  cookies.push(Math.random().toString() + "=1");
+  cookies.push(Math.random().toString() + "=1");
 
   let firstSiteURL;
   let secondSiteURL;
 
   if (aThirdParty) {
     // Add cookies into the third party site with different first party domain.
     firstSiteURL = THIRD_PARTY_SITE;
     secondSiteURL = THIRD_PARTY_SITE;
--- a/browser/components/originattributes/test/browser/browser_favicon_userContextId.js
+++ b/browser/components/originattributes/test/browser/browser_favicon_userContextId.js
@@ -123,18 +123,18 @@ function waitOnFaviconLoaded(aFaviconURL
     (uri, attr, value, id) => attr === Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON &&
                               value === aFaviconURL,
     "history");
 }
 
 async function generateCookies(aHost) {
   // we generate two different cookies for two userContextIds.
   let cookies = [];
-  cookies.push(Math.random().toString());
-  cookies.push(Math.random().toString());
+  cookies.push(Math.random().toString() + "=1");
+  cookies.push(Math.random().toString() + "=1");
 
   // Then, we add cookies into the site for 'personal' and 'work'.
   let tabInfoA = await openTabInUserContext(aHost, USER_CONTEXT_ID_PERSONAL);
   let tabInfoB = await openTabInUserContext(aHost, USER_CONTEXT_ID_WORK);
 
   await ContentTask.spawn(tabInfoA.browser, cookies[0], async function(value) {
     content.document.cookie = value;
   });
--- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_favicon.js
+++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_favicon.js
@@ -184,18 +184,18 @@ add_task(async function test_favicon_pri
 
   // Create a private browsing window.
   let privateWindow = await BrowserTestUtils.openNewBrowserWindow({ private: true });
   let pageURI = makeURI(TEST_PAGE);
 
   // Generate two random cookies for non-private window and private window
   // respectively.
   let cookies = [];
-  cookies.push(Math.random().toString());
-  cookies.push(Math.random().toString());
+  cookies.push(Math.random().toString() + "=1");
+  cookies.push(Math.random().toString() + "=1");
 
   // Open a tab in private window and add a cookie into it.
   await assignCookies(privateWindow.gBrowser, TEST_SITE, cookies[0]);
 
   // Open a tab in non-private window and add a cookie into it.
   await assignCookies(gBrowser, TEST_SITE, cookies[1]);
 
   // Add the observer earlier in case we don't capture events in time.
--- a/browser/components/sessionstore/test/browser_sessionStoreContainer.js
+++ b/browser/components/sessionstore/test/browser_sessionStoreContainer.js
@@ -109,17 +109,17 @@ add_task(async function test() {
     "set": [ [ "privacy.userContext.enabled", true ] ],
   });
 
   Services.cookies.removeAll();
 
   for (let userContextId of Object.keys(USER_CONTEXTS)) {
     // Load the page in 3 different contexts and set a cookie
     // which should only be visible in that context.
-    let cookie = USER_CONTEXTS[userContextId];
+    let cookie = USER_CONTEXTS[userContextId] + "=1";
 
     // Open our tab in the given user context.
     let { tab, browser } = await openTabInUserContext(userContextId);
 
     await Promise.all([
       waitForNewCookie(),
       ContentTask.spawn(browser, cookie,
         passedCookie => content.document.cookie = passedCookie),
--- a/dom/browser-element/mochitest/file_browserElement_CookiesNotThirdParty.html
+++ b/dom/browser-element/mochitest/file_browserElement_CookiesNotThirdParty.html
@@ -1,20 +1,20 @@
 <html>
 <body>
 file_browserElement_CookiesNotThirdParty.html
 
 <script type='text/javascript'>
 if (location.search != "?step=2") {
   // Step 1: Set a cookie.
-  document.cookie = "file_browserElement_CookiesNotThirdParty";
+  document.cookie = "file_browserElement_CookiesNotThirdParty=1";
   alert("next");
 } else {
   // Step 2: Read the cookie.
-  if (document.cookie == "file_browserElement_CookiesNotThirdParty") {
+  if (document.cookie == "file_browserElement_CookiesNotThirdParty=1") {
     alert("success: got the correct cookie");
   } else {
     alert('failure: got unexpected cookie: "' + document.cookie + '"');
   }
 
   alert("finish");
 }
 </script>
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -3234,17 +3234,18 @@ bool nsCookieService::CanSetCookie(nsIUR
   aCookieAttributes.expiryTime = INT64_MAX;
 
   // aCookieHeader is an in/out param to point to the next cookie, if
   // there is one. Save the present value for logging purposes
   nsDependentCString savedCookieHeader(aCookieHeader);
 
   // newCookie says whether there are multiple cookies in the header;
   // so we can handle them separately.
-  bool newCookie = ParseAttributes(aCookieHeader, aCookieAttributes);
+  bool discard = false;
+  bool newCookie = ParseAttributes(aCookieHeader, aCookieAttributes, discard);
 
   // Collect telemetry on how often secure cookies are set from non-secure
   // origins, and vice-versa.
   //
   // 0 = nonsecure and "http:"
   // 1 = nonsecure and "https:"
   // 2 = secure and "http:"
   // 3 = secure and "https:"
@@ -3380,17 +3381,17 @@ bool nsCookieService::CanSetCookie(nsIUR
       if (NS_FAILED(rv) || isThirdParty) {
         COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
                           "failed the samesite tests");
         return newCookie;
       }
     }
   }
 
-  aSetCookie = true;
+  aSetCookie = !discard;
   return newCookie;
 }
 
 // processes a single cookie, and returns true if there are more cookies
 // to be processed
 bool nsCookieService::SetCookieInternal(nsIURI* aHostURI,
                                         const mozilla::net::nsCookieKey& aKey,
                                         bool aRequireHostMatch,
@@ -3781,17 +3782,18 @@ bool nsCookieService::GetTokenValue(nsAC
   }
   return false;
 }
 
 // Parses attributes from cookie header. expires/max-age attributes aren't
 // folded into the cookie struct here, because we don't know which one to use
 // until we've parsed the header.
 bool nsCookieService::ParseAttributes(nsDependentCString& aCookieHeader,
-                                      nsCookieAttributes& aCookieAttributes) {
+                                      nsCookieAttributes& aCookieAttributes,
+                                      bool& aDiscard) {
   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";
@@ -3801,39 +3803,48 @@ bool nsCookieService::ParseAttributes(ns
   nsACString::const_char_iterator cookieStart, cookieEnd;
   aCookieHeader.BeginReading(cookieStart);
   aCookieHeader.EndReading(cookieEnd);
 
   aCookieAttributes.isSecure = false;
   aCookieAttributes.isHttpOnly = false;
   aCookieAttributes.sameSite = nsICookie2::SAMESITE_UNSET;
 
+  aDiscard = false;
+
   nsDependentCSubstring tokenString(cookieStart, cookieStart);
   nsDependentCSubstring tokenValue(cookieStart, cookieStart);
   bool newCookie, equalsFound;
 
   // extract cookie <NAME> & <VALUE> (first attribute), and copy the strings.
   // if we find multiple cookies, return for processing
   // note: if there's no '=', we assume token is <VALUE>. this is required by
   //       some sites (see bug 169091).
   // XXX fix the parser to parse according to <VALUE> grammar for this case
   newCookie = GetTokenValue(cookieStart, cookieEnd, tokenString, tokenValue,
                             equalsFound);
   if (equalsFound) {
     aCookieAttributes.name = tokenString;
     aCookieAttributes.value = tokenValue;
+    if (aCookieAttributes.name.IsEmpty()) {
+      aDiscard = true;
+    }
   } else {
-    aCookieAttributes.value = tokenString;
+    aDiscard = true;
   }
 
   // extract remaining attributes
   while (cookieStart != cookieEnd && !newCookie) {
     newCookie = GetTokenValue(cookieStart, cookieEnd, tokenString, tokenValue,
                               equalsFound);
 
+    if (aDiscard) {
+      continue;
+    }
+
     if (!tokenValue.IsEmpty()) {
       tokenValue.BeginReading(tempBegin);
       tokenValue.EndReading(tempEnd);
     }
 
     // decide which attribute we have, and copy the string
     if (tokenString.LowerCaseEqualsLiteral(kPath))
       aCookieAttributes.path = tokenValue;
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -336,17 +336,18 @@ class nsCookieService final : public nsI
   void UpdateCookieInList(nsCookie* aCookie, int64_t aLastAccessed,
                           mozIStorageBindingParamsArray* aParamsArray);
   static bool GetTokenValue(nsACString::const_char_iterator& aIter,
                             nsACString::const_char_iterator& aEndIter,
                             nsDependentCSubstring& aTokenString,
                             nsDependentCSubstring& aTokenValue,
                             bool& aEqualsFound);
   static bool ParseAttributes(nsDependentCString& aCookieHeader,
-                              nsCookieAttributes& aCookie);
+                              nsCookieAttributes& aCookieAttributes,
+                              bool& aDiscard);
   bool RequireThirdPartyCheck();
   static bool CheckDomain(nsCookieAttributes& aCookie, nsIURI* aHostURI,
                           const nsCString& aBaseDomain, bool aRequireHostMatch);
   static bool CheckPath(nsCookieAttributes& aCookie, nsIURI* aHostURI);
   static bool CheckPrefixes(nsCookieAttributes& aCookie, bool aSecureRequest);
   static bool GetExpiry(nsCookieAttributes& aCookie, int64_t aServerTime,
                         int64_t aCurrentTime, bool aFromHttp);
   void RemoveAllFromMemory();
--- a/netwerk/test/TestCookie.cpp
+++ b/netwerk/test/TestCookie.cpp
@@ -615,35 +615,35 @@ TEST(TestCookie, TestCookieMain)
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
   SetACookie(cookieService, "http://parser.test/", nullptr,
              "test=\"fubar! = foo;bar\\\";\" parser; domain=.parser.test; "
              "max-age=6\nfive; max-age=2.63,",
              nullptr);
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_CONTAIN, R"(test="fubar! = foo)"));
-  EXPECT_TRUE(CheckResult(cookie.get(), MUST_CONTAIN, "five"));
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_NOT_CONTAIN, "five"));
   SetACookie(cookieService, "http://parser.test/", nullptr,
              "test=kill; domain=.parser.test; max-age=0 \n five; max-age=0",
              nullptr);
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
 
   // test the handling of VALUE-only cookies (see bug 169091),
   // i.e. "six" should assume an empty NAME, which allows other VALUE-only
   // cookies to overwrite it
   SetACookie(cookieService, "http://parser.test/", nullptr, "six", nullptr);
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
-  EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, "six"));
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
   SetACookie(cookieService, "http://parser.test/", nullptr, "seven", nullptr);
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
-  EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, "seven"));
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
   SetACookie(cookieService, "http://parser.test/", nullptr, " =eight", nullptr);
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
-  EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, "eight"));
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
   SetACookie(cookieService, "http://parser.test/", nullptr, "test=six",
              nullptr);
   GetACookie(cookieService, "http://parser.test/", nullptr, cookie);
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_CONTAIN, "test=six"));
 
   // *** path ordering tests
 
   // test that cookies are returned in path order - longest to shortest.
--- a/testing/web-platform/meta/cookies/http-state/chromium-tests.html.ini
+++ b/testing/web-platform/meta/cookies/http-state/chromium-tests.html.ini
@@ -1,19 +1,7 @@
 [chromium-tests.html]
-  [chromium0009 - chromium0009]
-    expected: FAIL
-
   [chromium0006 - chromium0006]
     expected: FAIL
 
-  [chromium0012 - chromium0012]
-    expected: FAIL
-
-  [disabled-chromium0022 - disabled-chromium0022]
-    expected: FAIL
-
-  [chromium0010 - chromium0010]
-    expected: FAIL
-
   [chromium0007 - chromium0007]
     expected: FAIL
 
--- a/testing/web-platform/meta/cookies/http-state/general-tests.html.ini
+++ b/testing/web-platform/meta/cookies/http-state/general-tests.html.ini
@@ -1,18 +1,4 @@
 [general-tests.html]
-  [0021 - Ignore cookie without key in all 'Set-Cookie'.]
-    expected: FAIL
-
-  [0023 - Ignore cookies without '=' in all 'Set-Cookie'.]
-    expected: FAIL
-
-  [0026 - Ignore malformed cookies in all 'Set-Cookie' v2.]
-    expected: FAIL
-
-  [0027 - Ignore malformed cookies in all 'Set-Cookie' v3.]
-    expected: FAIL
-
   [0028 - [INVALID EXPECTATION\] Ignore malformed cookies in all 'Set-Cookie' v4.]
     expected: FAIL
 
-  [0004 - Ignore cookie without key.]
-    expected: FAIL
--- a/toolkit/components/thumbnails/test/thumbnails_background.sjs
+++ b/toolkit/components/thumbnails/test/thumbnails_background.sjs
@@ -14,23 +14,23 @@ function handleRequest(req, resp) {
   let opts = {};
   try {
     opts = JSON.parse(decodeURIComponent(req.queryString));
   }
   catch (err) {}
 
   let setCookieScript = "";
   if (opts.setRedCookie) {
-    resp.setHeader("Set-Cookie", "red", false);
-    setCookieScript = '<script>document.cookie="red";</script>';
+    resp.setHeader("Set-Cookie", "red=1", false);
+    setCookieScript = '<script>document.cookie="red=1";</script>';
   }
 
   if (opts.setGreenCookie) {
-    resp.setHeader("Set-Cookie", "green", false);
-    setCookieScript = '<script>document.cookie="green";</script>';
+    resp.setHeader("Set-Cookie", "green=1", false);
+    setCookieScript = '<script>document.cookie="green=1";</script>';
   }
 
   if (opts.iframe) {
     setCookieScript += '<iframe src="' + opts.iframe + '" />';
   }
 
   if (opts.xhr) {
     setCookieScript += `
@@ -38,24 +38,24 @@ function handleRequest(req, resp) {
          var req = new XMLHttpRequest();
          req.open("GET", "${opts.xhr}", true);
          req.send();
       </script>
     `;
   }
 
   if (req.hasHeader("Cookie") &&
-      req.getHeader("Cookie").split(";").indexOf("red") >= 0) {
+      req.getHeader("Cookie").split(";").indexOf("red=1") >= 0) {
     resp.write('<html style="background: #f00;">' + setCookieScript + '</html>');
     resp.finish();
     return;
   }
 
   if (req.hasHeader("Cookie") &&
-      req.getHeader("Cookie").split(";").indexOf("green") >= 0) {
+      req.getHeader("Cookie").split(";").indexOf("green=1") >= 0) {
     resp.write('<html style="background: #0f0;">' + setCookieScript + '</html>');
     resp.finish();
     return;
   }
 
   if (opts.redirect) {
     resp.setHeader("Location", opts.redirect);
     resp.setStatusLine(null, 303, null);