RFR: 8367008: Algorithm identifiers for HmacSHA* should always have NULL as params [v7]

[Mikhail Yankelevich]

On Sat, 18 Oct 2025 20:23:41 GMT, Koushik Muthukrishnan Thirupattur <duke at openjdk.org> wrote:

>> Looking at RFC 9879 on PBES2 and PBMAC1 in PKCS12, algorithm identifiers for HmacSHA*** (like SHA***) should always contain NULL as params. We can update the list at AlgorithmId.encode(DOS) to enforce this rule.
>
> Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8367008: Algorithm identifiers for HmacSHA* should always have NULL as params

Looks good, just a few questions about the test

test/jdk/sun/security/x509/AlgorithmId/AlgorithmIdEqualsHashCode.java line 32:

> 30:  */
> 31: 
> 32: import java.io.*;

Nit: as you're touching this tests, if there is another commit, could you please change all wildcard imports in the test to the explicit ones?

test/jdk/sun/security/x509/AlgorithmId/AlgorithmIdEqualsHashCode.java line 42:

> 40: public class AlgorithmIdEqualsHashCode {
> 41: 
> 42:     static boolean failed = false;

As far as I can see, this is only used as a part of the test. Could you please make it local variable if it stays at all?

test/jdk/sun/security/x509/AlgorithmId/AlgorithmIdEqualsHashCode.java line 103:

> 101:         }
> 102: 
> 103:         try {

I have 2 questions here: 

1. Wouldn't it be easier if it was fail by default and pass only if conditions are met? In case of the future changes this seems to me to be a bit clearer and easier to maintain. But i'm ok with how it is  now as well. 
2. I strongly feel you should throw an exception when this is failed, otherwise this will never indicate a fail, only log. This way it is also consistent with all the other code above. However, if you want to keep it as it is, what do you think about checking the failed status underneath and throwing an exception if it failed?

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

PR Review: https://git.openjdk.org/jdk/pull/27700#pullrequestreview-3353766572
PR Review Comment: https://git.openjdk.org/jdk/pull/27700#discussion_r2442593187
PR Review Comment: https://git.openjdk.org/jdk/pull/27700#discussion_r2442592276
PR Review Comment: https://git.openjdk.org/jdk/pull/27700#discussion_r2442590144