Backed out 2 changesets (bug 1293067) for failures on browser_oversize.js.
authorCosmin Sabou <csabou@mozilla.com>
Wed, 25 Mar 2020 13:24:19 +0200
changeset 520346 0577d24b6ca5be503767d9e82536d431bcabe575
parent 520345 5df9d39b0b98eb17eecad01752b736a28c316cc6
child 520347 1bac37288dedbd859353227ad8b722475b6d7d64
push id37248
push userbtara@mozilla.com
push dateWed, 25 Mar 2020 16:40:49 +0000
treeherdermozilla-central@c5112a7573ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1293067
milestone76.0a1
backs out5df9d39b0b98eb17eecad01752b736a28c316cc6
8ed41aa3ed4f2611ed2efd0bf835542874155071
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 2 changesets (bug 1293067) for failures on browser_oversize.js. CLOSED TREE Backed out changeset 5df9d39b0b98 (bug 1293067) Backed out changeset 8ed41aa3ed4f (bug 1293067)
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
netwerk/cookie/test/browser/browser.ini
netwerk/cookie/test/browser/browser_oversize.js
netwerk/cookie/test/browser/oversize.sjs
netwerk/locales/en-US/necko.properties
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -3259,27 +3259,16 @@ bool nsCookieService::CanSetCookie(nsIUR
     aCookieData.isSession() = true;
   }
 
   // reject cookie if it's over the size limit, per RFC2109
   if ((aCookieData.name().Length() + aCookieData.value().Length()) >
       kMaxBytesPerCookie) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
                       "cookie too big (> 4kb)");
-
-    AutoTArray<nsString, 2> params = {
-        NS_ConvertUTF8toUTF16(aCookieData.name())};
-
-    nsString size;
-    size.AppendInt(kMaxBytesPerCookie);
-    params.AppendElement(size);
-
-    LogMessageToConsole(aChannel, aHostURI, nsIScriptError::warningFlag,
-                        CONSOLE_GENERIC_CATEGORY,
-                        NS_LITERAL_CSTRING("CookieOversize"), params);
     return newCookie;
   }
 
   const char illegalNameCharacters[] = {
       0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B,
       0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16,
       0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, 0x00};
 
@@ -3292,17 +3281,17 @@ bool nsCookieService::CanSetCookie(nsIUR
   // domain & path checks
   if (!CheckDomain(aCookieData, aHostURI, aKey.mBaseDomain,
                    aRequireHostMatch)) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
                       "failed the domain tests");
     return newCookie;
   }
 
-  if (!CheckPath(aCookieData, aChannel, aHostURI)) {
+  if (!CheckPath(aCookieData, aHostURI)) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
                       "failed the path tests");
     return newCookie;
   }
 
   // magic prefix checks. MUST be run after CheckDomain() and CheckPath()
   if (!CheckPrefixes(aCookieData, potentiallyTurstworthy)) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
