On Mon, 20 Oct 2025 22:19:41 GMT, Anthony Scarpino <[email protected]> 
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 two 
> additional commits since the last revision:
> 
>  - fix non-pbe
>  - doc updates, zeroing, fix unencrypted keypair encoding, exception mods

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

> 1: /*

In the javadoc, I notice the `equals` and `hashCode` methods are marked final 
(which is done by the compiler I guess), but the other methods are not. Since 
records are always final, maybe it would be clearer to mark the other methods 
as `final` too.

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

> 296:      * @param str a String containing PEM data
> 297:      * @return a {@code DEREncodable}
> 298:      * @throws IllegalArgumentException on error in decoding or no PEM 
> data found

It looks like this method will throw `EOFException` and not 
`IllegalArgumentException` if there is no PEM data because it calls decode with 
a `ByteArrayInputStream` so uses that method which throws an `EOFException`.

Same comment for method that takes a `Class` argument.

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

> 282:      *                 stored in the new instance.
> 283:      * @return a new {@code PEMEncoder} instance configured for 
> encryption
> 284:      * @throws NullPointerException when password is {@code null}

s/when/if/

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

> 283:      * @return a new {@code PEMEncoder} instance configured for 
> encryption
> 284:      * @throws NullPointerException when password is {@code null}
> 285:      * @throws ProviderException if generating the encryption key fails.

No period needed at end.

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

> 283:      * @return a new {@code PEMEncoder} instance configured for 
> encryption
> 284:      * @throws NullPointerException when password is {@code null}
> 285:      * @throws ProviderException if generating the encryption key fails.

This actually throws an `IllegalArgumentException` if the key derivation fails.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2452395054
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2452972189
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2452790848
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2452794999
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2452948135

Reply via email to