From 09639b067e5df114868a66474e5f1880aee9a51c Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Thu, 6 Feb 2025 14:16:44 +0000 Subject: [PATCH] Bug 1939087 - Truncate long name and log warning a=dmeehan Original Revision: https://phabricator.services.mozilla.com/D233025 Differential Revision: https://phabricator.services.mozilla.com/D236900 --- .../components/extensions/Extension.sys.mjs | 19 ++++++++++++++++ .../extensions/schemas/manifest.json | 1 + .../test/xpcshell/test_ext_manifest.js | 22 +++++++++++++++++++ .../extensions/internal/XPIInstall.sys.mjs | 5 +++++ .../extensions/test/xpcshell/test_locale.js | 22 +++++++++++++++++++ 5 files changed, 69 insertions(+) diff --git a/toolkit/components/extensions/Extension.sys.mjs b/toolkit/components/extensions/Extension.sys.mjs index 8252b8dfdf73..47a6d1a3e1de 100644 --- a/toolkit/components/extensions/Extension.sys.mjs +++ b/toolkit/components/extensions/Extension.sys.mjs @@ -1585,6 +1585,17 @@ export class ExtensionData { ); } + // AMO enforces a maximum length of 45 on the name since at least 2017, via + // https://github.com/mozilla/addons-linter/blame/c4507688899aaafe29c522f1b1aec94b78b8a095/src/schema/updates/manifest.json#L111 + // added in https://github.com/mozilla/addons-linter/pull/1169 + // To avoid breaking add-ons that do not go through AMO (e.g. temporarily + // loaded extensions), we enforce the limit by truncating and warning if + // needed, instead enforcing a maxLength on "name" in schemas/manifest.json. + // + // We set the limit to 75, which is a safe limit that matches the CWS, + // see https://bugzilla.mozilla.org/show_bug.cgi?id=1939087#c5 + static EXT_NAME_MAX_LEN = 75; + async initializeAddonTypeAndID() { if (this.type) { // Already initialized. @@ -1714,6 +1725,14 @@ export class ExtensionData { } } + if (manifest.name.length > ExtensionData.EXT_NAME_MAX_LEN) { + // Truncate and warn - see comment in EXT_NAME_MAX_LEN. + manifest.name = manifest.name.slice(0, ExtensionData.EXT_NAME_MAX_LEN); + this.manifestWarning( + `Warning processing "name": must be shorter than ${ExtensionData.EXT_NAME_MAX_LEN}` + ); + } + if ( this.manifestVersion < 3 && manifest.background && diff --git a/toolkit/components/extensions/schemas/manifest.json b/toolkit/components/extensions/schemas/manifest.json index e14d7d0591a0..293e5d255c62 100644 --- a/toolkit/components/extensions/schemas/manifest.json +++ b/toolkit/components/extensions/schemas/manifest.json @@ -29,6 +29,7 @@ "name": { "type": "string", + "description": "Name must be at least 2, at should be at most 75 characters", "optional": false, "preprocess": "localize" }, diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_manifest.js b/toolkit/components/extensions/test/xpcshell/test_ext_manifest.js index bd66d5594dfd..19ed67ba985e 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_manifest.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_manifest.js @@ -156,6 +156,28 @@ add_task( } ); +add_task(async function test_name_too_long() { + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + // This length is 80, which exceeds ExtensionData.EXT_NAME_MAX_LEN: + name: "123456789_".repeat(8), + }, + }); + await extension.startup(); + equal( + extension.extension.name, + "123456789_123456789_123456789_123456789_123456789_123456789_123456789_12345", + "Name should be truncated" + ); + Assert.deepEqual( + extension.extension.warnings, + ['Reading manifest: Warning processing "name": must be shorter than 75'], + "Expected error message when the name is too long" + ); + + await extension.unload(); +}); + add_task(async function test_simpler_version_format() { const TEST_CASES = [ // Valid cases diff --git a/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs b/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs index 030b336c795d..f5d100b444bf 100644 --- a/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs +++ b/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs @@ -579,6 +579,11 @@ async function loadManifestFromWebManifest(aPackage, aLocation) { contributors: null, locales: [aLocale], }; + if (result.name.length > lazy.ExtensionData.EXT_NAME_MAX_LEN) { + // See comment at EXT_NAME_MAX_LEN in Extension.sys.mjs. + logger.warn(`Truncating add-on name ${addon.id} for locale ${aLocale}`); + result.name = result.name.slice(0, lazy.ExtensionData.EXT_NAME_MAX_LEN); + } return result; } diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_locale.js b/toolkit/mozapps/extensions/test/xpcshell/test_locale.js index 2824c489b41b..c2f4d8665ecf 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_locale.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_locale.js @@ -51,6 +51,13 @@ add_task(async function test_1() { description: "name", }, }, + "_locales/es-ES/messages.json": { + name: { + // This length is 80, which exceeds ExtensionData.EXT_NAME_MAX_LEN: + message: "123456789_".repeat(8), + description: "name with 80 chars, should truncate to 75", + }, + }, }, }); @@ -101,3 +108,18 @@ add_task(async function test_6() { await addon.enable(); }); + +add_task(async function test_name_too_long() { + await restartWithLocales(["es-ES"]); + + let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org"); + Assert.notEqual(addon, null); + + Assert.equal( + addon.name, + "123456789_123456789_123456789_123456789_123456789_123456789_123456789_12345", + "Name should be truncated" + ); + + await addon.enable(); +});