@@ -3861,100 +3850,102 @@ bool nsCookieService::ParseAttributes(ns
         aCookieData.sameSite() = nsICookie::SAMESITE_STRICT;
         aCookieData.rawSameSite() = nsICookie::SAMESITE_STRICT;
         sameSiteSet = true;
       } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteNone)) {
         aCookieData.sameSite() = nsICookie::SAMESITE_NONE;
         aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE;
         sameSiteSet = true;
       } else {
-        LogMessageToConsole(
-            aChannel, aHostURI, nsIScriptError::infoFlag,
-            CONSOLE_GENERIC_CATEGORY,
-            NS_LITERAL_CSTRING("CookieSameSiteValueInvalid"),
-            AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
+        LogMessageToConsole(aChannel, aHostURI, nsIScriptError::infoFlag,
+                            CONSOLE_GENERIC_CATEGORY,
+                            NS_LITERAL_CSTRING("CookieSameSiteValueInvalid"),
+                            aCookieData.name());
       }
     }
   }
 
   Telemetry::Accumulate(Telemetry::COOKIE_SAMESITE_SET_VS_UNSET,
                         sameSiteSet ? 1 : 0);
 
   // re-assign aCookieHeader, in case we need to process another cookie
   aCookieHeader.Assign(Substring(cookieStart, cookieEnd));
 
   // If same-site is set to 'none' but this is not a secure context, let's abort
   // the parsing.
   if (!aCookieData.isSecure() &&
       aCookieData.sameSite() == nsICookie::SAMESITE_NONE) {
     if (laxByDefault &&
         StaticPrefs::network_cookie_sameSite_noneRequiresSecure()) {
-      LogMessageToConsole(
-          aChannel, aHostURI, nsIScriptError::infoFlag,
-          CONSOLE_SAMESITE_CATEGORY,
-          NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecure"),
-          AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
+      LogMessageToConsole(aChannel, aHostURI, nsIScriptError::infoFlag,
+                          CONSOLE_SAMESITE_CATEGORY,
+                          NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecure"),
+                          aCookieData.name());
       return newCookie;
     }
 
     // if sameSite=lax by default is disabled, we want to warn the user.
     LogMessageToConsole(
         aChannel, aHostURI, nsIScriptError::warningFlag,
         CONSOLE_SAMESITE_CATEGORY,
         NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecureForBeta"),
-        AutoTArray<nsString, 2>{NS_ConvertUTF8toUTF16(aCookieData.name()),
-                                SAMESITE_MDN_URL});
+        aCookieData.name(), SAMESITE_MDN_URL);
   }
 
   if (aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE &&
       aCookieData.sameSite() == nsICookie::SAMESITE_LAX) {
     if (laxByDefault) {
-      LogMessageToConsole(
-          aChannel, aHostURI, nsIScriptError::infoFlag,
-          CONSOLE_SAMESITE_CATEGORY, NS_LITERAL_CSTRING("CookieLaxForced"),
-          AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
+      LogMessageToConsole(aChannel, aHostURI, nsIScriptError::infoFlag,
+                          CONSOLE_SAMESITE_CATEGORY,
+                          NS_LITERAL_CSTRING("CookieLaxForced"),
+                          aCookieData.name());
     } else {
-      LogMessageToConsole(
-          aChannel, aHostURI, nsIScriptError::warningFlag,
-          CONSOLE_SAMESITE_CATEGORY,
-          NS_LITERAL_CSTRING("CookieLaxForcedForBeta"),
-          AutoTArray<nsString, 2>{NS_ConvertUTF8toUTF16(aCookieData.name()),
-                                  SAMESITE_MDN_URL});
+      LogMessageToConsole(aChannel, aHostURI, nsIScriptError::warningFlag,
+                          CONSOLE_SAMESITE_CATEGORY,
+                          NS_LITERAL_CSTRING("CookieLaxForcedForBeta"),
+                          aCookieData.name(), SAMESITE_MDN_URL);
     }
   }
 
   // Cookie accepted.
   aAcceptedByParser = true;
 
   MOZ_ASSERT(nsCookie::ValidateRawSame(aCookieData));
   return newCookie;
 }
 
 // static
 void nsCookieService::LogMessageToConsole(nsIChannel* aChannel, nsIURI* aURI,
                                           uint32_t aErrorFlags,
                                           const nsACString& aCategory,
                                           const nsACString& aMsg,
-                                          const nsTArray<nsString>& aParams) {
+                                          const nsACString& aCookieName,
+                                          const nsAString& aMDNURL) {
   MOZ_ASSERT(aURI);
 
   nsCOMPtr<HttpBaseChannel> httpChannel = do_QueryInterface(aChannel);
   if (!httpChannel) {
     return;
   }
 
   nsAutoCString uri;
   nsresult rv = aURI->GetSpec(uri);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
+  AutoTArray<nsString, 1> params = {NS_ConvertUTF8toUTF16(aCookieName)};
+
+  if (!aMDNURL.IsEmpty()) {
+    params.AppendElement(aMDNURL);
+  }
+
   httpChannel->AddConsoleReport(aErrorFlags, aCategory,
                                 nsContentUtils::eNECKO_PROPERTIES, uri, 0, 0,
-                                aMsg, aParams);
+                                aMsg, params);
 }
 
 /******************************************************************************
  * nsCookieService impl:
  * private domain & permission compliance enforcement functions
  ******************************************************************************/
 
 // Get the base domain for aHostURI; e.g. for "www.bbc.co.uk", this would be
