On Thu, 23 Oct 2025 13:59:29 GMT, Weijun Wang <[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 31 commits: >> >> - merge >> - stragglers >> - checkpoint >> - remaining comments >> - more review comments from Sean and Weijun >> - more review comments from Weijun and Sean >> - another day another iteration >> - move algorithm-specific code into MacData and no change to SunJCE >> - fix behavior with keytool >> - default salt length and one other comment from Weijun >> - ... and 21 more: https://git.openjdk.org/jdk/compare/d8ebe387...8f0b0d02 > > src/java.base/share/classes/com/sun/crypto/provider/PBES2Parameters.java line > 244: > >> 242: } >> 243: DerValue pBKDF2_params = kdf.data.getDerValue(); >> 244: if (pBKDF2_params.tag != DerValue.tag_Sequence) { > > This check seems like it should belong to the `PBKDF2Parameters` constructor. fixed > src/java.base/share/classes/sun/security/pkcs12/MacData.java line 77: > >> 75: private final int keyLength; >> 76: private final String kdfHmac; >> 77: private final String hmac; > > Add a comment that the 3 fields above are only for PBMAC1. done > src/java.base/share/classes/sun/security/pkcs12/MacData.java line 135: > >> 133: } >> 134: >> 135: private static byte[] getMac(String macAlgorithm, char[] password, > > Why the difference in the names `getMac` and `calculateMac`? > > This method is the basic one for calculation, and `processMacData` is > verification and `calculateMac` is generation. Add more comments. What about this: `getMac` -> `calculateMac` `calculateMac` -> `generateMac` `processMacData` ->`verifyMac` I'll add more comments. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2456039764 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2456039311 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2456038719
