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

[Anthony Scarpino]

On Fri, 9 May 2025 18:06:10 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> 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 66:
> 
>> 64:  * For decode methods that accept a {@code Class<S> tClass} as input, they can
>> 65:  * modify the return type to a specific {@code DEREncodable} subclass.
>> 66:  * For example, {@code ECPublicKey.class} can be used to cast a
> 
> The "modify" and "cast" words seem to reveal some implementation detail. It's just "parsing the input as the specified type".

If the cast is wrong, it throws a CastClassException.  I don't see why we need to hide this from the user.

> src/java.base/share/classes/java/security/PEMDecoder.java line 146:
> 
>> 144: 
>> 145:         try {
>> 146:             return switch (pem.type()) {
> 
> Do you still allow `type` being null?
> 
> `PEMDecoder.of().decode("s")` throws an NPE.

Hmm... I'm surprised it wouldn't trigger the default case or allow a case `null`

> src/java.base/share/classes/java/security/PEMDecoder.java line 407:
> 
>> 405:      * the default provider configuration.
>> 406:      *
>> 407:      * @param provider the factory provider.
> 
> This is a little awkward because the argument of `withFactory` is a provider. Shall we rename it?
> 
> Also, can we add some more description on how this method is used? For example, suppose a provider named `P1` extends `ECPublicKey` to `P1ECPublicKey`, then users should call `withFactory(p1).decode(pem, P1ECPublicKey.class)`. I assume we are not ready to do some kind of "delayed provider selection" trick to make it possible with the "original" decoder.

I think the javadoc explains the name clearly with links to what these factories do.

The class javadoc has details about using the decode class parameter and I don't think the javadoc should explaining how to use a 3rd party provider with 3rd party classes. 

I would wish this API to never use a delayed provider solution.

> src/java.base/share/classes/java/security/PEMRecord.java line 69:
> 
>> 67:  * @see PEMEncoder
>> 68:  */
>> 69: public record PEMRecord(String type, String pem, byte[] leadingData)
> 
> Add `@PreviewFeature`.

yep

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2083274944
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2083401700
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2083389219
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2083279739