Bug 1338525 - Add schema validation for webextension themes r?mikedeboer draft
authorMatthew Wein <mwein@mozilla.com>
Tue, 14 Feb 2017 13:18:10 +0000
changeset 485466 eb8bc4c4e555e77bde22a012b308b02d00e23613
parent 483637 47391e531350873bfccd576b689259ec249aede8
child 546025 abe4386ccda6b0fa990858f46e3a8ee504ffc8bb
push id45740
push usermwein@mozilla.com
push dateThu, 16 Feb 2017 18:55:45 +0000
reviewersmikedeboer
bugs1338525
milestone54.0a1
Bug 1338525 - Add schema validation for webextension themes r?mikedeboer MozReview-Commit-ID: 3QjDrTeMKH0
browser/components/extensions/ext-theme.js
browser/components/extensions/schemas/theme.json
browser/components/extensions/test/browser/browser-common.ini
browser/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
browser/components/extensions/test/browser/browser_ext_themes_validation.js
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionParent.jsm
--- a/browser/components/extensions/ext-theme.js
+++ b/browser/components/extensions/ext-theme.js
@@ -138,20 +138,25 @@ extensions.on("shutdown", (type, extensi
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 extensions.registerSchemaAPI("theme", "addon_parent", context => {
   let {extension} = context;
   return {
     theme: {
       update(details) {
+        if (!Preferences.get("extensions.webextensions.themes.enabled")) {
+          // Return early if themes are disabled.
+          return;
+        }
+
         let theme = themeMap.get(extension);
 
-        // We won't have a theme if theme's aren't enabled.
         if (!theme) {
-          return;
+          // We need to initialize the theme the first time this is called.
+          themeMap.set(extension, new Theme(extension.baseURI));
         }
 
         theme.load(details);
       },
     },
   };
 });
--- a/browser/components/extensions/schemas/theme.json
+++ b/browser/components/extensions/schemas/theme.json
@@ -2,16 +2,25 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 [
   {
     "namespace": "manifest",
     "types": [
       {
+        "$extend": "Permission",
+        "choices": [{
+          "type": "string",
+          "enum": [
+            "theme"
+          ]
+        }]
+      },
+      {
         "id": "ThemeType",
         "type": "object",
         "properties": {
           "images": {
             "type": "object",
             "optional": true,
             "properties": {
               "headerURL": {
@@ -63,17 +72,17 @@
           }
         }
       }
     ]
   },
   {
     "namespace": "theme",
     "description": "The theme API allows customizing of visual elements of the browser.",
-    "permissions": ["manifest:theme"],
+    "permissions": ["theme"],
     "functions": [
       {
         "name": "update",
         "type": "function",
         "async": true,
         "description": "Make complete or partial updates to the theme. Resolves when the update has completed.",
         "parameters": [
           {
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -108,16 +108,17 @@ support-files =
 [browser_ext_tabs_sendMessage.js]
 [browser_ext_tabs_cookieStoreId.js]
 [browser_ext_tabs_update.js]
 [browser_ext_tabs_zoom.js]
 [browser_ext_tabs_update_url.js]
 [browser_ext_themes_chromeparity.js]
 [browser_ext_themes_dynamic_updates.js]
 [browser_ext_themes_lwtsupport.js]
+[browser_ext_themes_validation.js]
 [browser_ext_topwindowid.js]
 [browser_ext_url_overrides.js]
 [browser_ext_webRequest.js]
 [browser_ext_webNavigation_frameId0.js]
 [browser_ext_webNavigation_getFrames.js]
 [browser_ext_webNavigation_urlbar_transitions.js]
 [browser_ext_windows.js]
 [browser_ext_windows_create.js]
--- a/browser/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
+++ b/browser/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
@@ -35,40 +35,44 @@ add_task(function* setup() {
   yield SpecialPowers.pushPrefEnv({
     set: [["extensions.webextensions.themes.enabled", true]],
   });
 });
 
 add_task(function* test_dynamic_theme_updates() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
-      "theme": {
-        "images": {
-          "headerURL": BACKGROUND_1,
-        },
-        "colors": {
-          "accentcolor": ACCENT_COLOR_1,
-          "textcolor": TEXT_COLOR_1,
-        },
-      },
+      permissions: ["theme"],
     },
     background() {
       browser.test.onMessage.addListener((msg, details) => {
         if (msg != "update-theme") {
           browser.test.fail("expected 'update-theme' message");
         }
 
         browser.theme.update(details);
         browser.test.sendMessage("theme-updated");
       });
     },
   });
 
   yield extension.startup();
 
+  extension.sendMessage("update-theme", {
+    "images": {
+      "headerURL": BACKGROUND_1,
+    },
+    "colors": {
+      "accentcolor": ACCENT_COLOR_1,
+      "textcolor": TEXT_COLOR_1,
+    },
+  });
+
+  yield extension.awaitMessage("theme-updated");
+
   validateTheme(BACKGROUND_1, ACCENT_COLOR_1, TEXT_COLOR_1);
 
   extension.sendMessage("update-theme", {
     "images": {
       "headerURL": BACKGROUND_2,
     },
     "colors": {
       "accentcolor": ACCENT_COLOR_2,
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_themes_validation.js
@@ -0,0 +1,53 @@
+"use strict";
+
+add_task(function* setup() {
+  yield SpecialPowers.pushPrefEnv({
+    set: [["extensions.webextensions.themes.enabled", true]],
+  });
+});
+
+function* testTheme(invalidProps) {
+  let manifest = {
+    "theme": {},
+  };
+
+  invalidProps.forEach(prop => {
+    if (prop === "background") {
+      manifest[prop] = {
+        "scripts": ["background.js"],
+      };
+    } else if (prop === "permissions") {
+      manifest[prop] = ["tabs"];
+    } else if (prop === "omnibox") {
+      manifest[prop] = {
+        "keyword": "test",
+      };
+    } else {
+      manifest[prop] = {};
+    }
+  });
+
+  let extension = ExtensionTestUtils.loadExtension({manifest});
+
+  SimpleTest.waitForExplicitFinish();
+  let waitForConsole = new Promise(resolve => {
+    SimpleTest.monitorConsole(resolve, [{
+      message: new RegExp(`Reading manifest: Static theme contains invalid properties: ${invalidProps}`),
+    }]);
+  });
+
+  yield extension.startup();
+
+  SimpleTest.endMonitorConsole();
+  yield waitForConsole;
+
+  yield extension.unload();
+}
+
+add_task(function* test_theme_with_invalid_properties() {
+  let invalidProps = ["page_action", "browser_action", "background", "permissions", "omnibox", "commands"];
+  for (let prop in invalidProps) {
+    yield testTheme([prop]);
+  }
+  yield testTheme(invalidProps);
+});
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -398,16 +398,40 @@ this.ExtensionData = class {
     // a *.domain.com to specific-host.domain.com that's actually a
     // drop in permissions but the simple test below will cause a prompt.
     return {
       hosts: newPermissions.hosts.filter(perm => !oldPermissions.hosts.includes(perm)),
       permissions: newPermissions.permissions.filter(perm => !oldPermissions.permissions.includes(perm)),
     };
   }
 
+  /**
+   * Validates a theme to ensure it only contains static resources.
+   *
+   * @throws if theme is invalid.
+   * @param {object} context The extension's context.
+   */
+  validateTheme(context) {
+    let blacklist = ["background", "content_scripts", "permissions"];
+    let baseProperties = ExtensionParent.getBaseManifestProperties();
+    let whitelist = baseProperties.filter(prop => !blacklist.includes(prop));
+
+    let invalidProps = [];
+    for (let propName of Object.keys(this.manifest)) {
+      if (propName != "theme" && !whitelist.includes(propName)) {
+        invalidProps.push(propName);
+      }
+    }
+
+    if (invalidProps.length) {
+      let message = `Static theme contains invalid properties: ${invalidProps}`;
+      context.logError(message);
+    }
+  }
+
   // Reads the extension's |manifest.json| file, and stores its
   // parsed contents in |this.manifest|.
   readManifest() {
     return Promise.all([
       this.readJSON("manifest.json"),
       Management.lazyInit(),
     ]).then(([manifest]) => {
       this.manifest = manifest;
@@ -424,16 +448,20 @@ this.ExtensionData = class {
 
         logError: error => {
           this.logger.warn(`Loading extension '${this.id}': Reading manifest: ${error}`);
         },
 
         preprocessors: {},
       };
 
+      if ("theme" in this.manifest) {
+        this.validateTheme(context);
+      }
+
       if (this.localeData) {
         context.preprocessors.localize = (value, context) => this.localize(value);
       }
 
       let normalized = Schemas.normalize(this.manifest, "manifest.WebExtensionManifest", context);
       if (normalized.error) {
         this.manifestError(normalized.error);
       } else {
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -889,16 +889,29 @@ function watchExtensionProxyContextLoad(
 
   extension.on("extension-proxy-context-load", listener);
 
   return () => {
     extension.off("extension-proxy-context-load", listener);
   };
 }
 
+/**
+ * @returns {Array} the list of base properties defined in the base manifest.
+ */
+function getBaseManifestProperties() {
+  let types = Schemas.schemaJSON.get(BASE_SCHEMA)[0].types;
+  let manifest = types.find(type => type.id === "WebExtensionManifest");
+  if (!manifest) {
+    throw new Error("Unabled to find base manifest properties");
+  }
+  return Object.keys(manifest.properties);
+}
+
 const ExtensionParent = {
   GlobalManager,
   HiddenExtensionPage,
   ParentAPIManager,
   apiManager,
+  getBaseManifestProperties,
   promiseExtensionViewLoaded,
   watchExtensionProxyContextLoad,
 };