On Tue, 2 Sep 2025 19:55:13 GMT, Valerie Peng <[email protected]> wrote:
>> Mark Powers has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 17 commits: >> >> - merge >> - removed changes to PBMAC1Core and addressed some comments from Valerie >> - small changes >> - not used >> - refresh index >> - Merge >> - rework to eliminate PBMAC1ParameterSpec >> - merge >> - comments from Valerie >> - missed this new file >> - ... and 7 more: https://git.openjdk.org/jdk/compare/075ebb4e...624ef92e > > src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java > line 88: > >> 86: * >> 87: * id-hmacWithSHA1 OBJECT IDENTIFIER ::= {digestAlgorithm 7} >> 88: * > > nit: instead of repeating all this here, maybe refer to > sun.security.util.PBKDF2Parameters class for PBKDF2 related ASN.1 definition? added a link > src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java > line 96: > >> 94: >> 95: private static final ObjectIdentifier pkcs5PBKDF2_OID = >> 96: ObjectIdentifier.of(KnownOIDs.PBKDF2WithHmacSHA1); > > nit: duplicated to the one defined in sun.security.util.PBKDF2Parameters > class? Good catch. Fixed. > src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 36: > >> 34: /** >> 35: * This class implements the parameter set used with password-based >> 36: * key derivation function 2 (PBKDF2), which is defined in PKCS#5 as >> follows: > > nit: add link to RFC 8018 here for reference done > src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 67: > >> 65: * id-hmacWithSHA1 OBJECT IDENTIFIER ::= {digestAlgorithm 7} >> 66: */ >> 67: public class PBKDF2Parameters { > > nit: can be marked final? yes > src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 72: > >> 70: ObjectIdentifier.of(KnownOIDs.PBKDF2WithHmacSHA1); >> 71: >> 72: // AlgorithmIdentifier > > nit: comment doesn't seem related or useful? Maybe remove? done ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2353274668 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2353274843 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2353274459 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2353275031 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2353275189
