Bug 1373293: Drop invalid permissions when normalizing manifests. r=bsilverberg
authorKris Maglione <maglione.k@gmail.com>
Wed, 21 Jun 2017 12:12:51 -0700
changeset 365427 b04ed6a0a52af869867351bc0a10e86033e718e8
parent 365426 1fb9b344f56d15211d996900d806ef70d7eb40d2
child 365428 fb2f8f1146634034024fd39b83ddaff54606de14
push id32072
push usercbook@mozilla.com
push dateThu, 22 Jun 2017 10:47:53 +0000
treeherdermozilla-central@13e37d5702f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsilverberg
bugs1373293
milestone56.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 1373293: Drop invalid permissions when normalizing manifests. r=bsilverberg MozReview-Commit-ID: EIGhP6rRLzW
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/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));
   }
 }
@@ -1714,40 +1719,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(policy.permissions, ["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
@@ -82,16 +82,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]