Bug 1911834 - Implement matches OR includeGlobs semantics for user scripts r=zombie

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

dom/chrome-webidl/WebExtensionContentScript.webidl

@@ -85,6 +85,14 @@ interface MozDocumentMatcher {
   [Constant]
   readonly attribute MatchPatternSet? excludeMatches;
 
+  /**
+   * Whether the matcher is used by the MV3 userScripts API.
+   * If true, URLs are matched when they match "matches" OR "includeGlobs",
+   * instead of the usual AND.
+   */
+  [Constant]
+  readonly attribute boolean isUserScript;
+
   /**
    * The originAttributesPattern for which this script should be enabled for.
    */
@@ -119,6 +127,8 @@ dictionary MozDocumentMatcherInit {
 
   sequence<MatchGlobOrString>? excludeGlobs = null;
 
+  boolean isUserScript = false;
+
   boolean hasActiveTabPermission = false;
 };
 

toolkit/components/extensions/ExtensionUserScripts.sys.mjs

@@ -409,8 +409,13 @@ class UserScriptsManager {
       // scripting API, in ExtensionScriptingStore.getInitialScriptIdsMap.
       allFrames: publicScript.allFrames ?? oldScript?.allFrames ?? false,
       jsPaths,
-      // TODO bug 1911834: use nonEmptyOrNull when matches can be omitted.
-      matches: publicScript.matches || oldScript.matches,
+      // WebExtensionContentScript requires matches to be set to an Array.
+      // Although "matches" is optional in the userScripts API (because
+      // includeGlobs can be used instead with OR semantics), it is required
+      // for most other use cases (content scripts and MozDocumentMatcher). For
+      // clarity, WebExtensionContentScript.webidl therefore marks "matches" as
+      // a required array, and we fall back to an empty array if needed.
+      matches: publicScript.matches || oldScript?.matches || [],
       excludeMatches: nonEmptyOrNull(
         publicScript.excludeMatches || oldScript?.excludeMatches
       ),
@@ -459,8 +464,8 @@ class UserScriptsManager {
       id: publicId,
       allFrames: internalScript.allFrames,
       js,
-      // TODO bug 1911834: simplify to internalScript.matches when "matches"
-      // can be omitted.
+      // See #makeInternalUserScript - "matches" is internally required, but if
+      // it is an empty array, it is semantically equivalent to null.
       matches: internalScript.matches.length ? internalScript.matches : null,
       excludeMatches: internalScript.excludeMatches,
       includeGlobs: internalScript.includeGlobs,
@@ -488,7 +493,6 @@ class UserScriptsManager {
     // permits an empty js array. The "js" property is guaranteed to exist
     // because it is a required key in userScripts.register.
 
-    // TODO bug 1911834: require only userScripts matches or includeGlobs.
     if (!script.matches?.length && !script.includeGlobs?.length) {
       throw new ExtensionUtils.ExtensionError(
         "matches or includeGlobs must be specified."

toolkit/components/extensions/WebExtensionContentScript.h

@@ -148,6 +148,8 @@ class MozDocumentMatcher : public nsISupports, public nsWrapperCache {
   MatchPatternSet* GetExcludeMatches() { return mExcludeMatches; }
   const MatchPatternSet* GetExcludeMatches() const { return mExcludeMatches; }
 
+  bool IsUserScript() const { return mIsUserScript; }
+
   Nullable<uint64_t> GetFrameID() const { return mFrameID; }
 
   void GetOriginAttributesPatterns(JSContext* aCx,
@@ -177,6 +179,7 @@ class MozDocumentMatcher : public nsISupports, public nsWrapperCache {
 
   Nullable<MatchGlobSet> mIncludeGlobs;
   Nullable<MatchGlobSet> mExcludeGlobs;
+  bool mIsUserScript;
 
   bool mAllFrames;
   bool mCheckPermissions;

toolkit/components/extensions/WebExtensionPolicy.cpp

@@ -705,6 +705,7 @@ MozDocumentMatcher::MozDocumentMatcher(GlobalObject& aGlobal,
                                        bool aRestricted, ErrorResult& aRv)
     : mHasActiveTabPermission(aInit.mHasActiveTabPermission),
       mRestricted(aRestricted),
+      mIsUserScript(aInit.mIsUserScript),
       mAllFrames(aInit.mAllFrames),
       mCheckPermissions(aInit.mCheckPermissions),
       mFrameID(aInit.mFrameID),
@@ -872,18 +873,34 @@ bool MozDocumentMatcher::MatchesURI(const URLInfo& aURL,
                                     bool aIgnorePermissions) const {
   MOZ_ASSERT((!mRestricted && !mCheckPermissions) || mExtension);
 
-  if (!mMatches->Matches(aURL)) {
-    return false;
+  if (MOZ_LIKELY(!mIsUserScript)) {
+    // mMatches must always match for normal content scripts.
+    if (!mMatches->Matches(aURL)) {
+      return false;
+    }
+    // mIncludeGlobs is optional, but if specified must match.
+    if (!mIncludeGlobs.IsNull() &&
+        !mIncludeGlobs.Value().Matches(aURL.CSpec())) {
+      return false;
+    }
+  } else {
+    // Unlike normal content scripts that match by (mMatches AND mIncludeGlobs),
+    // user scripts accept if there is any match: (mMatches OR mIncludeGlobs).
+    // This implies that mMatches does not have to be specified.
+    // mMatches is not a Nullable because we want it to be specified for content
+    // scripts (which is by far the most common case). An empty MatchPatternSet
+    // is equivalent to an unspecified (non-matching) mMatches.
+    if (!mMatches->Matches(aURL) &&
+        (mIncludeGlobs.IsNull() ||
+         !mIncludeGlobs.Value().Matches(aURL.CSpec()))) {
+      return false;
+    }
   }
 
   if (mExcludeMatches && mExcludeMatches->Matches(aURL)) {
     return false;
   }
 
-  if (!mIncludeGlobs.IsNull() && !mIncludeGlobs.Value().Matches(aURL.CSpec())) {
-    return false;
-  }
-
   if (!mExcludeGlobs.IsNull() && mExcludeGlobs.Value().Matches(aURL.CSpec())) {
     return false;
   }

toolkit/components/extensions/test/xpcshell/test_WebExtensionContentScript.js

@@ -119,6 +119,122 @@ add_task(function test_WebExtensionContentScript_mv3_specific() {
   });
 });
 
+add_task(async function test_WebExtensionContentScript_isUserScript() {
+  let policy = new WebExtensionPolicy({
+    id: "foo@bar.baz",
+    mozExtensionHostname: "ba159687-9472-4816-a1b2-8b14721d2ea6",
+    baseURL: "file:///foo/",
+    manifestVersion: 3,
+    allowedOrigins: new MatchPatternSet(["https://example.com/*"]),
+    localizeCallback() {},
+  });
+
+  const matches = new MatchPatternSet(["https://example.com/match/*"]);
+  const includeGlobs = [new MatchGlob("*/glob/*")];
+  const exampleMatchesURI = newURI("https://example.com/match/");
+  const exampleGlobURI = newURI("https://example.com/glob/");
+  const exampleNotMatchedURI = newURI("https://example.com/nomatch/");
+  const exampleNoPermissionURI = newURI("https://example.net/glob/");
+
+  let defaultScript = new WebExtensionContentScript(policy, {
+    matches,
+    includeGlobs,
+  });
+  let nonUserScript = new WebExtensionContentScript(policy, {
+    isUserScript: false,
+    matches,
+    includeGlobs,
+  });
+  let userScript = new WebExtensionContentScript(policy, {
+    isUserScript: true,
+    matches,
+    includeGlobs,
+  });
+
+  // Sanity checks: isUserScript flag is accurate.
+  equal(defaultScript.isUserScript, false, "isUserScript defaults to false");
+  equal(nonUserScript.isUserScript, false, "isUserScript set to false");
+  equal(userScript.isUserScript, true, "isUserScript set to true");
+
+  // Default, equivalent to isUserScript=false: matches AND includeGlobs.
+  ok(
+    !defaultScript.matchesURI(exampleMatchesURI),
+    "By default: ignore matches if includeGlobs does not match"
+  );
+  ok(
+    !defaultScript.matchesURI(exampleGlobURI),
+    "By default: ignore includeGlobs if matches does not match"
+  );
+
+  // With isUserScript=false explicitly: matches AND includeGlobs
+  ok(
+    !nonUserScript.matchesURI(exampleMatchesURI),
+    "Non-userScript: ignore matches if includeGlobs does not match"
+  );
+  ok(
+    !nonUserScript.matchesURI(exampleGlobURI),
+    "non-userScript: ignore includeGlobs if includeGlobs does not match"
+  );
+
+  // With isUserScript=true explicitly: matches OR includeGlobs
+  ok(
+    userScript.matchesURI(exampleMatchesURI),
+    "userScript: accept matches even if includeGlobs does not match"
+  );
+  ok(
+    userScript.matchesURI(exampleGlobURI),
+    "userScript: accept includeGlobs even if matches does not match"
+  );
+  ok(
+    !userScript.matchesURI(exampleNoPermissionURI),
+    "userScript: ignore includeGlobs if permission is missing"
+  );
+
+  // Now verify that empty matches is permitted.
+  let nonUserScriptEmptyMatches = new WebExtensionContentScript(policy, {
+    isUserScript: false,
+    matches: [],
+    includeGlobs,
+  });
+  let userScriptEmptyMatches = new WebExtensionContentScript(policy, {
+    isUserScript: true,
+    matches: [],
+    includeGlobs,
+  });
+  ok(
+    !nonUserScriptEmptyMatches.matchesURI(exampleGlobURI),
+    "non-userScript: ignore includeGlobs with empty matches"
+  );
+  ok(
+    userScriptEmptyMatches.matchesURI(exampleGlobURI),
+    "userScript: accept includeGlobs despite empty matches"
+  );
+  ok(
+    !userScriptEmptyMatches.matchesURI(exampleNotMatchedURI),
+    "userScript: ignore when not matched by includeGlobs (and empty matches)"
+  );
+  ok(
+    !userScriptEmptyMatches.matchesURI(exampleNoPermissionURI),
+    "userScript: ignore includeGlobs (and empty matches) without permission"
+  );
+
+  // Now verify that without includeGlobs, that matches works as usual.
+  // The isUserScript=false case is already extensively covered elsewhere, so
+  // we just do a sanity check for isUserScript=true.
+  let userScriptNoGlobs = new WebExtensionContentScript(policy, {
+    isUserScript: true,
+    matches,
+  });
+  ok(
+    userScriptNoGlobs.matchesURI(exampleMatchesURI),
+    "userScript: accept matches when includeGlobs is null"
+  );
+  ok(
+    !userScriptNoGlobs.matchesURI(exampleNotMatchedURI),
+    "userScript: ignore when not matched by matches and includeGlobs is null"
+  );
+});
+
 add_task(function test_WebExtensionContentScript_restricted() {
   let tests = [
     {

toolkit/components/extensions/test/xpcshell/test_ext_userScripts_mv3_injection.js

@@ -101,6 +101,18 @@ add_task(async function userScript_require_host_permissions() {
           runAt: "document_end",
           world: "MAIN",
         },
+        {
+          // For extra coverage use includeGlobs without matches, since most
+          // other tests do use "matches". The underlying userScripts-specific
+          // matches+includeGlobs matching logic is extensively covered by
+          // test_WebExtensionContentScript_isUserScript in
+          // test_WebExtensionContentScript.js.
+          id: "includeGlobs without matches",
+          includeGlobs: ["*resultCollector"],
+          js: [{ code: "resultCollector.push(origin)" }],
+          runAt: "document_end",
+          world: "MAIN",
+        },
       ]);
       browser.test.sendMessage("registered");
     },
@@ -115,7 +127,7 @@ add_task(async function userScript_require_host_permissions() {
     );
     Assert.deepEqual(
       await collectResults(contentPage),
-      ["http://example.net"],
+      ["http://example.net", "http://example.net"],
       "Can execute with host permissions"
     );
     await contentPage.close();

toolkit/components/extensions/test/xpcshell/test_ext_userScripts_mv3_persistence.js

@@ -294,6 +294,7 @@ add_task(async function register_and_update_all_values() {
 
   async function checkScriptsInContentProcess(previousJsPaths = null) {
     const expectedScriptsInContent = {
+      isUserScript: true,
       allFrames: true,
       matches: ["https://example.org/path/*"],
       excludeMatches: ["*://*/excludeme"],