Bug 1373293 - Drop invalid permissions when normalizing manifests. r=bsilverberg, a=jcristau
authorKris Maglione <maglione.k@gmail.com>
Fri, 23 Jun 2017 10:55:48 -0700
changeset 411722 b435b2f239458d25ce54caa2fd4459491faf6c70
parent 411721 8987f5c030175b4a1f208672ffbc462c21f206cf
child 411723 8e494ed0894087d24f2f68104d4eadad8c4a42a4
push id7445
push userryanvm@gmail.com
push dateFri, 23 Jun 2017 18:41:40 +0000
treeherdermozilla-beta@8e494ed08940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsilverberg, jcristau
bugs1373293
milestone55.0
Bug 1373293 - Drop invalid permissions when normalizing manifests. r=bsilverberg, a=jcristau MozReview-Commit-ID: EIGhP6rRLzW
browser/components/extensions/test/xpcshell/test_ext_manifest_commands.js
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/schemas/manifest.json
toolkit/components/extensions/test/mochitest/test_ext_schema.html
toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
toolkit/components/extensions/test/xpcshell/test_ext_unknown_permissions.js
toolkit/components/extensions/test/xpcshell/xpcshell.ini
--- a/browser/components/extensions/test/xpcshell/test_ext_manifest_commands.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_manifest_commands.js
@@ -9,17 +9,17 @@ add_task(async function test_manifest_co
       "toggle-feature": {
         "suggested_key": {"default": "Shifty+Y"},
         "description": "Send a 'toggle-feature' event to the extension",
       },
     },
   });
 
   let expectedError = (
-    String.raw`commands.toggle-feature.suggested_key.default: Value must either: ` +
+    String.raw`commands.toggle-feature.suggested_key.default: Value "Shifty+Y" must either: ` +
     String.raw`match the pattern /^\s*(Alt|Ctrl|Command|MacCtrl)\s*\+\s*(Shift\s*\+\s*)?([A-Z0-9]|Comma|Period|Home|End|PageUp|PageDown|Space|Insert|Delete|Up|Down|Left|Right)\s*$/, ` +
     String.raw`match the pattern /^\s*((Alt|Ctrl|Command|MacCtrl)\s*\+\s*)?(Shift\s*\+\s*)?(F[1-9]|F1[0-2])\s*$/, or ` +
     String.raw`match the pattern /^(MediaNextTrack|MediaPlayPause|MediaPrevTrack|MediaStop)$/`
   );
 
   ok(normalized.error.includes(expectedError),
      `The manifest error ${JSON.stringify(normalized.error)} must contain ${JSON.stringify(expectedError)}`);
 });
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -1238,17 +1238,17 @@ let IconDetails = {
   escapeUrl(url) {
     return url.replace(/[\\\s"]/g, encodeURIComponent);
   },
 };
 
 let StartupCache = {
   DB_NAME: "ExtensionStartupCache",
 
-  SCHEMA_VERSION: 3,
+  SCHEMA_VERSION: 4,
 
   STORE_NAMES: Object.freeze(["locales", "manifests", "schemas"]),
 
   dbPromise: null,
 
   initDB(db) {
     for (let name of StartupCache.STORE_NAMES) {
       try {
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -1174,17 +1174,22 @@ class ChoiceType extends Type {
     }
     if (choices.length <= 1) {
       return error;
     }
 
     let n = choices.length - 1;
     choices[n] = `or ${choices[n]}`;
 
-    let message = `Value must either: ${choices.join(", ")}`;
+    let message;
+    if (typeof value === "object") {
+      message = `Value must either: ${choices.join(", ")}`;
+    } else {
+      message = `Value ${JSON.stringify(value)} must either: ${choices.join(", ")}`;
+    }
 
     return context.error(message, null);
   }
 
   checkBaseType(baseType) {
     return this.choices.some(t => t.checkBaseType(baseType));
   }
 }
@@ -1705,40 +1710,46 @@ class BooleanType extends Type {
 class ArrayType extends Type {
   static get EXTRA_PROPERTIES() {
     return ["items", "minItems", "maxItems", ...super.EXTRA_PROPERTIES];
   }
 
   static parseSchema(schema, path, extraProperties = []) {
     this.checkSchemaProperties(schema, path, extraProperties);
 
-    let items = Schemas.parseSchema(schema.items, path);
+    let items = Schemas.parseSchema(schema.items, path, ["onError"]);
 
     return new this(schema, items, schema.minItems || 0, schema.maxItems || Infinity);
   }
 
   constructor(schema, itemType, minItems, maxItems) {
     super(schema);
     this.itemType = itemType;
     this.minItems = minItems;
     this.maxItems = maxItems;
+    this.onError = schema.items.onError || null;
   }
 
   normalize(value, context) {
     let v = this.normalizeBase("array", value, context);
     if (v.error) {
       return v;
     }
     value = v.value;
 
     let result = [];
     for (let [i, element] of value.entries()) {
       element = context.withPath(String(i), () => this.itemType.normalize(element, context));
       if (element.error) {
-        return element;
+        if (this.onError == "warn") {
+          context.logError(element.error);
+        } else if (this.onError != "ignore") {
+          return element;
+        }
+        continue;
       }
       result.push(element.value);
     }
 
     if (result.length < this.minItems) {
       return context.error(`Array requires at least ${this.minItems} items; you have ${result.length}`,
                            `have at least ${this.minItems} items`);
     }
--- a/toolkit/components/extensions/schemas/manifest.json
+++ b/toolkit/components/extensions/schemas/manifest.json
@@ -163,23 +163,18 @@
             "format": "contentSecurityPolicy",
             "onError": "warn"
           },
 
           "permissions": {
             "type": "array",
             "default": [],
             "items": {
-              "choices": [
-                { "$ref": "Permission" },
-                {
-                  "type": "string",
-                  "deprecated": "Unknown permission ${value}"
-                }
-              ]
+              "$ref": "Permission",
+              "onError": "warn"
             },
             "optional": true
           },
 
           "optional_permissions": {
             "type": "array",
             "items": {
               "choices": [
--- a/toolkit/components/extensions/test/mochitest/test_ext_schema.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_schema.html
@@ -45,17 +45,17 @@ add_task(async function testUnknownPrope
 
       unknown_property: {},
     },
 
     background,
   });
 
   let messages = [
-    {message: /processing permissions\.0: Unknown permission "unknownPermission"/},
+    {message: /processing permissions\.0: Value "unknownPermission"/},
     {message: /processing unknown_property: An unexpected property was found in the WebExtension manifest/},
   ];
 
   let waitForConsole = new Promise(resolve => {
     SimpleTest.monitorConsole(resolve, messages);
   });
 
   await extension.startup();
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
@@ -1086,17 +1086,17 @@ add_task(async function testChoices() {
   await Schemas.load(url);
 
   let root = {};
   Schemas.inject(root, wrapper);
 
   talliedErrors.length = 0;
 
   Assert.throws(() => root.choices.meh("frog"),
-                /Value must either: be one of \["foo", "bar", "baz"\], match the pattern \/florg\.\*meh\/, or be an integer value/);
+                /Value "frog" must either: be one of \["foo", "bar", "baz"\], match the pattern \/florg\.\*meh\/, or be an integer value/);
 
   Assert.throws(() => root.choices.meh(4),
                 /be a string value, or be at least 12/);
 
   Assert.throws(() => root.choices.meh(43),
                 /be a string value, or be no greater than 42/);
 
 
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_unknown_permissions.js
@@ -0,0 +1,30 @@
+"use strict";
+
+add_task(async function test_unknown_permissions() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: [
+        "activeTab",
+        "fooUnknownPermission",
+        "http://*/",
+        "chrome://favicon/",
+      ],
+    },
+  });
+
+  let {messages} = await promiseConsoleOutput(
+    () => extension.startup());
+
+  const {WebExtensionPolicy} = Cu.import("resource://gre/modules/Extension.jsm", {});
+
+  let policy = WebExtensionPolicy.getByID(extension.id);
+  Assert.deepEqual(Array.from(policy.permissions).sort(), ["activeTab", "http://*/*"]);
+
+  ok(messages.some(message => /Error processing permissions\.1: Value "fooUnknownPermission" must/.test(message)),
+     'Got expected error for "fooUnknownPermission"');
+
+  ok(messages.some(message => /Error processing permissions\.3: Value "chrome:\/\/favicon\/" must/.test(message)),
+     'Got expected error for "chrome://favicon/"');
+
+  await extension.unload();
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell.ini
@@ -81,16 +81,17 @@ skip-if = os == "android" # Bug 1350559
 [test_ext_storage_sync.js]
 head = head.js head_sync.js
 skip-if = os == "android"
 [test_ext_storage_sync_crypto.js]
 skip-if = os == "android"
 [test_ext_themes_supported_properties.js]
 [test_ext_topSites.js]
 skip-if = os == "android"
+[test_ext_unknown_permissions.js]
 [test_ext_legacy_extension_context.js]
 [test_ext_legacy_extension_embedding.js]
 [test_locale_converter.js]
 [test_locale_data.js]
 [test_native_messaging.js]
 skip-if = os == "android"
 [test_proxy_scripts.js]
 [include:xpcshell-content.ini]