Bug 1338525 - Add schema validation for webextension themes r?mikedeboer,kmag draft
authorMatthew Wein <mwein@mozilla.com>
Fri, 17 Mar 2017 18:17:14 -0400
changeset 500912 71b4e0852e305de4bb85271c188bf3fcb31058b0
parent 500108 3945f2297b997d59662bdb8ac20e092363eea532
child 500913 9826232151e97d21f6a26130a3dae9851344392d
push id49840
push usermwein@mozilla.com
push dateFri, 17 Mar 2017 22:20:39 +0000
reviewersmikedeboer, kmag
bugs1338525
milestone55.0a1
Bug 1338525 - Add schema validation for webextension themes r?mikedeboer,kmag MozReview-Commit-ID: 3QjDrTeMKH0
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/ext-theme.js
toolkit/components/extensions/schemas/theme.json
toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -80,16 +80,17 @@ var {
   apiManager: Management,
 } = ExtensionParent;
 
 const {
   EventEmitter,
   LocaleData,
   StartupCache,
   getUniqueId,
+  validateThemeManifest,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyGetter(this, "console", ExtensionUtils.getConsole);
 
 const LOGGER_ID_BASE = "addons.webextension.";
 const UUID_MAP_PREF = "extensions.webextensions.uuids";
 const LEAVE_STORAGE_PREF = "extensions.webextensions.keepStorageOnUninstall";
 const LEAVE_UUID_PREF = "extensions.webextensions.keepUuidOnUninstall";
@@ -424,16 +425,27 @@ this.ExtensionData = class {
 
         logError: error => {
           this.logger.warn(`Loading extension '${this.id}': Reading manifest: ${error}`);
         },
 
         preprocessors: {},
       };
 
+      if (this.manifest.theme) {
+        let invalidProps = validateThemeManifest(Object.getOwnPropertyNames(this.manifest));
+
+        if (invalidProps.length) {
+          let message = `Themes defined in the manifest may only contain static resources. ` +
+            `If you would like to use additional properties, please use the "theme" permission instead. ` +
+            `(the invalid properties found are: ${invalidProps})`;
+          this.manifestError(message);
+        }
+      }
+
       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
@@ -856,16 +856,33 @@ function watchExtensionProxyContextLoad(
 
   extension.on("extension-proxy-context-load", listener);
 
   return () => {
     extension.off("extension-proxy-context-load", listener);
   };
 }
 
+// Used to cache the list of WebExtensionManifest properties defined in the BASE_SCHEMA.
+let gBaseManifestProperties = null;
+
 const ExtensionParent = {
   GlobalManager,
   HiddenExtensionPage,
   ParentAPIManager,
   apiManager,
+  get baseManifestProperties() {
+    if (gBaseManifestProperties) {
+      return gBaseManifestProperties;
+    }
+
+    let types = Schemas.schemaJSON.get(BASE_SCHEMA)[0].types;
+    let manifest = types.find(type => type.id === "WebExtensionManifest");
+    if (!manifest) {
+      throw new Error("Unable to find base manifest properties");
+    }
+
+    gBaseManifestProperties = Object.getOwnPropertyNames(manifest.properties);
+    return gBaseManifestProperties;
+  },
   promiseExtensionViewLoaded,
   watchExtensionProxyContextLoad,
 };
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -56,16 +56,49 @@ XPCOMUtils.defineLazyGetter(this, "conso
 
 let nextId = 0;
 XPCOMUtils.defineLazyGetter(this, "uniqueProcessID", () => Services.appinfo.uniqueProcessID);
 
 function getUniqueId() {
   return `${nextId++}-${uniqueProcessID}`;
 }
 
+// The list of properties that themes are allowed to contain.
+XPCOMUtils.defineLazyGetter(this, "gAllowedThemeProperties", () => {
+  Cu.import("resource://gre/modules/ExtensionParent.jsm");
+  let propertiesInBaseManifest = ExtensionParent.baseManifestProperties;
+
+  // The properties found in the base manifest contain all of the properties that
+  // themes are allowed to have. However, the list also contains several properties
+  // that aren't allowed, so we need to filter them out first before the list can
+  // be used to validate themes.
+  return propertiesInBaseManifest.filter(prop => {
+    const propertiesToRemove = ["background", "content_scripts", "permissions"];
+    return !propertiesToRemove.includes(prop);
+  });
+});
+
+/**
+ * Validates a theme to ensure it only contains static resources.
+ *
+ * @param {Array<string>} manifestProperties The list of top-level keys found in the
+ *    the extension's manifest.
+ * @returns {Array<string>} A list of invalid properties or an empty list
+ *    if none are found.
+ */
+function validateThemeManifest(manifestProperties) {
+  let invalidProps = [];
+  for (let propName of manifestProperties) {
+    if (propName != "theme" && !gAllowedThemeProperties.includes(propName)) {
+      invalidProps.push(propName);
+    }
+  }
+  return invalidProps;
+}
+
 let StartupCache = {
   DB_NAME: "ExtensionStartupCache",
 
   SCHEMA_VERSION: 1,
 
   STORE_NAMES: Object.freeze(["locales", "manifests", "schemas"]),
 
   dbPromise: null,
@@ -1309,16 +1342,17 @@ this.ExtensionUtils = {
   promiseDocumentReady,
   promiseEvent,
   promiseObserved,
   runSafe,
   runSafeSync,
   runSafeSyncWithoutClone,
   runSafeWithoutClone,
   stylesheetMap,
+  validateThemeManifest,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
   ExtensionError,
   IconDetails,
   LimitedSet,
   LocaleData,
   MessageManagerProxy,
--- a/toolkit/components/extensions/ext-theme.js
+++ b/toolkit/components/extensions/ext-theme.js
@@ -2,16 +2,20 @@
 
 Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
                                   "resource://gre/modules/Preferences.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager",
                                   "resource://gre/modules/LightweightThemeManager.jsm");
 
+XPCOMUtils.defineLazyGetter(this, "gThemesEnabled", () => {
+  return Preferences.get("extensions.webextensions.themes.enabled");
+});
+
 // WeakMap[Extension -> Theme]
 let themeMap = new WeakMap();
 
 const ICONS = Preferences.get("extensions.webextensions.themes.icons.buttons", "").split(",");
 
 /** Class representing a theme. */
 class Theme {
   /**
@@ -149,17 +153,17 @@ class Theme {
     Services.obs.notifyObservers(null,
       "lightweight-theme-styling-update",
       JSON.stringify(lwtStyles));
   }
 }
 
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("manifest_theme", (type, directive, extension, manifest) => {
-  if (!Preferences.get("extensions.webextensions.themes.enabled")) {
+  if (!gThemesEnabled) {
     // Return early if themes are disabled.
     return;
   }
 
   let theme = new Theme(extension.baseURI);
   theme.load(manifest.theme);
   themeMap.set(extension, theme);
 });
@@ -176,20 +180,28 @@ extensions.on("shutdown", (type, extensi
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 extensions.registerSchemaAPI("theme", "addon_parent", context => {
   let {extension} = context;
   return {
     theme: {
       update(details) {
+        if (!gThemesEnabled) {
+          // 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;
+          // Themes which use `update` will not a theme defined
+          // in the manifest. Therefore, we need to initialize the
+          // theme the first time `update` is called.
+          theme = new Theme(extension.baseURI);
+          themeMap.set(extension, theme);
         }
 
         theme.load(details);
       },
     },
   };
 });
--- a/toolkit/components/extensions/schemas/theme.json
+++ b/toolkit/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": {
@@ -197,17 +206,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/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
+++ b/toolkit/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,