On Thu, 18 Sep 2025 14:25:14 GMT, Sean Mullan <[email protected]> wrote:
>> Anthony Scarpino has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> rework test & commented out code.
>
> src/java.base/share/classes/java/security/PEM.java line 63:
>
>> 61: * PRIVATE KEY, ENCRYPTED PRIVATE KEY, RSA PRIVATE KEY, or PUBLIC KEY.
>> 62: *
>> 63: * <p> {@code leadingData} may be null if no non-PEM data preceded PEM
>> header
>
> Suggest rewording as "... if there was no data preceding the PEM header ..."
>
> Why do you allow null when there is a ctor that does not have a `leadingData`
> parameter - is there a reason for this?
I thought it was more friendly to specify a constructor when `leadingData` was
known to be not needed. The other constructor is used by PEMDecoder decoding
is not known until decoding the stream occurs.
> src/java.base/share/classes/java/security/PEMEncoder.java line 265:
>
>> 263: * uses the default encryption parameters of the provider that is
>> selected.
>> 264: * For greater flexibility with encryption options and parameters,
>> use
>> 265: * {@link EncryptedPrivateKeyInfo#encryptKey(DEREncodable, Key,
>
> Line 256, did you mean to say "encoded" instead of "encrypted"? i.e.: "Only
> {@link PrivateKey} objects can be encoded with this newly ..."
>
> It makes more sense when you read the following sentence. Also the result is
> an encoding, the encryption is an intermediate step.
I agree that "encoded" makes more sense here given the following sentences says
encoded.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2377020526
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2377570024
