RFR: 8298420: PEM API: Implementation (Preview) [v14]

[Mark Reinhold]

On Thu, 17 Apr 2025 15:51:09 GMT, Anthony Scarpino <ascarpino at 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