RFR: 8360564: Implement JEP 524: PEM Encodings of Cryptographic Objects (Second Preview) [v4]

Fri Oct 3 19:18:11 UTC 2025

On Thu, 25 Sep 2025 23:03:11 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> Hi
>> 
>> Please review the [Second Preview](https://openjdk.org/jeps/8360563) for the PEM API.  The most significant changes from [JEP 470](https://openjdk.org/jeps/470) are:
>> 
>> - Renamed the name of `PEMRecord` class to `PEM`.
>> - Revised the new `encryptKey` methods of the `EncryptedPrivateKeyInfo` class to accept `DEREncodable` objects rather than just `PrivateKey` objects so that cryptographic objects with public keys, i.e., `KeyPair` and `PKCS8EncodedKeySpec`, can also be encrypted.
>> - Enhanced the `PEMEncoder` and `PEMDecoder` classes to support the encryption and decryption of `KeyPair` and `PKCS8EncodedKeySpec` objects.
>> 
>> thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
> 
>   missed some decoder comments

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

> 1: /*

Line 506: s/still/also/
508: s/decrypt/decrypt the/
510: s/PEMEncoder/PEMDecoder
511: put code around null

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

> 1: /*

Line 492: "Any errors using the {@code Provider} will occur during decoding."

What does that mean? Does it mean they are thrown by this method? If so, it should be declared as an exception.

Also: 495: s/to/with/
496: put code around null

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

> 84:  * <p> If the PEM type does not have a corresponding class,
> 85:  * {@code decode(String)} and {@code decode(InputStream)} will return a
> 86:  * {@link PEM}.

s/PEM/PEM object/

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

> 101:  *
> 102:  * <p> A new {@code PEMDecoder} instance is created when configured
> 103:  * with {@link #withFactory(Provider)} and/or

I think you can just say "or" instead of "and/or".

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

> 104:  * {@link #withDecryption(char[])}. The {@link #withFactory(Provider)} method
> 105:  * uses the specified provider to produce cryptographic objects from
> 106:  * {@link KeyFactory} and{@link CertificateFactory}.

space after "and".

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

> 105:  * uses the specified provider to produce cryptographic objects from
> 106:  * {@link KeyFactory} and{@link CertificateFactory}.
> 107:  * {@link #withDecryption(char[])} configures the decoder to process

Say "The withDecryption(char[]) method" so this is more visible as a new sentence.

Also: s/process/decode and decrypt/

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

> 107:  * {@link #withDecryption(char[])} configures the decoder to process
> 108:  * encrypted private key PEM data using the given password.
> 109:  * If decryption fails, a {@link IllegalArgumentException} is thrown.

s/a/an/

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

> 133:  * {@code X.509 CERTIFICATE}, {@code CRL}, and {@code RSA PRIVATE KEY}.
> 134:  *
> 135:  * @see PEMEncoder

line 126, I think the "." should be on this line.

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

> 337:      * a {@link PEM} is returned containing the
> 338:      * type identifier and Base64 encoding. Any non-PEM data preceding
> 339:      * the PEM header will be stored in {@code leadingData}.  Other

Most of my comments below on decode that takes an input stream also apply to this method.

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

> 372:      * the PEM footer or the end of the stream. If an I/O error occurs,
> 373:      * the read position in the stream may become inconsistent.
> 374:      * It is recommended to perform no further decoding operations

First sentence (line 367), change to: "Decodes and returns a DEREncodable of the specified class for the specified InputStream."

368: s/extend/extend or implement/

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

> 376:      *
> 377:      * <p> If the class parameter is {@code PEM.class},
> 378:      * a {@link PEM} is returned containing the

s/PEM/PEM object/

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

> 377:      * <p> If the class parameter is {@code PEM.class},
> 378:      * a {@link PEM} is returned containing the
> 379:      * type identifier and Base64 encoding. Any non-PEM data preceding

s/Base64 encoding/Base64-encoded data/

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

> 379:      * type identifier and Base64 encoding. Any non-PEM data preceding
> 380:      * the PEM header will be stored in {@code leadingData}.  Other
> 381:      * class parameters will not return preceding non-PEM data.

Suggest rewording as: "For `DEREncodable` types other than `PEM`, leading data is ignored and not returned as part of the `DEREncodable` object."

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

> 389:      *   {@code DEREncodable}.
> 390:      * @return a {@code DEREncodable} typecast to {@code tClass}
> 391:      * @throws IOException on IO or PEM syntax error where the

386: "extends or implements"

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

> 389:      *   {@code DEREncodable}.
> 390:      * @return a {@code DEREncodable} typecast to {@code tClass}
> 391:      * @throws IOException on IO or PEM syntax error where the

Why would bad PEM syntax be an IOE? Should this be an IAE, similar to a decoding error?

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

> 391:      * @throws IOException on IO or PEM syntax error where the
> 392:      * {@code InputStream} did not complete decoding.
> 393:      * @throws EOFException at the end of the {@code InputStream}

How about: "if the end of the input stream has been reached unexpectedly"

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

> 392:      * {@code InputStream} did not complete decoding.
> 393:      * @throws EOFException at the end of the {@code InputStream}
> 394:      * @throws IllegalArgumentException on error with arguments or in decoding

Can you be more specific here? What does "on error with arguments" mean? What type of decoding issues result in IAE?

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

> 393:      * @throws EOFException at the end of the {@code InputStream}
> 394:      * @throws IllegalArgumentException on error with arguments or in decoding
> 395:      * @throws ClassCastException if {@code tClass} is invalid for the PEM type

suggest: s/is invalid for/does not represent/

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

> 394:      * @throws IllegalArgumentException on error with arguments or in decoding
> 395:      * @throws ClassCastException if {@code tClass} is invalid for the PEM type
> 396:      * @throws NullPointerException when any input values are null

put code font around null

src/java.base/share/classes/java/security/PEMEncoder.java line 1:

> 1: /*

For withEncryption, suggest changing the first sentence to better match `PEMDecoder.withEncryption` which reads better to me. So suggest:

"Returns a copy of this PEMEncoder that encrypts and encodes private keys using the specified password and default encryption algorithm."

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2402961418
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2402979334
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403075672
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403081459
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403083470
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403085853
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403092375
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403098282
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403066797
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403007926
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403008593
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403015740
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403025383
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403032724
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403063120
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403047428
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403043967
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403040845
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2403038460
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2402952350