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

[Anthony Scarpino]

On Thu, 24 Apr 2025 20:04:19 GMT, Mark Reinhold <mr at openjdk.org> wrote:

>> Anthony Scarpino has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - javadoc updates
>>  - code review comments
>
> 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" ?

Actually this comment isn't necessary as this method doesn't return Strings.

> 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/

ok

> 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.

What about the other decode methods that read from InputStream?  I thought it would be inconsistent to read `decode(String)` as a ISO 8859-1, but `decode(InputStream)` as a different charset.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2062695888
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2062696020
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2062701688