Bug 1583211 - Use the URL file path when performing cookie path matching instead of the url path query ref; r=baku

Differential Revision: https://phabricator.services.mozilla.com/D46809

--HG--
extra : moz-landing-system : lando

netwerk/cookie/CookieServiceChild.cpp

@@ -292,7 +292,7 @@ void CookieServiceChild::GetCookieStringFromCookieHashTable(
 
   nsAutoCString hostFromURI, pathFromURI;
   aHostURI->GetAsciiHost(hostFromURI);
-  aHostURI->GetPathQueryRef(pathFromURI);
+  aHostURI->GetFilePath(pathFromURI);
   bool isSecure = aHostURI->SchemeIs("https");
   int64_t currentTimeInUsec = PR_Now();
   int64_t currentTime = currentTimeInUsec / PR_USEC_PER_SEC;

netwerk/cookie/nsCookie.cpp

@@ -8,6 +8,8 @@
 #include "mozilla/StaticPrefs_network.h"
 #include "nsAutoPtr.h"
 #include "nsCookie.h"
+#include "nsIURLParser.h"
+#include "nsURLHelper.h"
 #include <stdlib.h>
 
 /******************************************************************************
@@ -87,7 +89,8 @@ size_t nsCookie::SizeOfIncludingThis(
          mData.name().SizeOfExcludingThisIfUnshared(MallocSizeOf) +
          mData.value().SizeOfExcludingThisIfUnshared(MallocSizeOf) +
          mData.host().SizeOfExcludingThisIfUnshared(MallocSizeOf) +
-         mData.path().SizeOfExcludingThisIfUnshared(MallocSizeOf);
+         mData.path().SizeOfExcludingThisIfUnshared(MallocSizeOf) +
+         mFilePathCache.SizeOfExcludingThisIfUnshared(MallocSizeOf);
 }
 
 bool nsCookie::IsStale() const {
@@ -170,6 +173,35 @@ nsCookie::GetOriginAttributes(JSContext* aCx,
   return NS_OK;
 }
 
+const nsCString& nsCookie::GetFilePath() {
+  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
+
+  if (Path().IsEmpty()) {
+    // If we don't have a path, just return the (empty) file path cache.
+    return mFilePathCache;
+  }
+  if (!mFilePathCache.IsEmpty()) {
+    // If we've computed the answer before, just return it.
+    return mFilePathCache;
+  }
+
+  nsIURLParser* parser = net_GetStdURLParser();
+  NS_ENSURE_TRUE(parser, mFilePathCache);
+
+  int32_t pathLen = Path().Length(), filepathLen = 0;
+  uint32_t filepathPos = 0;
+
+  nsresult rv = parser->ParsePath(PromiseFlatCString(Path()).get(), pathLen,
+                                  &filepathPos, &filepathLen, nullptr,
+                                  nullptr,            // don't care about query
+                                  nullptr, nullptr);  // don't care about ref
+  NS_ENSURE_SUCCESS(rv, mFilePathCache);
+
+  mFilePathCache = Substring(Path(), filepathPos, filepathLen);
+
+  return mFilePathCache;
+}
+
 // compatibility method, for use with the legacy nsICookie interface.
 // here, expires == 0 denotes a session cookie.
 NS_IMETHODIMP

netwerk/cookie/nsCookie.h

@@ -70,6 +70,7 @@ class nsCookie final : public nsICookie {
     return nsDependentCSubstring(mData.host(), IsDomain() ? 1 : 0);
   }
   inline const nsCString& Path() const { return mData.path(); }
+  const nsCString& GetFilePath();
   inline int64_t Expiry() const { return mData.expiry(); }  // in seconds
   inline int64_t LastAccessed() const {
     return mData.lastAccessed();
@@ -108,6 +109,7 @@ class nsCookie final : public nsICookie {
   // Please update SizeOfIncludingThis if this strategy changes.
   mozilla::net::CookieStruct mData;
   mozilla::OriginAttributes mOriginAttributes;
+  nsCString mFilePathCache;
 };
 
 // Comparator class for sorting cookies before sending to a server.

netwerk/cookie/nsCookieService.cpp

@@ -2949,23 +2949,25 @@ bool nsCookieService::DomainMatches(nsCookie* aCookie,
 }
 
 bool nsCookieService::PathMatches(nsCookie* aCookie, const nsACString& aPath) {
+  nsCString cookiePath(aCookie->GetFilePath());
+
   // if our cookie path is empty we can't really perform our prefix check, and
   // also we can't check the last character of the cookie path, so we would
   // never return a successful match.
-  if (aCookie->Path().IsEmpty()) return false;
+  if (cookiePath.IsEmpty()) return false;
 
   // if the cookie path and the request path are identical, they match.
-  if (aCookie->Path().Equals(aPath)) return true;
+  if (cookiePath.Equals(aPath)) return true;
 
   // if the cookie path is a prefix of the request path, and the last character
   // of the cookie path is %x2F ("/"), they match.
-  bool isPrefix = StringBeginsWith(aPath, aCookie->Path());
-  if (isPrefix && aCookie->Path().Last() == '/') return true;
+  bool isPrefix = StringBeginsWith(aPath, cookiePath);
+  if (isPrefix && cookiePath.Last() == '/') return true;
 
   // if the cookie path is a prefix of the request path, and the first character
   // of the request path that is not included in the cookie path is a %x2F ("/")
   // character, they match.
-  uint32_t cookiePathLen = aCookie->Path().Length();
+  uint32_t cookiePathLen = cookiePath.Length();
   if (isPrefix && aPath[cookiePathLen] == '/') return true;
 
   return false;
@@ -3000,7 +3002,7 @@ void nsCookieService::GetCookiesForURI(
   nsresult rv =
       GetBaseDomain(mTLDService, aHostURI, baseDomain, requireHostMatch);
   if (NS_SUCCEEDED(rv)) rv = aHostURI->GetAsciiHost(hostFromURI);
-  if (NS_SUCCEEDED(rv)) rv = aHostURI->GetPathQueryRef(pathFromURI);
+  if (NS_SUCCEEDED(rv)) rv = aHostURI->GetFilePath(pathFromURI);
   if (NS_FAILED(rv)) {
     COOKIE_LOGFAILURE(GET_COOKIE, aHostURI, VoidCString(),
                       "invalid host/path from URI");
@@ -4877,7 +4879,7 @@ bool nsCookieService::FindSecureCookie(const nsCookieKey& aKey,
       // aren't "/", then this situation needs to compare paths to
       // ensure only that a newly-created non-secure cookie does not
       // overlay an existing secure cookie.
-      if (PathMatches(cookie, aCookie->Path())) {
+      if (PathMatches(cookie, aCookie->GetFilePath())) {
         return true;
       }
     }

netwerk/test/TestCookie.cpp

@@ -332,7 +332,7 @@ TEST(TestCookie, TestCookieMain)
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, "test=path"));
   GetACookie(cookieService, "http://path.net/path?hithere/foo", nullptr,
              cookie);
-  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, "test=path"));
   GetACookie(cookieService, "http://path.net/path2", nullptr, cookie);
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
   GetACookie(cookieService, "http://path.net/path2/", nullptr, cookie);