On Thu, 14 May 2026 20:41:27 GMT, Weijun Wang <[email protected]> wrote:
>> Anthony Scarpino has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> comments
>
> src/java.base/share/classes/sun/security/util/Pem.java line 395:
>
>> 393: .getBytes(StandardCharsets.ISO_8859_1);
>> 394:
>> 395: int crlfLen = (base64.length == 0 ||
>
> Why is CRLF needed if `base64` is empty?
The spec says `base64text` and `strictbase64text` can be empty, but must have
an eol. `laxbase64text` allows no eol, but I think it's better to write the
default and strict modes rather than lax.
> src/java.base/share/classes/sun/security/util/Pem.java line 507:
>
>> 505: // In case decode() could not read the public key, the
>> 506: // KeyFactory can try. Failure is ok as there may not
>> 507: // be a public key in the encoding.
>
> Will this ever happen? It seems `new PKCS8Key(encoded)` can always find the
> public key even if it's inside a SEC1v2 format.
I don't think it's very likely, but it's in there to be safe. I had concerns
with third parties so it cannot hurt having extra coverage.
> src/java.base/share/classes/sun/security/util/Pem.java line 526:
>
>> 524:
>> 525: private static boolean matchesAt(byte[] source, int offset, byte[]
>> match) {
>> 526: if (offset < 0) {
>
> Will this happen?
no
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29640#discussion_r3249657447
PR Review Comment: https://git.openjdk.org/jdk/pull/29640#discussion_r3249869403
PR Review Comment: https://git.openjdk.org/jdk/pull/29640#discussion_r3249870085
