On Wed, 24 Sep 2025 19:28:12 GMT, Sean Mullan <[email protected]> wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix behavior with keytool
>
> src/java.base/share/classes/sun/security/pkcs12/MacData.java line 94:
>
>> 92: pbeSpec =
>> 93: this.digestAlgorithmParams.getParameterSpec(
>> 94: PBEParameterSpec.class);
>
> I think you may already be working on this, so mainly registering this as a
> comment. This code should be replaced with an internal method that calls
> `AlgorithmId.getEncodedParams()` and decodes the parameters, reusing much of
> the code you have already written in `PBMAC1Parameters.engineInit()`. This
> will avoid having to create an `AlgorithmParameters` implementation as part
> of this feature, which isn't strictly needed. We can consider adding that on
> a follow-on Enhancement.
Done.
> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1953:
>
>> 1951: private void processMacData(AlgorithmParameterSpec params,
>> 1952: MacData macData, char[] password, byte[] data, String
>> macAlgorithm)
>> 1953: throws Exception {
>
> Try just throwing the exceptions that can be thrown by code in this method,
> rather than `Exception` for everything. I know there is a "try/catch
> (Exception)" block in `engineLoad` when calling this method, but I think it
> is cleaner to only declare the exceptions that can be thrown here.
Agreed. I'll fix this.
> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2214:
>
>> 2212: new PBEParameterSpec(salt, ic);
>> 2213: processMacData(params, macData, password,
>> authSafeData,
>> 2214: macAlgorithm);
>
> These 4 lines can be moved below after the if/else block since they are the
> same for both conditions.
Done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2388311568
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2388312083
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2388312524
