Bug 1373293: Drop invalid permissions when normalizing manifests. r=bsilverberg

MozReview-Commit-ID: EIGhP6rRLzW

--HG--
extra : rebase_source : 28926b3ed4e65351c28ec7176ccdf4af2e5e0471
extra : amend_source : 7f2b1b990f9b75082be4aacb25593352dc5cca94

toolkit/components/extensions/Schemas.jsm

@@ -1179,7 +1179,12 @@ class ChoiceType extends Type {
     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);
   }
@@ -1719,7 +1724,7 @@ class ArrayType extends Type {
   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);
   }
@@ -1729,6 +1734,7 @@ class ArrayType extends Type {
     this.itemType = itemType;
     this.minItems = minItems;
     this.maxItems = maxItems;
+    this.onError = schema.items.onError || null;
   }
 
   normalize(value, context) {
@@ -1742,8 +1748,13 @@ class ArrayType extends Type {
     for (let [i, element] of value.entries()) {
       element = context.withPath(String(i), () => this.itemType.normalize(element, context));
       if (element.error) {
+        if (this.onError == "warn") {
+          context.logError(element.error);
+        } else if (this.onError != "ignore") {
           return element;
         }
+        continue;
+      }
       result.push(element.value);
     }
 

toolkit/components/extensions/schemas/manifest.json

@@ -168,13 +168,8 @@
             "type": "array",
             "default": [],
             "items": {
-              "choices": [
-                { "$ref": "Permission" },
-                {
-                  "type": "string",
-                  "deprecated": "Unknown permission ${value}"
-                }
-              ]
+              "$ref": "Permission",
+              "onError": "warn"
             },
             "optional": true
           },

toolkit/components/extensions/test/mochitest/test_ext_schema.html

@@ -50,7 +50,7 @@ add_task(async function testUnknownProperties() {
   });
 
   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/},
   ];
 

toolkit/components/extensions/test/xpcshell/test_ext_schemas.js

@@ -1091,7 +1091,7 @@ add_task(async function testChoices() {
   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/);

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();
+});

toolkit/components/extensions/test/xpcshell/xpcshell.ini

@@ -87,6 +87,7 @@ 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]