On Fri, 2 May 2025 06:09:52 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 with a new target base due to a > merge or a rebase. The pull request now contains 66 commits: > > - major code review comments update > - Merge branch 'master' into pem > - Merge branch 'master' into pem > - javadoc updates > - code review comments > - merge with master > - better comment and remove commented out code > - Merge branch 'master' into pem > - Merge branch 'pem-merge' into pem > - merge > - ... and 56 more: https://git.openjdk.org/jdk/compare/e2ae50d8...0c540327 src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 79: > 77: > 78: /* PKCS8 version of the PEM */ > 79: protected int version; Do these need to be protected? It doesn't seem like any subclasses need them. src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 193: > 191: * > 192: * @param encoded the DER-encoded SubjectPublicKeyInfo value > 193: * @exception IOException on data format errors Change to `InvalidKeyException`. src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 243: > 241: } > 242: > 243: public byte[] getPrivKeyMaterial() { Do we really need this method, esp since it is the private key? I can't find any code that calls this. src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 305: > 303: } catch (IOException e) { > 304: // encodedKey is still null > 305: throw new SecurityException(e); Should return null instead since that is what `getEncoded()` specifies. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075978714 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075985424 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075990253 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075998393
