On Tue, 13 May 2025 09:27:37 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 one > additional commit since the last revision: > > comments on the 11th src/java.base/share/classes/java/security/PEMRecord.java line 42: > 40: * {@code DEREncodable} for the type. > 41: * > 42: * <p>{@code PEMRecord} may have a null {@code type} and {@code pem} when I think this sentence can be removed now that non-PEM data is considered an error. src/java.base/share/classes/java/security/PEMRecord.java line 48: > 46: * > 47: * <p> During the instantiation of this record, there is no validation > for the > 48: * {@code type} or {@code pem}. The ctors throw `IllegalArgumentException` though. I think you need to be more specific about what type of validation you mean here. src/java.base/share/classes/java/security/PEMRecord.java line 76: > 74: * {@code null} if there is no PEM data. > 75: * @param pem the data between the PEM header and footer. > 76: * @param leadingData Any non-PEM data read during the decoding > process s/Any/any/ src/java.base/share/classes/java/security/PEMRecord.java line 77: > 75: * @param pem the data between the PEM header and footer. > 76: * @param leadingData Any non-PEM data read during the decoding > process > 77: * before the PEM header. This value maybe {@code > null}. s/maybe/may be/ src/java.base/share/classes/java/security/PEMRecord.java line 87: > 85: if (type == null && pem != null || type != null && pem == null) { > 86: throw new IllegalArgumentException("\"type\" and \"pem\" must > be" + > 87: " both null or non-null"); If type and pem can be both null, and leadingData is also null (which is allowed per spec) then that should not be an error? This check may need to be updated now that non-PEM data is an error. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2087677034 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2087681167 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2087680087 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2087680232 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2087684345
