On Tue, 30 Sep 2025 19:29:07 GMT, Sean Mullan <[email protected]> wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> another day another iteration
>
> src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java
> line 84:
>
>> 82: private int keyLength = -1;
>> 83:
>> 84: protected void Init(AlgorithmParameterSpec paramSpec)
>
> I don't think you need a separate method. Put this code in the constructor.
> Same for the other `Init` method. Then you can also make the fields final.
fixed
> src/java.base/share/classes/sun/security/pkcs12/MacData.java line 174:
>
>> 172: }
>> 173:
>> 174: void processMacData(AlgorithmParameterSpec params,
>
> Can be static?
No - perhaps in an earlier iteration of the code, but not in the latest.
> src/java.base/share/classes/sun/security/pkcs12/MacData.java line 186:
>
>> 184: kdfHmac = macAlgorithm;
>> 185: Hmac = macAlgorithm;
>> 186: }
>
> Did you consider adding another kdfHmac parameter to the method and passing
> in the correct values?
This comment appears to apply to an earlier iteration. It is not easy to keep
track of all these iterations. I believe the latest iteration fixes this.
> src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 71:
>
>> 69: * </pre>
>> 70: */
>> 71: final public class PBKDF2Parameters {
>
> `public` should be before `final`. For reference, see the last sentence of
> https://docs.oracle.com/javase/specs/jls/se25/html/jls-8.html#jls-8.1.1
fixed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173249
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173197
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173157
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173428
