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

[Sean Mullan]

On Thu, 8 May 2025 20:40:28 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 three additional commits since the last revision:
> 
>  - comments
>  - toString update
>  - non-sealed
>    Better X509 KeyPair parsing

src/java.base/share/classes/java/security/PEMDecoder.java line 44:

> 42: 
> 43: /**
> 44:  * {@code PEMDecoder} is used for decoding Privacy-Enhanced Mail (PEM) data.

"is used" doesn't sound right. I would suggest changing "is used" to "implements a decoder" which is consistent with the `Base64.Decoder` class.

src/java.base/share/classes/java/security/PEMDecoder.java line 51:

> 49:  * and footer.
> 50:  *
> 51:  * <p> Decoding methods return an instance of a class that matches the data

You should say the specific decode methods that return `DEREncodable` objects, as you are not talking about the decode methods that take Class params here. Suggest: "The {@linkplain #decode(String)} and {@linkplain #decode(InputStream)} methods ..."

Also I think you can remove "unless otherwise specified" because the next sentence says what happens if the PEM does not have an API for it. So that is the only other case.

src/java.base/share/classes/java/security/PEMDecoder.java line 62:

> 60:  * <p> If the PEM does not have a JCE object representation, it returns a
> 61:  * {@link PEMRecord}. Any PEM can be decoded into a {@code PEMRecord} if the
> 62:  * class is specified.

I would move the second sentence to later after you descibe the `decode` methods that take a `Class` param.

src/java.base/share/classes/java/security/PEMDecoder.java line 64:

> 62:  * class is specified.
> 63:  *
> 64:  * For decode methods that accept a {@code Class<S> tClass} as input, they can

Start a new paragraph. Suggest rewording the beginning as "The `{@linkplain #decode(String, Class<S>) }` and `{@linkplain #decode(InputStream, Class<S>)}` methods take a `Class` parameter which determines the type of `DerEncodable` that is returned. These methods are useful when ..."

Then you can insert the sentence above somewhere towards the end of this paragraph: "Any type of PEM data can be decoded into a `{@code PEMRecord}` by specifying `{@code PEMRecord.class}` as a parameter."

src/java.base/share/classes/java/security/PEMDecoder.java line 79:

> 77:  * Configuring an instance for decryption does not prevent decoding with
> 78:  * unencrypted PEM. Any encrypted PEM that does not use the configured password
> 79:  * will throw an {@link RuntimeException}.

s/an/a/

src/java.base/share/classes/java/security/PEMDecoder.java line 93:

> 91: 
> 92:  * @apiNote
> 93:  * Here is an example of decoding a PrivateKey object:

Put `@code` around PrivateKey.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2081858947
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2081897903
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2081903041
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2081903359
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2081921762
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2081926584