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

Version.InvalidVersionError due to version with prerelease with leading 0 (zero) #37

Open
zoldar opened this issue Jul 16, 2024 · 0 comments

Comments

@zoldar
Copy link

zoldar commented Jul 16, 2024

Unfortunately, the problem originally reported in #35 still persists for exact same example UA string:

UAInspector.parse("Mozilla/5.0 (Linux; arm_64; Android 10; Mi Note 10) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.5765.05 Mobile Safari/537.36"

I have done some digging - my findings follow below:

Although there is a regression test added at

test "engine version with leading zero in fourth place (x.y.z.0[0-9]+)" do
agent =
"Mozilla/5.0 (Linux; arm_64; Android 10; Mi Note 10) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.5765.05 Mobile Safari/537.36"
parsed = UAInspector.parse(agent)
result = %UAInspector.Result.Client{
engine: "WebKit",
engine_version: "537.36",
name: "Chrome Mobile",
type: "browser",
version: "115.0.5765.05"
}
assert ^result = parsed.client
end
, it does not really test for problematic version comparison. That's because the default database fixture used in tests does not have a matching "WebKit" entry with "versions", so the code path resolving and comparing versions at
if :lt != Util.Version.compare(version, engine_version) do
is not even triggered and default is used.

The root problem here seems to be the fact that after semver conversion for comparison, the version string ends up like this:

UAInspector.Util.Version.to_semver("115.0.5765.05", 4)
"115.0.5765-05"

which in turn causes

Version.compare(semver1, semver2)
to fail because leading "0" in prerelease is treated as invalid (which is correct according to https://semver.org/ AFAICT, so it seems to make sense that Version follows that spec). Unfortunately, apparently versions in UA do not always follow the conventions.

Perhaps it would make sense to sanitize the prerelease by trimming any leading zeroes somewhere close to

if String.contains?(semver, "-") do
semver
else
semver <> "-0"
end
? It would have to account for a case where prelease consists entirely of "0", where it should be left intact AFAIU.

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

1 participant