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

[Anthony Scarpino]

On Thu, 25 Jul 2024 15:42:59 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Anthony Scarpino has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - bad test check
>>  - internal PEMRecord optimization
>
> src/java.base/share/classes/java/security/Key.java line 1:
> 
>> 1: /*
> 
> This file is not modified.

Same as EKS :(

> src/java.base/share/classes/java/security/PEMEncoder.java line 88:
> 
>> 86: 
>> 87:     // If non-null, encoder is configured for encryption
>> 88:     private Cipher cipher = null;
> 
> Is it worth creating `cipher` lazily? Also, why does `PEMDecoder` allows setting a factory but not here?

if cipher is defined this is an encrypted PEMEncoder instance, so yes it's important.
Encoding doesn't need a factory when the object already provides the DER encoding.

> src/java.base/share/classes/java/security/spec/EncodedKeySpec.java line 32:
> 
>> 30: 
>> 31: import java.io.IOException;
>> 32: import java.security.DEREncodable;
> 
> Useless import.

Yes, probably change that didn't get completely undone

> src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 109:
> 
>> 107:         throws InvalidKeyException {
>> 108:         this(privEncoding);
>> 109:         pubKeyEncoded = pubEncoding;
> 
> So if there is already a public key in `privEncoding`, it will be overwritten? BTW, it seems this method is not used anywhere.

If it isn't used anywhere, then it's probably from an old idea that I didn't completely clean up

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r1692459154
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r1692458145
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r1692458787
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r1692461176