RFR: 8366182: Some PKCS11Tests are being skipped when they shouldn't

Matthew Donovan mdonovan at openjdk.org
Thu Sep 4 16:58:50 UTC 2025


On Thu, 4 Sep 2025 15:37:57 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

>> This PR updates PKCS11 tests to better handle NSS version numbers. The previous code treated the version numbers as double values and used comparison operators. The problem is that it incorrectly treats 3.111 as between 3.11 and 3.12. This update parses and compares the major and minor version numbers separately.
>
> test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java line 330:
> 
>> 328:                 String osName = System.getProperty("os.name");
>> 329:                 int idx = ver.indexOf(".");
>> 330:                 double major = Double.parseDouble(ver.substring(0, idx));
> 
> IMO the split between major version and minor is a bit hard to read. Wouldn't it be easier to just get a major.minor version entirely with something like:
> 
> 
> String[] splitParts = ver.split("//.");
> Double.parseDouble(splitParts.length > 1
>         ? splitParts[0] + "." + splitParts[1] 
>         : splitParts[0]);
> 
> 
> This way it will always take a full double value (1.13 will not be the same as 1.1, as it's now as far as I can see) and would be a bit easier to understand 
> 
> The checking for only major version could be either `doubleVersion<4 && doubleVersion>=3` or even cleaner, using floor function `Math.floor(doubleVersion)`
> 
> And the same for the other files.
> 
> What do you think?

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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27095#discussion_r2322797235


More information about the security-dev mailing list