Bug 1637431 - Gives meaningful tabs.query error message. r=robwu,geck…

…oview-reviewers,agi *** Bug 1637431 - Gives meaningful tabs.query error message. r=robwu Differential Revision: https://phabricator.services.mozilla.com/D78466
Brli · Jun 16, 2020 · 1d8e13f · 1d8e13f
1 parent fa3acc4
commit 1d8e13f
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 24 deletions.
diff --git a/browser/components/extensions/parent/ext-tabs.js b/browser/components/extensions/parent/ext-tabs.js
@@ -933,18 +933,6 @@ this.tabs = class extends ExtensionAPI {
               });
             }
           }
-
-          queryInfo = Object.assign({}, queryInfo);
-
-          if (queryInfo.url !== null) {
-            queryInfo.url = new MatchPatternSet([].concat(queryInfo.url), {
-              restrictSchemes: false,
-            });
-          }
-          if (queryInfo.title !== null) {
-            queryInfo.title = new MatchGlob(queryInfo.title);
-          }
-
           return Array.from(tabManager.query(queryInfo, context), tab =>
             tab.convert()
           );

diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_query.js b/browser/components/extensions/test/browser/browser_ext_tabs_query.js
@@ -384,6 +384,35 @@ add_task(async function testQueryWithoutURLOrTitlePermissions() {
   await extension.unload();
 });
 
+add_task(async function testInvalidUrl() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["tabs"],
+    },
+    async background() {
+      await browser.test.assertRejects(
+        browser.tabs.query({ url: "http://test1.net" }),
+        "Invalid url pattern: http://test1.net",
+        "Expected url to match pattern"
+      );
+      await browser.test.assertRejects(
+        browser.tabs.query({ url: ["test2"] }),
+        "Invalid url pattern: test2",
+        "Expected an array with an invalid match pattern"
+      );
+      await browser.test.assertRejects(
+        browser.tabs.query({ url: ["http://www.bbc.com/", "test3"] }),
+        "Invalid url pattern: test3",
+        "Expected an array with an invalid match pattern"
+      );
+      browser.test.notifyPass("testInvalidUrl");
+    },
+  });
+  await extension.startup();
+  await extension.awaitFinish("testInvalidUrl");
+  await extension.unload();
+});
+
 add_task(async function test_query_index() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {

diff --git a/mobile/android/components/extensions/ext-tabs.js b/mobile/android/components/extensions/ext-tabs.js
@@ -463,18 +463,6 @@ this.tabs = class extends ExtensionAPI {
               });
             }
           }
-
-          queryInfo = Object.assign({}, queryInfo);
-
-          if (queryInfo.url !== null) {
-            queryInfo.url = new MatchPatternSet([].concat(queryInfo.url), {
-              restrictSchemes: false,
-            });
-          }
-          if (queryInfo.title !== null) {
-            queryInfo.title = new MatchGlob(queryInfo.title);
-          }
-
           return Array.from(tabManager.query(queryInfo, context), tab =>
             tab.convert()
           );

diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -287,6 +287,22 @@ function getMessageManager(target) {
 function flushJarCache(jarPath) {
   Services.obs.notifyObservers(null, "flush-cache-entry", jarPath);
 }
+function parseMatchPatterns(patterns, options) {
+  try {
+    return new MatchPatternSet(patterns, options);
+  } catch (e) {
+    let pattern;
+    for (pattern of patterns) {
+      try {
+        new MatchPattern(pattern, options);
+      } catch (e) {
+        throw new ExtensionError(`Invalid url pattern: ${pattern}`);
+      }
+    }
+    // Unexpectedly MatchPatternSet threw, but MatchPattern did not.
+    throw e;
+  }
+}
 
 var ExtensionUtils = {
   flushJarCache,
@@ -295,6 +311,7 @@ var ExtensionUtils = {
   getUniqueId,
   filterStack,
   getWinUtils,
+  parseMatchPatterns,
   promiseDocumentIdle,
   promiseDocumentLoaded,
   promiseDocumentReady,

diff --git a/toolkit/components/extensions/parent/ext-tabs-base.js b/toolkit/components/extensions/parent/ext-tabs-base.js
@@ -28,6 +28,7 @@ var {
   DefaultWeakMap,
   ExtensionError,
   getWinUtils,
+  parseMatchPatterns,
 } = ExtensionUtils;
 
 var { defineLazyGetter } = ExtensionCommon;
@@ -1962,6 +1963,21 @@ class TabManagerBase {
    * @returns {Iterator<TabBase>}
    */
   *query(queryInfo = null, context = null) {
+    if (queryInfo) {
+      if (queryInfo.url !== null) {
+        queryInfo.url = parseMatchPatterns([].concat(queryInfo.url), {
+          restrictSchemes: false,
+        });
+      }
+
+      if (queryInfo.title !== null) {
+        try {
+          queryInfo.title = new MatchGlob(queryInfo.title);
+        } catch (e) {
+          throw new ExtensionError(`Invalid title: ${queryInfo.title}`);
+        }
+      }
+    }
     function* candidates(windowWrapper) {
       if (queryInfo) {
         let { active, highlighted, index } = queryInfo;