Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(semver): Fix issue with prerelease numerics which lead with zeros #3521

Conversation

justinmchase
Copy link
Contributor

@justinmchase justinmchase commented Aug 7, 2023

If a pre-release version has a segment in the numeric section of a pre-release version, which starts with zero (e.g. .04) then it fails to parse the version entirely.

This doesn't make sense to me and I thought it was a regression but it has been that way for a long time, but I do still think it is an issue which should be fixed. I cannot think of a reason why 04 should not be parsed as a numeric when parseInt(v) will clearly still parse it correctly. I tested it manually and this will also work on any number regardless of its leading zeros (e.g. .00004)

Background

The npm semver library has this code in it:
https://github.com/npm/node-semver/blob/cce61804ba6f997225a1267135c06676fe0524d2/internal/re.js#L52-L56

// ## Numeric Identifier
// A single `0`, or a non-zero digit followed by zero or more digits.

createToken('NUMERICIDENTIFIER', '0|[1-9]\\d*')
createToken('NUMERICIDENTIFIERLOOSE', '\\d+')

That change is over 4 years old and hard to trace back further than that. I am not sure why they would have wanted to exclude numbers which are not zero but lead with zero.

Real-World Example

I was planning on trying to show people in this thread how this library could be used to sort these versions with a simple example but many of the versions are being dropped due to this incompatibility and so its not working as expected in this real-world example.

With this PR applied then the example will work as expected.

import * as semver from "https://deno.land/std/semver/mod.ts";

const versions = [
  "v2.285.1-ubuntu-20.04",
  "v2.285.1-ubuntu-20.04-2bd6d63",
  "v2.286.0-ubuntu-20.04-8a7720d",
  "v2.286.0-ubuntu-20.04",
  "v2.286.0-ubuntu-20.04-ad48851",
  "v2.286.1-ubuntu-20.04",
  "v2.286.1-ubuntu-20.04-5e86881",
  "v2.287.0-ubuntu-20.04",
  "v2.287.0-ubuntu-20.04-81b2c5a",
  "v2.287.1-ubuntu-20.04-715e6a4",
  "v2.287.1-ubuntu-20.04-f09a974",
  "v2.287.1-ubuntu-20.04-cc25dd7",
  "v2.287.1-ubuntu-20.04",
  "v2.287.1-ubuntu-20.04-6f591ee",
  "v2.288.0-ubuntu-20.04",
  "v2.288.0-ubuntu-20.04-f0fa99f",
  "v2.288.1-ubuntu-20.04-e42db00",
  "v2.288.1-ubuntu-20.04-c221b6e",
  "v2.288.1-ubuntu-20.04",
  "v2.288.1-ubuntu-20.04-b25a0fd",
  "v2.289.1-ubuntu-20.04-4cbbcd6",
  "v2.289.1-ubuntu-20.04-2cb04dd",
  "v2.289.1-ubuntu-20.04",
  "v2.289.1-ubuntu-20.04-debf53c",
  "v2.289.2-ubuntu-20.04",
  "v2.289.2-ubuntu-20.04-971c54b",
  "v2.290.0-ubuntu-20.04-7b8057e",
  "v2.290.0-ubuntu-20.04-4a3b7bc",
  "v2.290.0-ubuntu-20.04",
  "v2.290.0-ubuntu-20.04-352e206",
  "v2.290.1-ubuntu-20.04-8195178",
  "v2.290.1-ubuntu-20.04",
  "v2.290.1-ubuntu-20.04-059481b",
  "v2.291.1-ubuntu-20.04-800d6bd",
  "v2.291.1-ubuntu-20.04-c1e5829",
  "v2.291.1-ubuntu-20.04-d01595c",
  "latest",
  "v2.291.1-ubuntu-20.04",
  "v2.291.1-ubuntu-20.04-3ca1152",
  "v2.291.1-ubuntu-20.04-2120981",
  "v2.292.0-ubuntu-20.04",
  "v2.292.0-ubuntu-20.04-ac27df8",
  "v2.292.0-ubuntu-20.04-0cd13fe",
  "v2.293.0-ubuntu-20.04",
  "v2.293.0-ubuntu-20.04-933b0c7",
  "v2.293.0-ubuntu-20.04-23f091d",
  "v2.293.0-ubuntu-20.04-3c7d3d6",
  "v2.294.0-ubuntu-20.04",
  "v2.294.0-ubuntu-20.04-f24e2fa",
  "v2.294.0-ubuntu-20.04-071898c",
  "v2.294.0-ubuntu-20.04-84d16c1",
  "v2.294.0-ubuntu-20.04-e2c8163",
  "v2.294.0-ubuntu-20.04-bc7a3ca",
  "v2.294.0-ubuntu-20.04-abb8615",
  "v2.294.0-ubuntu-20.04-af96de6",
  "v2.294.0-ubuntu-20.04-0386c07",
  "v2.294.0-ubuntu-20.04-ddd417f",
  "v2.294.0-ubuntu-20.04-d86bd2b",
  "v2.294.0-ubuntu-20.04-067ed2e",
  "v2.294.0-ubuntu-20.04-5ea0841",
  "v2.294.0-ubuntu-20.04-fc63d6d",
  "v2.294.0-ubuntu-20.04-63935d2",
  "v2.294.0-ubuntu-20.04-1ce0a18",
  "v2.294.0-ubuntu-20.04-858ef89",
  "v2.294.0-ubuntu-20.04-73e430c",
  "v2.294.0-ubuntu-20.04-f661249",
  "v2.294.0-ubuntu-20.04-d4f35cf",
  "v2.294.0-ubuntu-20.04-9b28e63",
  "v2.294.0-ubuntu-20.04-2fe6adf",
  "v2.294.0-ubuntu-20.04-82641e5",
  "v2.294.0-ubuntu-20.04-e3deb0d",
  "v2.294.0-ubuntu-20.04-11cb9b7",
  "v2.294.0-ubuntu-20.04-c658dcf",
  "v2.294.0-ubuntu-20.04-98b17dc",
  "v2.294.0-ubuntu-20.04-37aa1a0",
  "v2.294.0-ubuntu-20.04-3f78f71",
  "v2.294.0-ubuntu-20.04-fc55477",
  "v2.295.0-ubuntu-20.04",
  "v2.295.0-ubuntu-20.04-784019f",
  "v2.295.0-ubuntu-20.04-3724b46",
  "v2.296.0-ubuntu-20.04",
  "v2.296.0-ubuntu-20.04-55ca7bf",
  "v2.296.1-ubuntu-20.04",
  "v2.296.1-ubuntu-20.04-e233f7a",
  "v2.296.2-ubuntu-20.04",
  "v2.296.2-ubuntu-20.04-0615c2a"
]

