Bug 1338525 - Add schema validation for webextension themes r=mikedeboer,mossop
authorMatthew Wein <mwein@mozilla.com>
Fri, 17 Mar 2017 18:17:14 -0400
changeset 348327 a8caa66acbfc9a397b3ef9161145b1a558a3942a
parent 348326 874293ae98ea8b2f8582be0d03a52557a11baabb
child 348328 0232885afcb2f016a005e7db6ff5d25b4b6ef765
push id88187
push userarchaeopteryx@coole-files.de
push dateSat, 18 Mar 2017 15:27:00 +0000
treeherdermozilla-inbound@0b1d3324cffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer, mossop
bugs1338525
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1338525 - Add schema validation for webextension themes r=mikedeboer,mossop 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,