Closed Bug 1793925 Opened 2 years ago Closed 2 years ago

Warn about complex versioning formats used in manifest

Categories

(WebExtensions :: General, enhancement)

enhancement

Tracking

(firefox108 fixed)

RESOLVED FIXED
108 Branch
Tracking Status
firefox108 --- fixed

People

(Reporter: willdurand, Assigned: willdurand)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [addons-jira])

Attachments

(1 file)

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.

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.

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)
    • (signed) integer in C = 2^31 - 1 = 2_147_483_648 (i.e. 9-10 digits)
    • (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.
Blocks: 1795285

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.

Keywords: dev-doc-needed
See Also: → 1733396

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.

Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f112c0942b0
Warn about complex versioning formats used in manifest. r=robwu

Backed out for causing build bustages

Backout link

Push with failures

Failure log

There's also this valgrind. Failure log here

Flags: needinfo?(wdurand)

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.

Blocks: 1795750
Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c8274750e0e
Warn about complex versioning formats used in manifest. r=robwu

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

Flags: needinfo?(wdurand) → needinfo?(rbloor)
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch

: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.

Flags: needinfo?(rbloor)
See Also: → 1796531

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

Flags: needinfo?(rob)
Flags: needinfo?(rob)
See Also: → 1824240
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: