On Mon, 28 Apr 2025 17:18:05 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> I view this as an advanced feature for experienced users. The list is large >> and algorithm-dependent. For example an EC private key PEM could be >> PrivateKey.class, ECPrivateKey.class, PEMRecord.class, >> PKCS8EncodedKeySpec.class. I don't think it's realistic to list everything. > > I see. Maybe at least point out `PEMRecord` is always a valid option? This > gives people a chance to read arbitrary (even invalid) PEMs. That is documented in the next paragraph. >> This is a private method that just converts the `pem` and `type` into a >> properly formatted PEM. Leading data is not applicable here. > > Yes, this method is private. But you allow > `PEMEncoder().of().encode(PEMRecord)`. People might wonder why their leading > data is lost. Then they can call PEMRecord.leadingData(). >> I'm not sure what you mean by "final" as it contains whatever the stream >> contains before the dashes. The point is not to parse the non-PEM data and >> store it as is. > > I meant the newline char at the end (before the "------BEGIN" chars). I just > tried out your implementation, and noticed if there is nothing there, then > `leadingData` is null; and if there is a line of text, `leadingData` contains > the newline char at the end. > > I still think this is worth mentioning. Suppose someone wants to rewrite the > PEM file with the leading data, they need to know they should not use > `println`. > > BTW, I prefer trimming that newline char. Just my opinion. I see your point now, but `leadingData` is a byte array. If an app wrote the `leadingData` in a `ByteArrayInputStream`, followed by the PEM text, it would not be the same data written as the newline would be gone. Correct? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2069702691 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2069704157 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2069737448