@@ -4257,18 +4248,17 @@ nsAutoCString nsCookieService::GetPathFr
   if (lastSlash != firstSlash && lastSlash != kNotFound &&
       lastSlash == (int32_t)(path.Length() - 1)) {
     path.Truncate(lastSlash);
   }
 
   return path;
 }
 
-bool nsCookieService::CheckPath(CookieStruct& aCookieData, nsIChannel* aChannel,
-                                nsIURI* aHostURI) {
+bool nsCookieService::CheckPath(CookieStruct& aCookieData, nsIURI* aHostURI) {
   // if a path is given, check the host has permission
   if (aCookieData.path().IsEmpty() || aCookieData.path().First() != '/') {
     aCookieData.path() = GetPathFromURI(aHostURI);
 
 #if 0
   } else {
     /**
      * The following test is part of the RFC2109 spec.  Loosely speaking, it says that a site
@@ -4280,33 +4270,19 @@ bool nsCookieService::CheckPath(CookieSt
     nsAutoCString pathFromURI;
     if (NS_FAILED(aHostURI->GetPathQueryRef(pathFromURI)) ||
         !StringBeginsWith(pathFromURI, aCookieData.path())) {
       return false;
     }
 #endif
   }
 
-  if (aCookieData.path().Length() > kMaxBytesPerPath) {
-    AutoTArray<nsString, 2> params = {
-        NS_ConvertUTF8toUTF16(aCookieData.name())};
-
-    nsString size;
-    size.AppendInt(kMaxBytesPerPath);
-    params.AppendElement(size);
-
-    LogMessageToConsole(aChannel, aHostURI, nsIScriptError::warningFlag,
-                        CONSOLE_GENERIC_CATEGORY,
-                        NS_LITERAL_CSTRING("CookiePathOversize"), params);
+  if (aCookieData.path().Length() > kMaxBytesPerPath ||
+      aCookieData.path().Contains('\t'))
     return false;
-  }
-
-  if (aCookieData.path().Contains('\t')) {
-    return false;
-  }
 
   return true;
 }
 
 // CheckPrefixes
 //
 // Reject cookies whose name starts with the magic prefixes from
 // https://tools.ietf.org/html/draft-ietf-httpbis-cookie-prefixes-00
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -302,17 +302,17 @@ class nsCookieService final : public nsI
                               mozilla::net::CookieStruct& aCookieData,
                               nsACString& aExpires, nsACString& aMaxage,
                               bool& aAcceptedByParser);
   bool RequireThirdPartyCheck();
   static bool CheckDomain(mozilla::net::CookieStruct& aCookieData,
                           nsIURI* aHostURI, const nsCString& aBaseDomain,
                           bool aRequireHostMatch);
   static bool CheckPath(mozilla::net::CookieStruct& aCookieData,
-                        nsIChannel* aChannel, nsIURI* aHostURI);
+                        nsIURI* aHostURI);
   static bool CheckPrefixes(mozilla::net::CookieStruct& aCookieData,
                             bool aSecureRequest);
   static bool GetExpiry(mozilla::net::CookieStruct& aCookieData,
                         const nsACString& aExpires, const nsACString& aMaxage,
                         int64_t aServerTime, int64_t aCurrentTime,
                         bool aFromHttp);
   void RemoveAllFromMemory();
   already_AddRefed<nsIArray> PurgeCookies(int64_t aCurrentTimeInUsec);
@@ -348,17 +348,18 @@ class nsCookieService final : public nsI
   nsresult RemoveCookiesFromExactHost(
       const nsACString& aHost,
       const mozilla::OriginAttributesPattern& aPattern);
 
   static void LogMessageToConsole(nsIChannel* aChannel, nsIURI* aURI,
                                   uint32_t aErrorFlags,
                                   const nsACString& aCategory,
                                   const nsACString& aMsg,
-                                  const nsTArray<nsString>& aParams);
+                                  const nsACString& aCookieName,
+                                  const nsAString& aMDNURL = VoidString());
 
   // cached members.
   nsCOMPtr<nsICookiePermission> mPermissionService;
   nsCOMPtr<mozIThirdPartyUtil> mThirdPartyUtil;
   nsCOMPtr<nsIEffectiveTLDService> mTLDService;
   nsCOMPtr<nsIIDNService> mIDNService;
   nsCOMPtr<mozIStorageService> mStorageService;
 
--- a/netwerk/cookie/test/browser/browser.ini
+++ b/netwerk/cookie/test/browser/browser.ini
@@ -11,10 +11,8 @@ support-files = server.sjs
 [browser_indexedDB.js]
 [browser_originattributes.js]
 [browser_storage.js]
 [browser_serviceWorker.js]
 [browser_sharedWorker.js]
 skip-if = fission #Bug 1582318
 [browser_sameSiteConsole.js]
 support-files = sameSite.sjs
-[browser_oversize.js]
-support-files = oversize.sjs
deleted file mode 100644
--- a/netwerk/cookie/test/browser/browser_oversize.js
+++ /dev/null
@@ -1,94 +0,0 @@
-const OVERSIZE_DOMAIN = "http://example.com/";
-const OVERSIZE_PATH = "browser/netwerk/cookie/test/browser/";
-const OVERSIZE_TOP_PAGE = OVERSIZE_DOMAIN + OVERSIZE_PATH + "oversize.sjs";
-
-add_task(async _ => {
-  const expected = [];
-
-  const consoleListener = {
-    observe(what) {
-      if (!(what instanceof Ci.nsIConsoleMessage)) {
-        return;
-      }
-
-      info("Console Listener: " + what);
-      for (let i = expected.length - 1; i >= 0; --i) {
-        const e = expected[i];
-
-        if (what.message.includes(e.match)) {
-          ok(true, "Message received: " + e.match);
-          expected.splice(i, 1);
-          e.resolve();
-        }
-      }
-    },
-  };
-
-  Services.console.registerListener(consoleListener);
-
-  registerCleanupFunction(() =>
-    Services.console.unregisterListener(consoleListener)
-  );
-
-  const netPromises = [
-    new Promise(resolve => {
-      expected.push({
-        resolve,
-        match:
-          "Cookie “a” is invalid because its size is too big. Max size is 4096 bytes.",
-      });
-    }),
-
-    new Promise(resolve => {
-      expected.push({
-        resolve,
-        match:
-          "Cookie “b” is invalid because its path size is too big. Max size is 1024 bytes.",
-      });
-    }),
-  ];
-
-  // Let's open our tab.
-  const tab = BrowserTestUtils.addTab(gBrowser, OVERSIZE_TOP_PAGE);
-  gBrowser.selectedTab = tab;
-
-  const browser = gBrowser.getBrowserForTab(tab);
-  await BrowserTestUtils.browserLoaded(browser);
-
-  // Let's wait for the first set of console events.
-  await Promise.all(netPromises);
-
-  // the DOM list of events.
-  const domPromises = [
-    new Promise(resolve => {
-      expected.push({
-        resolve,
-        match:
-          "Cookie “d” is invalid because its size is too big. Max size is 4096 bytes.",
-      });
-    }),
-
-    new Promise(resolve => {
-      expected.push({
-        resolve,
-        match:
-          "Cookie “e” is invalid because its path size is too big. Max size is 1024 bytes.",
-      });
-    }),
-  ];
-
-  // Let's use document.cookie
-  SpecialPowers.spawn(browser, [], () => {
-    const maxBytesPerCookie = 4096;
-    const maxBytesPerCookiePath = 1024;
-    content.document.cookie = "d=" + Array(maxBytesPerCookie + 1).join("x");
-    content.document.cookie =
-      "e=f; path=/" + Array(maxBytesPerCookiePath + 1).join("x");
-  });
-
-  // Let's wait for the dom events.
-  await Promise.all(domPromises);
-
-  // Let's close the tab.
-  BrowserTestUtils.removeTab(tab);
-});
deleted file mode 100644
--- a/netwerk/cookie/test/browser/oversize.sjs
+++ /dev/null
@@ -1,9 +0,0 @@
-function handleRequest(aRequest, aResponse) {
-  aResponse.setStatusLine(aRequest.httpVersion, 200);
-
-  const maxBytesPerCookie = 4096;
-  const maxBytesPerCookiePath = 1024;
-
-  aResponse.setHeader("Set-Cookie", "a=" + Array(maxBytesPerCookie + 1).join('x'), true);
-  aResponse.setHeader("Set-Cookie", "b=c; path=/" + Array(maxBytesPerCookiePath + 1).join('x'), true);
-}
--- a/netwerk/locales/en-US/necko.properties
+++ b/netwerk/locales/en-US/necko.properties
@@ -54,12 +54,8 @@ CookieRejectedNonRequiresSecure=Cookie “%1$S” rejected because it has the “sameSite=none” attribute but is missing the “secure” attribute.
 # LOCALIZATION NOTE(CookieRejectedNonRequiresSecureForBeta): %1$S is the cookie name. %2$S is a URL. Do not localize "sameSite", "sameSite=none" and "secure".
 CookieRejectedNonRequiresSecureForBeta=Cookie “%1$S” will be soon rejected because it has the “sameSite” attribute set to “none” or an invalid value, without the “secure” attribute. To know more about the “sameSite“ attribute, read %2$S
 # LOCALIZATION NOTE(CookieLaxForced): %1$S is the cookie name. Do not localize "sameSite", "lax" and "sameSite=lax".
 CookieLaxForced=Cookie “%1$S” has “sameSite” policy set to “lax” because it is missing a “sameSite” attribute, and “sameSite=lax” is the default value for this attribute.
 # LOCALIZATION NOTE(CookieLaxForcedForBeta): %1$S is the cookie name. %2$S is a URL. Do not localize "sameSite", "lax" and "sameSite=lax", "sameSite=none".
 CookieLaxForcedForBeta=Cookie “%1$S” does not have a proper “sameSite” attribute value. Soon, cookies without the “sameSite” attribute or with an invalid value will be treated as “lax”. This means that the cookie will no longer be sent in third-party contexts. If your application depends on this cookie being available in such contexts, please add the “sameSite=none“ attribute to it. To know more about the “sameSite“ attribute, read %2$S
 # LOCALIZATION NOTE: %1$S is cookie name. Do not localize "sameSite", "lax", "strict" and "none"
 CookieSameSiteValueInvalid=Invalid “sameSite“ value for cookie “%1$S”. The supported values are: “lax“, “strict“, “none“.
-# LOCALIZATION NOTE (CookieOversize): %1$S is the cookie name. %2$S is the number of bytes. "B" means bytes.
-CookieOversize=Cookie “%1$S” is invalid because its size is too big. Max size is %2$S B.
-# LOCALIZATION NOTE (CookiePathOversiz): %1$S is the cookie name. %2$S is the number of bytes. "B" means bytes.
-CookiePathOversize=Cookie “%1$S” is invalid because its path size is too big. Max size is %2$S B.