Bug 1590922 - Fix parsing the blocklist in nsContentUtils::IsURIInList(); r=baku

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

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

dom/base/nsContentUtils.cpp

@@ -10174,13 +10174,6 @@ bool nsContentUtils::IsURIInList(nsIURI* aURI, const nsCString& aBlackList) {
     return false;
   }
 
-  nsAutoCString host;
-  aURI->GetHost(host);
-  if (host.IsEmpty()) {
-    return false;
-  }
-  ToLowerCase(host);
-
   if (aBlackList.IsEmpty()) {
     return false;
   }
@@ -10188,42 +10181,54 @@ bool nsContentUtils::IsURIInList(nsIURI* aURI, const nsCString& aBlackList) {
   // The list is comma separated domain list.  Each item may start with "*.".
   // If starts with "*.", it matches any sub-domains.
 
-  for (;;) {
-    int32_t index = aBlackList.Find(host, false);
-    if (index >= 0 &&
-        static_cast<uint32_t>(index) + host.Length() <= aBlackList.Length() &&
-        // If start of the black list or next to ","?
-        (!index || aBlackList[index - 1] == ',')) {
-      // If end of the black list or immediately before ","?
-      size_t indexAfterHost = index + host.Length();
-      if (indexAfterHost == aBlackList.Length() ||
-          aBlackList[indexAfterHost] == ',') {
-        return true;
-      }
-      // If next character is '/', we need to check the path too.
-      // We assume the path in blacklist means "/foo" + "*".
-      if (aBlackList[indexAfterHost] == '/') {
-        int32_t endOfPath = aBlackList.Find(",", false, indexAfterHost);
-        nsDependentCSubstring::size_type length =
-            endOfPath < 0 ? static_cast<nsDependentCSubstring::size_type>(-1)
-                          : endOfPath - indexAfterHost;
-        nsDependentCSubstring pathInBlackList(aBlackList, indexAfterHost,
-                                              length);
-        nsAutoCString filePath;
-        aURI->GetFilePath(filePath);
-        ToLowerCase(filePath);
-        if (StringBeginsWith(filePath, pathInBlackList)) {
-          return true;
-        }
-      }
-    }
-    int32_t startIndexOfCurrentLevel = host[0] == '*' ? 1 : 0;
-    int32_t startIndexOfNextLevel =
-        host.Find(".", false, startIndexOfCurrentLevel + 1);
-    if (startIndexOfNextLevel <= 0) {
+  nsCCharSeparatedTokenizer tokenizer(aBlackList, ',');
+  while (tokenizer.hasMoreTokens()) {
+    const nsCString token(tokenizer.nextToken());
+
+    nsAutoCString host;
+    aURI->GetHost(host);
+    if (host.IsEmpty()) {
       return false;
     }
-    host = NS_LITERAL_CSTRING("*") +
-           nsDependentCSubstring(host, startIndexOfNextLevel);
+    ToLowerCase(host);
+
+    for (;;) {
+      int32_t index = token.Find(host, false);
+      if (index >= 0 &&
+          static_cast<uint32_t>(index) + host.Length() <= token.Length()) {
+        // If we found a full match, return true.
+        size_t indexAfterHost = index + host.Length();
+        if (index == 0 && indexAfterHost == token.Length()) {
+          return true;
+        }
+        // If next character is '/', we need to check the path too.
+        // We assume the path in blacklist means "/foo" + "*".
+        if (token[indexAfterHost] == '/') {
+          nsDependentCSubstring pathInBlackList(
+              token, indexAfterHost,
+              static_cast<nsDependentCSubstring::size_type>(-1));
+          nsAutoCString filePath;
+          aURI->GetFilePath(filePath);
+          ToLowerCase(filePath);
+          if (StringBeginsWith(filePath, pathInBlackList) &&
+              (filePath.Length() == pathInBlackList.Length() ||
+               pathInBlackList.EqualsLiteral("/") ||
+               filePath[pathInBlackList.Length()] == '/' ||
+               filePath[pathInBlackList.Length()] == '?')) {
+            return true;
+          }
+        }
+      }
+      int32_t startIndexOfCurrentLevel = host[0] == '*' ? 1 : 0;
+      int32_t startIndexOfNextLevel =
+          host.Find(".", false, startIndexOfCurrentLevel + 1);
+      if (startIndexOfNextLevel <= 0) {
+        break;
+      }
+      host = NS_LITERAL_CSTRING("*") +
+             nsDependentCSubstring(host, startIndexOfNextLevel);
+    }
   }
+
+  return false;
 }

dom/base/test/gtest/TestContentUtils.cpp

@@ -11,6 +11,56 @@
 #include "mozilla/CycleCollectedJSContext.h"
 #include "mozilla/dom/SimpleGlobalObject.h"
 
+struct IsURIInListMatch {
+  nsLiteralCString pattern;
+  bool firstMatch, secondMatch;
+};
+
+TEST(DOM_Base_ContentUtils, IsURIInList)
+{
+  nsCOMPtr<nsIURI> uri, subURI;
+  nsresult rv =
+      NS_NewURI(getter_AddRefs(uri),
+                NS_LITERAL_CSTRING("https://example.com/path/favicon.ico"));
+  ASSERT_TRUE(rv == NS_OK);
+
+  rv = NS_NewURI(getter_AddRefs(subURI),
+                 NS_LITERAL_CSTRING("http://sub.example.com/favicon.ico?"));
+  ASSERT_TRUE(rv == NS_OK);
+
+  static constexpr IsURIInListMatch patterns[] = {
+      {NS_LITERAL_CSTRING("bar.com,*.example.com,example.com,foo.com"), true,
+       true},
+      {NS_LITERAL_CSTRING("bar.com,example.com,*.example.com,foo.com"), true,
+       true},
+      {NS_LITERAL_CSTRING("*.example.com,example.com,foo.com"), true, true},
+      {NS_LITERAL_CSTRING("example.com,*.example.com,foo.com"), true, true},
+      {NS_LITERAL_CSTRING("*.example.com,example.com"), true, true},
+      {NS_LITERAL_CSTRING("example.com,*.example.com"), true, true},
+      {NS_LITERAL_CSTRING("*.example.com/,example.com/"), true, true},
+      {NS_LITERAL_CSTRING("example.com/,*.example.com/"), true, true},
+      {NS_LITERAL_CSTRING("*.example.com/pa,example.com/pa"), false, false},
+      {NS_LITERAL_CSTRING("example.com/pa,*.example.com/pa"), false, false},
+      {NS_LITERAL_CSTRING("*.example.com/pa/,example.com/pa/"), false, false},
+      {NS_LITERAL_CSTRING("example.com/pa/,*.example.com/pa/"), false, false},
+      {NS_LITERAL_CSTRING("*.example.com/path,example.com/path"), true, false},
+      {NS_LITERAL_CSTRING("example.com/path,*.example.com/path"), true, false},
+      {NS_LITERAL_CSTRING("*.example.com/favicon.ico"), false, true},
+      {NS_LITERAL_CSTRING("*.example.com"), false, true},
+      {NS_LITERAL_CSTRING("example.com"), true, false},
+      {NS_LITERAL_CSTRING("foo.com"), false, false},
+      {NS_LITERAL_CSTRING("*.foo.com"), false, false},
+  };
+
+  for (auto& entry : patterns) {
+    bool result = nsContentUtils::IsURIInList(uri, entry.pattern);
+    ASSERT_EQ(result, entry.firstMatch) << "Matching " << entry.pattern;
+
+    result = nsContentUtils::IsURIInList(subURI, entry.pattern);
+    ASSERT_EQ(result, entry.secondMatch) << "Matching " << entry.pattern;
+  }
+}
+
 TEST(DOM_Base_ContentUtils, StringifyJSON_EmptyValue)
 {
   JS::RootedObject globalObject(