Open zoldar opened 4 months ago
Unfortunately, the problem originally reported in https://github.com/elixir-inspector/ua_inspector/issues/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 https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/test/ua_inspector/parser/client_test.exs#L39-L54, 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 https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/lib/ua_inspector/parser/client.ex#L304 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 https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/lib/ua_inspector/util/version.ex#L29 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.
Version
Perhaps it would make sense to sanitize the prerelease by trimming any leading zeroes somewhere close to https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/lib/ua_inspector/util/version.ex#L157-L161? It would have to account for a case where prelease consists entirely of "0", where it should be left intact AFAIU.
Unfortunately, the problem originally reported in https://github.com/elixir-inspector/ua_inspector/issues/35 still persists for exact same example UA string:
I have done some digging - my findings follow below:
Although there is a regression test added at https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/test/ua_inspector/parser/client_test.exs#L39-L54, 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 https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/lib/ua_inspector/parser/client.ex#L304 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:
which in turn causes https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/lib/ua_inspector/util/version.ex#L29 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
Versionfollows 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 https://github.com/elixir-inspector/ua_inspector/blob/89547217071203c8749661c5c8061ffff6e478ae/lib/ua_inspector/util/version.ex#L157-L161? It would have to account for a case where prelease consists entirely of "0", where it should be left intact AFAIU.