On Tue, 28 Oct 2025 18:30:51 GMT, Sean Mullan <[email protected]> wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> A4-A6 from RFC
>
> src/java.base/share/classes/sun/security/pkcs12/PBMAC1Parameters.java line
> 118:
>
>> 116: * Encode PBMAC1 parameters from components.
>> 117: */
>> 118: static byte[] encode(byte[] salt, int iterationCount, int keyLength,
>
> This is encoding more than the PBMAC1 parameters, it is also encoding the
> `MacData` structure. That should really be in the `MacData` class.
I moved it to the `MacData` class as you suggest. `macSalt` ("NOT USED") and
`iterations` (1) also belong in `MacData` and have been moved.
> src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 153:
>
>> 151: * Encode PBKDF2 parameters from components.
>> 152: */
>> 153: public static byte[] encode(byte[] salt, int iterationCount,
>
> This method actually encodes more than the parameters. It also encodes the
> outer algorithm id. I guess that's ok, but I suggest adding a comment that
> says that.
added
> src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 159:
>
>> 157: DerOutputStream out = new DerOutputStream();
>> 158: DerOutputStream tmp0 = new DerOutputStream();
>> 159: DerOutputStream tmp1 = new DerOutputStream();
>
> unused variable.
fixed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2474349234
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2474348526
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2474348262