On Tue, 14 Oct 2025 03:55:20 GMT, Koushik Muthukrishnan Thirupattur
<[email protected]> wrote:
>> Anthony Scarpino has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> updates
>
> src/java.base/share/classes/java/security/DEREncodable.java line 58:
>
>> 56: public sealed interface DEREncodable permits AsymmetricKey, KeyPair,
>> 57: PKCS8EncodedKeySpec, X509EncodedKeySpec, EncryptedPrivateKeyInfo,
>> 58: X509Certificate, X509CRL, PEM {
>
> Do we need to add any javadoc note here stating why PEM replaces PEMRecord to
> help understand intent?
No, that is for the JEP and/or CSR to explain.
> src/java.base/share/classes/java/security/PEMDecoder.java line 232:
>
>> 230: }
>> 231: byte[] p8 = Pem.decryptEncoding(
>> 232: decoder.decode(pem.content()),
>> password.getPassword());
>
> Should we need a null check for pem and a short defensive check for
> pem.content() being empty/blank before attempting decode? Also do we
> explicitly specify why explicit zeroing is used ?
PEM disallows a null 'context'. As for empty values, I purposefully let Base64
decoder do those checks.
> src/java.base/share/classes/java/security/PEMDecoder.java line 233:
>
>> 231: byte[] p8 = Pem.decryptEncoding(
>> 232: decoder.decode(pem.content()),
>> password.getPassword());
>> 233: DEREncodable d = Pem.toDEREncodable(p8, true,
>> factory);
>
> If toDEREncodable may throw, do we need to guarantee zeroing on all paths in
> case of success or exception?
Yes, that is was changed as part of a different code change.
> src/java.base/share/classes/java/security/PEMEncoder.java line 286:
>
>> 284: * privateKeyEncoding will be zeroed when the method returns
>> 285: */
>> 286: private String buildKey(byte[] publicEncoding, byte[]
>> privateEncoding) {
>
> Do we need to explicitly document argument contracts: which combination
> corresponds to which PEM types here ?
This is an internal method, so nothing needs to be documented. As far as PEM
types and their DEREncodables, that is done at the class-level javadoc
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2430766713
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2430780389
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2430782537
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2430782856