const filtered = versions.filter(v => semver.valid(v))
const sorted = semver.sort(filtered)
console.log(sorted)

@justinmchase justinmchase requested a review from kt3k as a code owner August 7, 2023 17:28
@github-actions github-actions bot added the semver label Aug 7, 2023
@kt3k
Copy link
Member

kt3k commented Aug 8, 2023

semver spec 9. says:

Numeric identifiers MUST NOT include leading zeroes.

So I don't think we should handle 04 as a numeric identifier. I think this should be handled as non numeric identifier.

@timreichen
Copy link
Contributor

I agree with @kt3k.
There is an official regexp on semver.org:

See: https://regex101.com/r/Ly7O1x/3/

^(?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)\.(?P<patch>0|[1-9]\d*)(?:-(?P<prerelease>(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?P<buildmetadata>[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$

I think we should stick to that.

@justinmchase
Copy link
Contributor Author

Shit I missed that, but should it still parse as a non numeric?

@kt3k
Copy link
Member

kt3k commented Aug 11, 2023

After reading the spec carefully, it seems 04 shouldn't be treated even as non numeric identifier (alphanumeric identifier).

pre-release identifier is either alphanumeric identifier or numeric identifier.

<pre-release identifier> ::= <alphanumeric identifier>
                           | <numeric identifier>

04 is not a numeric identifier because the leading 0 is not allowed (as mentioned above):

<numeric identifier> ::= "0"
                       | <positive digit>
                       | <positive digit> <digits>

04 is not a alphanumeric identifier either because it's defined as:

<alphanumeric identifier> ::= <non-digit>
                            | <non-digit> <identifier characters>
                            | <identifier characters> <non-digit>
                            | <identifier characters> <non-digit> <identifier characters>

From the above definition, alphanumeric identifier has to include at least one non-digit (i.e. a-z, A-Z, or '-'). 04 doesn't include non-digit. It's not an alphanumeric identifier, and it means it's invalid as pre-release identifier.

references:

@justinmchase
Copy link
Contributor Author

I think you're right, but I gotta say it seems overly specific. I can't actually imagine why someone thought to block leading zeros from numerics... I'll close this and take it up with upstream if it keeps bothering me but for now in practical terms it means we just can't parse some real world versions.

@justinmchase justinmchase deleted the fix/semver-regression-non-leading-zeros branch August 12, 2023 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants