On Mon, 3 Feb 2025 19:26:27 GMT, Ben Perez <[email protected]> wrote:
>> 8261513: Various BasicConstraintsExtension issues
>
> Ben Perez has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains three additional commits since
> the last revision:
>
> - Merge branch 'master' into JDK-8261513
> - Added IOException for negative pathLenConstraint
> - Changed constructor, getPathLen, and toString to return a canonical
> representation for unconstrained pathLens
A couple more comments. You'll need to add an appropriate `noreg` label to the
bug if you think it isn't practical to write a test for this.
src/java.base/share/classes/sun/security/x509/BasicConstraintsExtension.java
line 1:
> 1: /*
On line 186, it's questionable if we need to set the critical flag to the value
of the ca field. This was comment #6 in the bug report. RFC 5280 gives a few
cases where it is acceptable to have a non-critical BasicConstraintsExtension
with a ca field set to true. I would remove that and make sure all tests still
pass.
src/java.base/share/classes/sun/security/x509/BasicConstraintsExtension.java
line 167:
> 165: String pathLenAsString;
> 166: if (pathLen < 0 || pathLen == Integer.MAX_VALUE) {
> 167: pathLenAsString = " unconstained";
Typo: s/unconstained/unconstrained/
But I actually prefer the words "no limit" as that is what [RFC 5280, section
4.2.1.9](https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.9) uses, so
please restore those words. You can use that term for the "undefined" case as
well.
-------------
PR Review: https://git.openjdk.org/jdk/pull/20224#pullrequestreview-2627976730
PR Review Comment: https://git.openjdk.org/jdk/pull/20224#discussion_r1962385195
PR Review Comment: https://git.openjdk.org/jdk/pull/20224#discussion_r1962370853
