RFR: 8358594: Misleading keyLength value captured in JFR event for ML-KEM key [v3]

[Weijun Wang]

On Tue, 17 Jun 2025 13:08:24 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   enhance test to be exhaustive
>
> src/java.base/share/classes/sun/security/util/KeyUtil.java line 150:
> 
>> 148:                     && ak.getParams() instanceof NamedParameterSpec nps)
>> 149:                 ? nps.getName()
>> 150:                 : k.getAlgorithm();
> 
> This case is not covered. Could you please add something like this test? it should cover the case when this is false `k instanceof AsymmetricKey ak && ak.getParams() instanceof NamedParameterSpec nps`
> 
> 
>     static void testDummyKey() throws Exception {
> 
>         Key key = new Key() {
>             @Override
>             public String getAlgorithm() {
>                 return "";
>             }
> 
>             @Override
>             public String getFormat() {
>                 return "";
>             }
> 
>             @Override
>             public byte[] getEncoded() {
>                 return new byte[0];
>             }
>         };
> 
>         Asserts.assertEQ(-1, KeyUtil.getNistCategory(key));
>     }
> ``` 
> and `testDummyKey();` on line 52 of course.

Added a test case on `SecretKey`, which is not an `AsymmetricKey`.

> test/jdk/sun/security/provider/all/NistCategories.java line 46:
> 
>> 44:         for (var p : Security.getProviders()) {
>> 45:             for (var s : p.getServices()) {
>> 46:                 switch (s.getType()) {
> 
> Minor: why not just this? I don't mind, just asking
> 
> 
>  for (var p : Security.getProviders()) {
>             for (var s : p.getServices()) {
>                 if ("KeyPairGenerator".equals(s.getType())) {
>                     test(s);
>                 }
>             }
>         }

Oh, you're right it's not worth a switch. I might have just copied the same structure from another test where there can be different actions on more than 1 type. I'll change it. Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25642#discussion_r2152342047
PR Review Comment: https://git.openjdk.org/jdk/pull/25642#discussion_r2152335707