RFR: 8366182: Some PKCS11Tests are being skipped when they shouldn't [v2]
Matthew Donovan
mdonovan at openjdk.org
Fri Sep 5 13:46:52 UTC 2025
On Fri, 5 Sep 2025 09:29:28 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:
>> I may be misunderstanding your comment but it looks like you're using a single, `double` value to compare versions. The original code did that but it doesn't work in all cases. The problem occurs when checking a range such as
>>
>> `version >= 3.11 && version < 3.12`
>>
>> If the version number is 3.111, then the comparison is `true` and tests are skipped, even though _version_ 3.111 is "greater" than 3.12. So this code creates two doubles: `major` and `minor` to do separate comparisons of the values.
>>
>> To further complicate things, NSS also has versions of the form `x.y.z`. The original code combined the 'y' and 'z' components to go from 3.11.9 to 3.119. My change would create major version 3.0 and minor version 11.9.
>
> Ah, you're right, I didn't think of this case! This approach will be easier then, I agree.
>
> Still, `ver.substring(idx+1)` may cause errors in cases like this [NSS 3.15.3.1 release notes](https://nss-crypto.org/reference/security/nss/legacy/nss_releases/nss_3.15.3.1_release_notes/index.html#mozilla-projects-nss-nss-3-15-3-1-release-notes). It will try to convert 15.3.1 to a double. Do you think changing to something like this would be worth it?
>
>
> String verSubstring = ver.substring(idx+1);
> String[] splitParts = verSubstring.split("//.");
> Double minor = Double.parseDouble(splitParts.length > 1
> ? splitParts[0] + "." + splitParts[1]
> : splitParts[0]);
That's a good point about 4-component version numbers. Frankly, I was hoping that NSS never did that. I refactored the version parsing code to create a `Version` record with major, minor, and patch versions. This way, individual tests don't have to do the String parsing. If we end up with a test that has to be skipped due to that 4th component, we can update the parsing accordingly. Alternately, I can add it since I'm here now... what is that fourth component called?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27095#discussion_r2325142358
More information about the security-dev
mailing list