1793925 - Warn about complex versioning formats used in manifest

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement)

Description

[William Durand [:willdurand]]

The current format of the version field in the manifest can be very complicated. Let's encourage the use of a simpler versioning format (see below) by adding format + onError:warn to the schemas. Firefox will only emit warnings. We might enforce a simpler format later but that will be done in the linter, not in Firefox.

Simple format: <number>.<number>.<number>.<number> where <number> is 0 to 65535 (16 bit unsigned integer) 0 to 999999999 (an integer with up to 9 digits) and no leading zeros.

Comment 1

[William Durand [:willdurand]]

Attachment: Bug 1793925 - Warn about complex versioning formats used in manifest. r?robwu!

Comment 2

[Shane Caraveo (:mixedpuppy)]

16bit will not cover at least one addon currently on AMO. We've no reason to limit to that small of a value, we can use Tom's suggestion of 2^31.

Comment 3

[Rob Wu [:robwu]]

The current format (before the patch) does not really enforce a limit on the maximum size. We're trying to choose reasonable constraints, and have to account for minimums and maximums. There is much more context on the validity and actual observed versions in https://bugzilla.mozilla.org/show_bug.cgi?id=1593316#c5. That was based on the assumption of using the 16-bit version parts.

I did some more analysis, and have some more information:

Number of digits per part:

• Minimum values (TL;DR 8 digits in practice):
  • 8 digits, YYYYMMDD is used in the wild per https://bugzilla.mozilla.org/show_bug.cgi?id=1593316#c5
  • 65535 (unsigned 16-bit; version documented max length of Chrome's format).
  • The bare minimum is 65535, because this is Chrome's documented format. If an invalid version is specified, the error is "Required value 'version' is missing or invalid. It must be between 1-4 dot-separated integers each between 0 and 65536."
  • However, a higher value is still accepted by the browser. Up to 2^32-1 in fact, because Chrome's implementation represents the parts with unsigned 32-bit integers.
  • The Chrome Web Store does enforce the 16-bit limit, so despite Chromium's more generous limit, the parts of the Chrome extension are 16-bit integers in practice.
• Maximum values (TL;DR int32 or 9 digits):
  • Maximum safe integer in JavaScript = 2^53-1 = 9_007_199_254_740_991 (i.e. 15-16 digits)
    • Relevant for JS-based version comparators such as addons-moz-compare
  • (signed) integer in C = 2^31 - 1 = 2_147_483_648 (i.e. 9-10 digits)
    • Relevant for C-based version comparator such as nsVersionComparator.cpp. When the number is too large, bugs like bug 1732676 happen.
  • (unsigned) integer in C = 2 ^ 32 - 1 = 4_294_967_295 (i.e. 9-10 digits)
  • Telemetry limit: Maximum byte length limit of an individual value: 80 bytes. Minus 3 dots, the max length of each of the four version parts can be floor(77/4) = 19
  • Python integers have arbitrary precision, but the applications (AMO backend) may have additional constraints (see next).
  • AMO limit on version numbers = 255 characters

Ambiguity of version numbers

• When ambiguity is involved, equality of version numbers can no longer be determined by string comparisons.
• Ambiguous cases:
  • Non-zero digits preceded by zeroes, e.g. 1.1 vs 1.01 vs 1.001 - contrary to intuition when read as a number, these are equivalent versions.
    • Although forbidden according to Chrome's version documented, both the Chrome browser and the Chrome Web Store do accept numbers in this format.
  • Non-zero parts followed by zero-parts, e.g. 1 vs 1.0 vs 1.0.0

Zero:

• Chrome's version documented forbids all-zeroes, but in practice both Chrome and Firefox can load these without issues.

Comment 4

[Rob Wu [:robwu]]

The stricter version format needs to be documented, i.e. the content of https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/version should be revised.

Comment 5

[Rob Wu [:robwu]]

FYI: There are no signed extensions on AMO whose latest version number has up to 4 parts separated by a dot with up to 9 digits each.

I ran the query again for all versions ever signed (not just the latest), and there are still only 18 results, of which only 2 add-ons have users with a combines user count of less than 100: https://sql.telemetry.mozilla.org/queries/88108/source
This query excludes toolkit version formats; if I include these, there are 20 add-ons with less than 20k users combined whose latest version is in a toolkit version format (i.e. containing letters) and contain a leading zero in its part.

In other words, we won't breaking any published add-ons if we were to disallow leading zeroes in the strict digits-only version format, AND strictly enforce that.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f112c0942b0
Warn about complex versioning formats used in manifest. r=robwu

Comment 7

[Norisz Fay [:noriszfay]]

Backed out for causing build bustages

Backout link

Push with failures

Failure log

There's also this valgrind. Failure log here

Comment 8

[Rob Wu [:robwu]]

Build is busted because quitter.xpi at https://searchfox.org/mozilla-central/source/tools/quitter contains "version": "1.0.0buildid20220905.072618",

Extension source code viewer for quitter.xpi in the tree: https://robwu.nl/crxviewer/?crx=https%3A%2F%2Fsearchfox.org%2Fmozilla-central%2Fsource%2Ftools%2Fquitter%2Fquitter%40mozilla.org.xpi&q=!version&qf=manifest.json&qb=1&qh=0&qi=2

The source code on Github uses 1.0: https://github.com/mozilla-extensions/quitter/blob/809841afcbf92a0d2783b0e8b9cc13fb4a278c3f/manifest.json#L10
... so something in the privileged extension build pipeline has probably appended to the version.

To unblock the patch, you could consider setting extensions.webextensions.warnings-as-errors to false
at https://searchfox.org/mozilla-central/rev/0a2eba79c24300ce0539f91c1bebac2e75264e58/build/pgo/profileserver.py#120
and file a follow-up bug to require a new Quitter build without a non-conforming build number.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c8274750e0e
Warn about complex versioning formats used in manifest. r=robwu

Comment 10

[William Durand [:willdurand]]

:rbloor FYI, here is the documentation we will publish on extensionworkshop.com soon: https://github.com/mozilla/extension-workshop/pull/1430

Comment 11

[Rob Wu [:robwu]]

Richard, see this in particular: https://github.com/mozilla/extension-workshop/pull/1430#discussion_r997558988

Comment 12

[Sandor Molnar]

https://hg.mozilla.org/mozilla-central/rev/1c8274750e0e

Comment 13

[rbloor]

:robwu :willdurand sorry I missed this earlier. I think the EW changes probably need some more work and I'll look at those when I do the MDN changes. Depending on other priorities I will hopefully get to that this week.

Comment 14

[rbloor]

Were the changes we made in Version number simplification for MV3 (#21699) what was needed for this? Or is more needed?