ServiceWorker: * is only wildcard at end of scope URLs

BUG=372829

Review URL: https://codereview.chromium.org/288693002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272621 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/service_worker_utils.cc

@@ -5,7 +5,7 @@
 #include "content/browser/service_worker/service_worker_utils.h"
 
 #include "base/command_line.h"
-#include "base/strings/string_util.h"
+#include "base/logging.h"
 #include "content/public/common/content_switches.h"
 
 namespace content {
@@ -19,15 +19,15 @@ bool ServiceWorkerUtils::IsFeatureEnabled() {
 
 // static
 bool ServiceWorkerUtils::ScopeMatches(const GURL& scope, const GURL& url) {
-  // This is a really basic, naive
-  // TODO(alecflett): Formalize what scope matches mean.
-  // Temporarily borrowed directly from appcache::Namespace::IsMatch().
-  // We have to escape '?' characters since MatchPattern also treats those
-  // as wildcards which we don't want here, we only do '*'s.
-  std::string scope_spec(scope.spec());
-  if (scope.has_query())
-    ReplaceSubstringsAfterOffset(&scope_spec, 0, "?", "\\?");
-  return MatchPattern(url.spec(), scope_spec);
+  DCHECK(!scope.has_ref());
+  DCHECK(!url.has_ref());
+  const std::string& scope_spec = scope.spec();
+  const std::string& url_spec = url.spec();
+
+  size_t len = scope_spec.size();
+  if (len > 0 && scope_spec[len - 1] == '*')
+    return scope_spec.compare(0, len - 1, url_spec, 0, len - 1) == 0;
+  return scope_spec == url_spec;
 }
 
 }  // namespace content

content/browser/service_worker/service_worker_utils_unittest.cc

@@ -30,6 +30,7 @@ TEST(ServiceWorkerUtilsTest, ScopeMatches) {
   ASSERT_FALSE(ServiceWorkerUtils::ScopeMatches(
       GURL("http://www.example.com/"), GURL("http://www.example.com/x")));
 
+  // '?' is not a wildcard.
   ASSERT_FALSE(ServiceWorkerUtils::ScopeMatches(
       GURL("http://www.example.com/?"), GURL("http://www.example.com/x")));
   ASSERT_FALSE(ServiceWorkerUtils::ScopeMatches(
@@ -39,6 +40,30 @@ TEST(ServiceWorkerUtilsTest, ScopeMatches) {
   ASSERT_TRUE(ServiceWorkerUtils::ScopeMatches(
       GURL("http://www.example.com/?"), GURL("http://www.example.com/?")));
 
+  // Query string is part of the resource.
+  ASSERT_TRUE(
+      ServiceWorkerUtils::ScopeMatches(GURL("http://www.example.com/?a=b"),
+                                       GURL("http://www.example.com/?a=b")));
+  ASSERT_TRUE(
+      ServiceWorkerUtils::ScopeMatches(GURL("http://www.example.com/?a=*"),
+                                       GURL("http://www.example.com/?a=b")));
+  ASSERT_TRUE(ServiceWorkerUtils::ScopeMatches(
+      GURL("http://www.example.com/*"), GURL("http://www.example.com/?a=b")));
+  ASSERT_FALSE(ServiceWorkerUtils::ScopeMatches(
+      GURL("http://www.example.com/"), GURL("http://www.example.com/?a=b")));
+
+  // '*' only has special meaning in terminal position.
+  ASSERT_TRUE(ServiceWorkerUtils::ScopeMatches(
+      GURL("http://www.example.com/*/x"), GURL("http://www.example.com/*/x")));
+  ASSERT_FALSE(ServiceWorkerUtils::ScopeMatches(
+      GURL("http://www.example.com/*/x"), GURL("http://www.example.com/a/x")));
+  ASSERT_FALSE(
+      ServiceWorkerUtils::ScopeMatches(GURL("http://www.example.com/*/x/*"),
+                                       GURL("http://www.example.com/a/x/b")));
+  ASSERT_TRUE(
+      ServiceWorkerUtils::ScopeMatches(GURL("http://www.example.com/*/x/*"),
+                                       GURL("http://www.example.com/*/x/b")));
+
   // URLs canonicalize \ to / so this is equivalent to  "...//*" and "...//x"
   ASSERT_TRUE(ServiceWorkerUtils::ScopeMatches(
       GURL("http://www.example.com/\\*"), GURL("http://www.example.com/\\x")));