On Thu, 15 May 2025 14:57:53 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Anthony Scarpino has updated the pull request with a new target base due to >> a merge or a rebase. The pull request now contains 76 commits: >> >> - merge in JEP 513 >> - Merge branch 'master' into pem >> - comments >> - comments >> - comments >> - comments on the 11th >> - comments on the 11th >> - comments >> - toString update >> - non-sealed >> Better X509 KeyPair parsing >> - ... and 66 more: https://git.openjdk.org/jdk/compare/5e50a584...8bf36d6b > > src/java.base/share/classes/java/security/PEMDecoder.java line 55: > >> 53: * type and implements {@link DEREncodable}. >> 54: * >> 55: * The following lists the supported PEM types and the {@code >> DEREncodable} > > The list seems too early before introducing the decode-with-class methods. This is where the other list was that showed the PEM types. This is a good spot when talking about cryptographic objects and how them and PEMRecord relate > src/java.base/share/classes/java/security/PEMDecoder.java line 64: > >> 62: * <li>PRIVATE KEY : {@code PrivateKey}</li> >> 63: * <li>PRIVATE KEY : {@code PKCS8EncodedKeySpec} (Only supported when >> passed as a {@code Class} parameter)</li> >> 64: * <li>PRIVATE KEY : {@code KeyPair} (if the encoding also contains a >> public key)</li> > > In a later paragraph you talk about reading `PublicKey` from a `PRIVATE KEY` > if it is 2.0 and contains the public key. How about merging that info into > this list? I want to keep the table simple. > src/java.base/share/classes/java/security/PEMDecoder.java line 121: > >> 119: * } >> 120: * >> 121: * @implNote An implementation may support other PEM types and >> DEREncodables. > > Have you considered moving the whole decoding list above into this > `@implNote`? Same question with the encoder side. The list is part of the spec and needs to not be in the note ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2091770646 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2091767753 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2091772197