Closed Bug 1593316 Opened 5 years ago Closed 2 years ago

Restrict Webextension version strings to Chrome's version string format

Categories

(WebExtensions :: Compatibility, enhancement)

enhancement

Tracking

(Not tracked)

RESOLVED MOVED

People

(Reporter: eviljeff, Unassigned)

References

()

Details

(Whiteboard: [addons-jira])

Firefox currently allows [1] Webextension version strings to be in Toolkit version format [2]. Chrome has a more restrictive format [3], which is a subset of the Toolkit format. Toolkit format is, imo, unnecessarily complicated for extensions, and could lead to a version being incompatible with Chrome. I propose we standardize on the <number>.<number>.<number>.<number> format, where <number> is 0 to 65535 (16 bit unsigned integer).

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/version
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Toolkit_version_format
[3] https://developer.chrome.com/extensions/manifest/version

Flags: needinfo?(philipp)

Meh.

Priority: -- → P5

If developers select versions incompatible with Chrome, that is really up to them. I'd love to know more about why the version number restriction exists at all and what advantage we'd gain by adopting it. We don't expose version numbers much, so they don't matter much to the user. I feel as long as it is compatible with the version compare code in Firefox we should be good.

Flags: needinfo?(philipp)

As discussed on github, if we're ever to do this, the opportune time would be MV3. Stop accepting version strings that don't conform to the simpler Chrome format for new MV3 addon version submissions.
https://github.com/mozilla/addons/issues/1264

Since that's a strict subset of Toolkit version format, no other changes would be necessary, Services.vc.compare() would still work as expected.

Clearing priority to retriage.

Severity: normal → --
Priority: P5 → --

This bug recently got more attention because of concerns about differences between AMO and Firefox, and the potential complexity of maintaining this. Will created a library (https://github.com/mozilla/addons-moz-compare) and it turned out to be relatively straightforward, so there is no point in spending more efforts on this issue at the Firefox side.

As for versioning be more permissive than Chrome - that's not an issue.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
See Also: → 1793925

We discussed this, and decided to make the version stricter after all. If needed we can relax the requirement later, but the other way around is not feasible.

We're going to require a strict format in MV3, and warn when used in MV2.

The implementation is straightforward, at https://searchfox.org/mozilla-central/rev/f0ffbdcf82000a8ecab13954f6618797e37c6173/toolkit/components/extensions/schemas/manifest.json#54-57:

  • Introduce validation of the version format.
  • Treat it as a warning in MV2 (e.g. "onError": "warn") (bug 1793925).
  • Treat it as an error in MV3 (same validation, without "onError": "warn") (this bug?).

To assess the impact of this change, I ran some queries to see how many extensions would be considered invalid with the new format.
Note that the queries did not account for users who have installed two of the affected add-ons, these would be over-counted in the results. I expect that impact to be negligible. The results are as follows:

To determine validity, I used the following regexps:

  • Invalid version: ^([0-9]{1,4}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])(\.([0-9]{1,4}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])){0,3}$. While this ensures that the version number is between 0 and 65335, inclusive, it may accept ambiguous version numbers (e.g. 0 vs 00, 0.01 vs 0.1 or even 0.01.2.3 vs 0.1.2.3).
  • Major version too high: ^([0-9]{6}|[7-9][0-9]{4}|6[6-9][0-9]{3}|65[4-9][0-9]{2}|653[4-9][0-9]|6533[6-9])

Follow-ups:

  • We should determine the desired behavior with version parts starting with 0. E.g. 0.1 is obviously valid. But how about 0.01 vs 0.001? or 0001 vs 01?
  • What to do with the few add-ons that have a too large major version number? 1 extension and 13 (auto-generated?) dictionaries.
  • Fix langpack generation script (e.g. like done in bug 1732676) and ensure that the privileged extension pipeline generates valid versions. This should cover 36M users.
  • The dictionaries (1.8M users) seem to have been auto-generated and/or nor updated in a while. We should look into fixing the version. Note that there may be dictionaries that we don't know about, because currently dictionaries are not required to be signed. This fix-up effort could be considered along with enforcing dictionary signing in bug 1753276.
  • If the remaining 140 extensions/themes (~200k users) were to update to a valid version, then we could enforce the new version format on AMO even for MV2 extensions, ahead of requiring it in Firefox.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
See Also: → 1732676, 1753276
Whiteboard: [addons-jira]

We decided to not force the format in Firefox yet, and instead enforce the format validation before signing.

Moving to https://github.com/mozilla/addons-linter/issues/4507

Status: REOPENED → RESOLVED
Closed: 3 years ago2 years ago
Resolution: --- → MOVED

(In reply to Rob Wu [:robwu] from comment #6)

We decided to not force the format in Firefox yet, and instead enforce the format validation before signing.

Moving to https://github.com/mozilla/addons-linter/issues/4507

Is the plan to eventually enforce it in Firefox, or just rely on the linter to gate version strings? (If the former, will this bug be reopened at a later date, or will a new bug be created then)

:eviljeff there is no plan to enforce it in Firefox for now

If you decide to go ahead and hard deprecating toolkit versioning, please consider adding version_name support first (bug 1380219), so we can provide "pretty" labels for version numbers like "11.4.13.9001" (which is currently the way NoScript 11.4.14rc1 - in its private beta channel - is packaged for Chromium-based browsers) as suggested here.
Thanks.

See Also: → 1380219
You need to log in before you can comment on or make changes to this bug.