On Thu, 8 May 2025 20:40:28 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>> Hi all, >> >> I need a code review of the PEM API. Privacy-Enhanced Mail (PEM) is a >> format for encoding and decoding cryptographic keys and certificates. It >> will be integrated into JDK24 as a Preview Feature. Preview features does >> not permanently define the API and it is subject to change in future >> releases until it is finalized. >> >> Details about this change can be seen at [PEM API >> JEP](https://bugs.openjdk.org/browse/JDK-8300911). >> >> Thanks >> >> Tony > > Anthony Scarpino has updated the pull request incrementally with three > additional commits since the last revision: > > - comments > - toString update > - non-sealed > Better X509 KeyPair parsing src/java.base/share/classes/java/security/PEMDecoder.java line 91: > 89: * > 90: * <p>This class is immutable and thread-safe. > 91: Missing `*`. src/java.base/share/classes/java/security/PEMDecoder.java line 131: > 129: * Returns an instance of {@code PEMDecoder}. > 130: * > 131: * @return returns a {@code PEMDecoder} you don't need to say "returns", just say "a `PEMDecoder`" src/java.base/share/classes/java/security/PEMDecoder.java line 190: > 188: getKey(password.getPassword()); > 189: } > 190: case Pem.CERTIFICATE, Pem.X509_CERTIFICATE -> { What about the "X.509 CERTIFICATE" header which is also mentioned in RFC 7468? src/java.base/share/classes/java/security/PEMDecoder.java line 191: > 189: } > 190: case Pem.CERTIFICATE, Pem.X509_CERTIFICATE -> { > 191: CertificateFactory cf = getCertFactory("X509"); Use "X.509". "X509" is an alias and may not be supported by other JDK implementations. Same comment on line 196. src/java.base/share/classes/java/security/PEMDecoder.java line 200: > 198: new > ByteArrayInputStream(decoder.decode(pem.pem()))); > 199: } > 200: case Pem.RSA_PRIVATE_KEY -> { Is it necessary to support this? It is not mentioned in RFC 7468. src/java.base/share/classes/java/security/PEMDecoder.java line 220: > 218: * the decoder. > 219: * > 220: * @param str a String containing PEM data. General style comment throughout APIs: no period necessary at end when `@param`, `@return`, or `@throws` starts with a non-capital letter and no sentence follows. src/java.base/share/classes/java/security/PEMDecoder.java line 223: > 221: * @return a {@code DEREncodable} generated from the PEM data. > 222: * @throws IllegalArgumentException on error in decoding or if the > PEM is > 223: * unsupported. If the PEM is unsupported, you return a `PEMRecord` now, so you can remove those words. Same comment on lines 248-249. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082244005 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082246420 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082255980 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082251527 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082258814 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082307742 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082312862
