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

Reply via email to