On Sun, 27 Apr 2025 21:33:06 GMT, Anthony Scarpino <ascarp...@openjdk.org>
wrote:
>> src/java.base/share/classes/java/security/PEMEncoder.java line 134:
>>
>>> 132: private String pemEncoded(PEMRecord pem) {
>>> 133: StringBuilder sb = new StringBuilder(1024);
>>> 134: sb.append("-----BEGIN ").append(pem.type()).append("-----");
>>
>> So you throw away the leading data? Shall we document this somewhere?
>
> 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.
>> src/java.base/share/classes/java/security/PEMRecord.java line 79:
>>
>>> 77: * @param pem The data between the PEM header and footer.
>>> 78: * @param leadingData Any non-PEM data read during the decoding
>>> process
>>> 79: * before the PEM header. This value maybe
>>> {@code null}.
>>
>> Do we need to say if `leadingData` contains the final newline char?
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2064118829
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2064116516
