On Thu, 17 Apr 2025 15:51:09 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 two > additional commits since the last revision: > > - javadoc updates > - code review comments Changes requested by mr (Lead). src/java.base/share/classes/java/security/PEMDecoder.java line 78: > 76: * > 77: * <p> {@code String} values returned by this class use character set > 78: * {@link java.nio.charset.StandardCharsets#ISO_8859_1 ISO-8859-1} `String` values in Java are always encoded in UTF-16. Did you mean to write something like, "Byte streams consumed by methods in this class are assumed to represent characters encoded in the ISO-8859-1 charset" ? src/java.base/share/classes/java/security/PEMDecoder.java line 84: > 82: * <pre> > 83: * PEMDecoder pd = PEMDecoder.of(); > 84: * PrivateKey priKey = pd.decode(PriKeyPEM); s/PriKeyPEM/priKeyPEM/ src/java.base/share/classes/java/security/PEMDecoder.java line 213: > 211: Objects.requireNonNull(str); > 212: try { > 213: return decode(new ByteArrayInputStream(str.getBytes())); `str.getBytes()` will return a byte array encoded in the default charset, which these days is likely to be UTF-8, but might be something completely bizarre. You probably want `str.getBytes(StandardCharsets.ISO_8859_1)`. It could be worth running your unit tests in several different locales in order to catch similar issues. src/java.base/share/classes/java/security/PEMEncoder.java line 74: > 72: * > 73: * <p>{@code String} values returned by this class use character set > 74: * {@link java.nio.charset.StandardCharsets#ISO_8859_1 ISO-8859-1}. As above, did you mean to write something like, "Byte arrays returned by methods in this class represent characters encoded using the ISO-8859-1 charset" ? ------------- PR Review: https://git.openjdk.org/jdk/pull/17543#pullrequestreview-2792362333 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2059147360 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2059147944 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2059152266 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2059157190
