271269 - Add proper validation for incoming versions / version numbers

Status: VERIFIED DUPLICATE of bug 304857

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[alanjstr]

As stated in the official documentation on mozilla.org, extension versions must
be of form major.minor.release.build[+]. Old-style versions like 0.5pre are now
invalid.

Comment 1

[Wolf [:wolf]]

There's actaully a valid use-case for this staying unfixed. The webservice kicks
out "invalid" versions already, so items like Chatzilla, which use development
versions for releases that aren't in CVS like 0.9.65a. So, users of say, 0.9.65,
won't be offered the developmental .65a. and won't be bothered until the next
stable release 0.9.66.

Sometimes, if it isn't broke, don't fix it.

Comment 2

[Wolf [:wolf]]

Bulk Moving Developer Control Panel bugs to new component.
(Filter: massdevcpspam)

Comment 3

[alanjstr]

UMO is not meant for hosting "developmental" builds.  Even if it is, that's what
BuildIDs are for.  Playing games with what gets ignored and what does not is risky.

Comment 4

[Jens Bannmann]

The matching regular expression is:
/^\d+(\.\d+){0,3}\+{0,1}$/

Note that Chatzilla currently uses a valid version string. And if they want to
release in-between versions to UMO in the future, they can use the build portion
(like 0.9.66.20041130).

Comment 5

[alanjstr]

Does that allow the formats of 1, 1.1, and 1.1.1 too?  My regex knowledge is a
bit rusty.

Comment 6

[Jens Bannmann]

(In reply to comment #5)
> Does that allow the formats of 1, 1.1, and 1.1.1 too?  My regex knowledge is a
> bit rusty.

Yes, it does.

/^\d+(\.\d+){0,3}\+{0,1}$/ roughly translates to "String starts with one or more
digits, followed by 0 to 3 groups, each with a dot and one or more digits, then
followed zero or 1 plus signs before the string ends."

Comment 7

[alanjstr]

I don't think a plus sign was supposed to be there.

Comment 8

[Jens Bannmann]

(In reply to comment #7)
> I don't think a plus sign was supposed to be there.

Well, according to
http://www.mozilla.org/projects/firefox/extensions/update.html
[Section "Firefox Version Format"]
and
http://www.mozilla.org/projects/firefox/extensions/update.html
it is allowed there. However, I never understood how it's supposed to work with
update - e.g. if I have 1.2, I will be offered an 1.2+, but if another
"in-between" 1.2+ release occurs, how will Firefox distinguish it?

Comment 9

[alanjstr]

You're talking about FX versioning.  I'm talking about extension versioning.  In
terms of applications, a, b, and + are a whole range of things.

Comment 10

[Jens Bannmann]

(In reply to comment #9)
> You're talking about FX versioning.  I'm talking about extension versioning.  In
> terms of applications, a, b, and + are a whole range of things.

The versioning applies to both the application and the extensions:

"Firefox and all Extensions and Themes that work with Firefox must be versioned
in this way in order for compatibility checking and Update to work" (from the
versioning/update docs)

and

"em:version - A Firefox Version Format (FVF) string identifying the version that
is being supplied by this package." (from the packaging docs)

(Btw: In comment #8 I pasted the same URL twice, I meant to link also to
http://www.mozilla.org/projects/firefox/extensions/packaging/extensions.html )

Comment 11

[alanjstr]

hmmm.  ok, so the spec says it.  i, however, disagree, since we have the Build
ID.  UMO can't handle multiple a's or +'s.

Comment 12

[Jens Bannmann]

I'd also disallow '+' signs in item versions (I just wanted to state it's in the
spec). The appropriate regex is then:
/^\d+(\.\d+){0,3}$/

Comment 13

[alanjstr]

Attachment: version format check

Comment 14

[Wolf [:wolf]]

My understanding from Comment #12 and the attached patch is we're now jumping
from one "invalid" form to another. By leaving off the + which is valid in FVF.

I do not support this change. Its up to extension authors to use valid
versioning, that works for them, I don't feel placing update in the middle of it
as an enforcer of FVF is going to help anything. Particularly when this
obviously breaks versioning that both Firefox and Update find perfectly valid.

Comment 15

[alanjstr]

My goal is to make UMO sort data properly without having high overhead.

Comment 16

[Jens Bannmann]

Firefox 1.1 will reject invalid version strings, see
http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#2865

So it's clear that UMO should do that, too. The code used in nsVersionChecker is
/^([0-9]+\.){0,3}[0-9]+\+?$/.test(aVersion); - I guess the regex itself should
and can be copied as-is. What a trailing + sign should mean for UMO would have
to be discussed with one of the Fx devs...

Comment 17

[Mike Shaver (:shaver emeritus)]

*** This bug has been marked as a duplicate of 304857 ***