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

ec_semver:parse() incorrectly parsing "pre-release identifier" containing hyphen #144

Open
limeytexan opened this issue Sep 18, 2019 · 7 comments

Comments

@limeytexan
Copy link

https://semver.org contains the following statement regarding the semantic version spec for "pre-release identifiers":

  1. A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes. Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version. Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92.

Notably, ec_semver:parse() correctly parses all four of the above examples:

3> ec_semver:parse(<<"1.0.0-alpha">>).
{{1,0,0},{[<<"alpha">>],[]}}
4> ec_semver:parse(<<"1.0.0-alpha.1">>).
{{1,0,0},{[<<"alpha">>,1],[]}}
5> ec_semver:parse(<<"1.0.0-0.3.7">>).
{{1,0,0},{[<<"0">>,3,7],[]}}
6> ec_semver:parse(<<"1.0.0-x.7.z.92">>).
{{1,0,0},{[<<"x">>,7,<<"z">>,92],[]}}

However, a few [hex] packages contain hyphens in the "pre-release identifer" portion of the semantic version, e.g. https://hex.pm/packages/xclient/0.7.0-vendored-xhttp, and this breaks the parser:

7> ec_semver:parse(<<"0.7.0-vendored-xhttp">>).
{<<"0.7.0-vendored-xhttp">>,{[],[]}}

I've found that simply changing <<"[A-Za-z0-9]">> to <<"[A-Za-z0-9-]">> in src/ec_semver_parser.erl seems to address the problem, although I haven't reviewed the code sufficiently to say that this is the most appropriate fix.

@tsloughter
Copy link
Member

First, is this a bug in hex as well? Should it not be accepting versions like that (it is supposed to enforce semver format)? Either way we need to support this in ec_semver but would want to also notify the hex project.

@limeytexan
Copy link
Author

From my reading of the spec, a semantic version of 0.7.0-vendored-xhttp is valid because the "pre-release identifier" part (in this example, vendored-xhttp) is allowed to contain hyphens. I agree that hex is responsible for enforcing valid semantic version strings, but in this case I don't believe it's wrong to be accepting semantic versions like this one.

Many thanks for the quick response!

@paulo-ferraz-oliveira
Copy link
Contributor

Regarding this, do we want to import verl, or find some such similar solution?

@ferd
Copy link
Collaborator

ferd commented Apr 2, 2021

We'd need to eventually import verl, but the library is gonna need patches to fit along with rebar3 as well. I've just been working on other side projects rather than supporting this unfortunately.

@paulo-ferraz-oliveira
Copy link
Contributor

Let me know if I can help. You can probably describe 2 or 3 things that are necessary and I'd try to find time to tackle them.

@leeyisoft
Copy link

I am running well under erlware_commons v1.6.0

([email protected])2> ec_semver:parse(<<"0.7.0-vendored-xhttp">>).
{{0,7,0},{[<<"vendored-xhttp">>],[]}}

@ariel-anieli
Copy link

However, a few [hex] packages contain hyphens in the "pre-release identifer" portion of the semantic version, e.g. https://hex.pm/packages/xclient/0.7.0-vendored-xhttp, and this breaks the parser:

7> ec_semver:parse(<<"0.7.0-vendored-xhttp">>).
{<<"0.7.0-vendored-xhttp">>,{[],[]}}

I've found that simply changing <<"[A-Za-z0-9]">> to <<"[A-Za-z0-9-]">> in src/ec_semver_parser.erl seems to address the problem, although I haven't reviewed the code sufficiently to say that this is the most appropriate fix.

@limeytexan, from what I see, 4e3b177 fixed the issue:
In 1.7.0, the example you proposed is correctly parsed:

8> ec_semver:parse(<<"0.7.0-vendored-xhttp">>).
{{0,7,0},{[<<"vendored-xhttp">>],[]}}
9> ec_file:md5sum(ec_semver:module_info(md5)).
"b4f8e531a6a10f0a83d655bf991c921e"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants