diff --git a/browser/components/extensions/ext-utils.js b/browser/components/extensions/ext-utils.js index 27b735a19bdd..b7626d8f26c9 100644 --- a/browser/components/extensions/ext-utils.js +++ b/browser/components/extensions/ext-utils.js @@ -6,6 +6,10 @@ XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils", "resource://gre/modules/PrivateBrowsingUtils.jsm"); Cu.import("resource://gre/modules/ExtensionUtils.jsm"); +Cu.import("resource://gre/modules/AddonManager.jsm"); + +const INTEGER = /^[1-9]\d*$/; + var { EventManager, } = ExtensionUtils; @@ -18,38 +22,48 @@ var { // Manages icon details for toolbar buttons in the |pageAction| and // |browserAction| APIs. global.IconDetails = { - // Accepted icon sizes. - SIZES: ["19", "38"], - // Normalizes the various acceptable input formats into an object - // with two properties, "19" and "38", containing icon URLs. + // with icon size as key and icon URL as value. + // + // If a context is specified (function is called from an extension): + // Throws an error if an invalid icon size was provided or the + // extension is not allowed to load the specified resources. + // + // If no context is specified, instead of throwing an error, this + // function simply logs a warning message. normalize(details, extension, context=null, localize=false) { let result = {}; - if (details.imageData) { - let imageData = details.imageData; + try { + if (details.imageData) { + let imageData = details.imageData; - if (imageData instanceof Cu.getGlobalForObject(imageData).ImageData) { - imageData = {"19": imageData}; - } + if (imageData instanceof Cu.getGlobalForObject(imageData).ImageData) { + imageData = {"19": imageData}; + } + + for (let size of Object.keys(imageData)) { + if (!INTEGER.test(size)) { + throw new Error(`Invalid icon size ${size}, must be an integer`); + } - for (let size of this.SIZES) { - if (size in imageData) { result[size] = this.convertImageDataToPNG(imageData[size], context); } } - } - if (details.path) { - let path = details.path; - if (typeof path != "object") { - path = {"19": path}; - } + if (details.path) { + let path = details.path; + if (typeof path != "object") { + path = {"19": path}; + } - let baseURI = context ? context.uri : extension.baseURI; + let baseURI = context ? context.uri : extension.baseURI; + + for (let size of Object.keys(path)) { + if (!INTEGER.test(size)) { + throw new Error(`Invalid icon size ${size}, must be an integer`); + } - for (let size of this.SIZES) { - if (size in path) { let url = path[size]; if (localize) { url = extension.localize(url); @@ -60,25 +74,23 @@ global.IconDetails = { // The Chrome documentation specifies these parameters as // relative paths. We currently accept absolute URLs as well, // which means we need to check that the extension is allowed - // to load them. - try { - Services.scriptSecurityManager.checkLoadURIStrWithPrincipal( - extension.principal, url, - Services.scriptSecurityManager.DISALLOW_SCRIPT); - } catch (e) { - if (context) { - throw e; - } - // If there's no context, it's because we're handling this - // as a manifest directive. Log a warning rather than - // raising an error, but don't accept the URL in any case. - extension.manifestError(`Access to URL '${url}' denied`); - continue; - } + // to load them. This will throw an error if it's not allowed. + Services.scriptSecurityManager.checkLoadURIStrWithPrincipal( + extension.principal, url, + Services.scriptSecurityManager.DISALLOW_SCRIPT); result[size] = url; } } + } catch (e) { + // Function is called from extension code, delegate error. + if (context) { + throw e; + } + // If there's no context, it's because we're handling this + // as a manifest directive. Log a warning rather than + // raising an error. + extension.manifestError(`Invalid icon data: ${e}`); } return result; @@ -89,12 +101,7 @@ global.IconDetails = { getURL(icons, window, extension) { const DEFAULT = "chrome://browser/content/extension.svg"; - // Use the higher resolution image if we're doing any up-scaling - // for high resolution monitors. - let res = window.devicePixelRatio; - let size = res > 1 ? "38" : "19"; - - return icons[size] || icons["19"] || icons["38"] || DEFAULT; + return AddonManager.getPreferredIconURL({icons: icons}, 18, window) || DEFAULT; }, convertImageDataToPNG(imageData, context) { diff --git a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js index 1655f3ea9209..c41b34a6dc62 100644 --- a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js +++ b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js @@ -101,6 +101,29 @@ add_task(function* testDetailsObjects() { resolutions: { "1": imageData.red.url, "2": browser.runtime.getURL("data/a.png"), } }, + + // Various resolutions + { details: { "path": { "18": "a.png", "32": "a-x2.png" } }, + resolutions: { + "1": browser.runtime.getURL("data/a.png"), + "2": browser.runtime.getURL("data/a-x2.png"), } }, + { details: { "path": { "16": "16.png", "100": "100.png" } }, + resolutions: { + "1": browser.runtime.getURL("data/100.png"), + "2": browser.runtime.getURL("data/100.png"), } }, + { details: { "path": { "2": "2.png"} }, + resolutions: { + "1": browser.runtime.getURL("data/2.png"), + "2": browser.runtime.getURL("data/2.png"), } }, + { details: { "path": { + "6": "6.png", + "18": "18.png", + "32": "32.png", + "48": "48.png", + "128": "128.png" } }, + resolutions: { + "1": browser.runtime.getURL("data/18.png"), + "2": browser.runtime.getURL("data/48.png"), } }, ]; // Allow serializing ImageData objects for logging. @@ -197,6 +220,62 @@ add_task(function* testDetailsObjects() { yield extension.unload(); }); +// Test that an error is thrown when providing invalid icon sizes +add_task(function *testInvalidIconSizes() { + + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + "browser_action": {}, + "page_action": {}, + }, + + background: function () { + browser.tabs.query({ active: true, currentWindow: true }, tabs => { + var tabId = tabs[0].id; + + for (var api of ["pageAction", "browserAction"]) { + // helper function to run setIcon and check if it fails + let assertSetIconThrows = function(detail, error, message) { + try { + detail.tabId = tabId; + browser[api].setIcon(detail); + + browser.test.fail("Expected an error on invalid icon size."); + browser.test.notifyFail("setIcon with invalid icon size"); + return; + } catch (e) { + browser.test.succeed("setIcon with invalid icon size"); + } + } + + // test invalid icon size inputs + for (var type of ["path", "imageData"]) { + assertSetIconThrows({ [type]: { "abcdef": "test.png" } }); + assertSetIconThrows({ [type]: { "48px": "test.png" } }); + assertSetIconThrows({ [type]: { "20.5": "test.png" } }); + assertSetIconThrows({ [type]: { "5.0": "test.png" } }); + assertSetIconThrows({ [type]: { "-300": "test.png" } }); + assertSetIconThrows({ [type]: { + "abc": "test.png", + "5": "test.png" + }}); + } + + assertSetIconThrows({ imageData: { "abcdef": "test.png" }, path: {"5": "test.png"} }); + assertSetIconThrows({ path: { "abcdef": "test.png" }, imageData: {"5": "test.png"} }); + } + + browser.test.notifyPass("setIcon with invalid icon size"); + }); + } + }); + + yield Promise.all([extension.startup(), extension.awaitFinish("setIcon with invalid icon size")]); + + yield extension.unload(); +}); + + // Test that default icon details in the manifest.json file are handled // correctly. add_task(function *testDefaultDetails() { @@ -294,7 +373,7 @@ add_task(function* testSecureURLsDenied() { yield extension.startup(); - yield extension.awaitFinish(); + yield extension.awaitFinish("setIcon security tests"); yield extension.unload(); @@ -304,12 +383,12 @@ add_task(function* testSecureURLsDenied() { "javascript:true"]; let matchURLForbidden = url => ({ - message: new RegExp(`Loading extension.*Access to.*'${url}' denied`), + message: new RegExp(`Loading extension.*Invalid icon data: NS_ERROR_DOM_BAD_URI`), }); + // Because the underlying method throws an error on invalid data, + // only the first invalid URL of each component will be logged. let messages = [matchURLForbidden(urls[0]), - matchURLForbidden(urls[1]), - matchURLForbidden(urls[0]), matchURLForbidden(urls[1])]; let waitForConsole = new Promise(resolve => { @@ -330,8 +409,8 @@ add_task(function* testSecureURLsDenied() { }, "page_action": { "default_icon": { - "19": urls[0], - "38": urls[1], + "19": urls[1], + "38": urls[0], }, }, }, diff --git a/toolkit/mozapps/extensions/AddonManager.jsm b/toolkit/mozapps/extensions/AddonManager.jsm index 4a294c7f3485..e40050f3e3d4 100644 --- a/toolkit/mozapps/extensions/AddonManager.jsm +++ b/toolkit/mozapps/extensions/AddonManager.jsm @@ -84,6 +84,7 @@ XPCOMUtils.defineLazyGetter(this, "CertUtils", function certUtilsLazyGetter() { return certUtils; }); +const INTEGER = /^[1-9]\d*$/; this.EXPORTED_SYMBOLS = [ "AddonManager", "AddonManagerPrivate" ]; @@ -2346,7 +2347,10 @@ var AddonManagerInternal = { * match an icon of size 96px. * * @param aAddon - * The addon to find an icon for + * An addon object, meaning: + * An object with either an icons property that is a key-value + * list of icon size and icon URL, or an object having an iconURL + * and icon64URL property. * @param aSize * Ideal icon size in pixels * @param aWindow @@ -2380,12 +2384,13 @@ var AddonManagerInternal = { let bestSize = null; for (let size of Object.keys(icons)) { - size = parseInt(size, 10); - if (isNaN(size)) { + if (!INTEGER.test(size)) { throw Components.Exception("Invalid icon size, must be an integer", Cr.NS_ERROR_ILLEGAL_VALUE); } + size = parseInt(size, 10); + if (!bestSize) { bestSize = size; continue; diff --git a/toolkit/mozapps/extensions/internal/XPIProvider.jsm b/toolkit/mozapps/extensions/internal/XPIProvider.jsm index 3c0fbe9268a4..27f7ddd8b338 100644 --- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm +++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm @@ -241,6 +241,7 @@ function mustSign(aType) { return REQUIRE_SIGNING || Preferences.get(PREF_XPI_SIGNATURES_REQUIRED, false); } +const INTEGER = /^[1-9]\d*$/; // Keep track of where we are in startup for telemetry // event happened during XPIDatabase.startup() @@ -912,8 +913,8 @@ var loadManifestFromWebManifest = Task.async(function* loadManifestFromWebManife if (icons) { // filter out invalid (non-integer) size keys Object.keys(icons) + .filter((size) => INTEGER.test(size)) .map((size) => parseInt(size, 10)) - .filter((size) => !isNaN(size)) .forEach((size) => addon.icons[size] = icons[size]); } diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js index b36e6576ce19..6f113b6c8590 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js @@ -87,6 +87,9 @@ add_task(function*() { icons: { 32: "icon32.png", banana: "bananana.png", + "20.5": "icon20.5.png", + "20.0": "also invalid", + "123banana": "123banana.png", 64: "icon64.png" } }, profileDir);