On Tue, 22 Nov 2022 20:09:02 GMT, Sean Mullan <mul...@openjdk.org> wrote:
> A general comment is that since we are adding checks for illegal values to
> the `*Extension` classes, we should probably go one step further and do the
> same for all the classes in `sun.security.x509` package. I'm ok if you want
> to handle this as a separate issue though.
I see. I'll work on making `GeneralNames` immutable within this issue. The
`NameConstraintsExtension` and `GeneralSubtrees` case is complicated where
mutability is still needed by the merge action.
> src/java.base/share/classes/sun/security/x509/Extension.java line 178:
>
>> 176:
>> 177: Objects.requireNonNull(extensionId,
>> 178: "Null OID to encode for the extension!");
>
> I would probably drop the exclamation points. And change "Null OID" to "No
> OID".
OK.
> src/java.base/share/classes/sun/security/x509/PolicyMappingsExtension.java
> line 78:
>
>> 76: * Create a PolicyMappings with the List of CertificatePolicyMap.
>> 77: *
>> 78: * @param maps the List of CertificatePolicyMap, cannot be null or
>> empty.
>
> Do you want to add similar null checks to the `CertificatePolicyMap` ctor?
Yes.
> src/java.base/share/classes/sun/security/x509/SubjectInfoAccessExtension.java
> line 89:
>
>> 87: if (accessDescriptions == null || accessDescriptions.isEmpty()) {
>> 88: throw new IllegalArgumentException(
>> 89: "AccessDescription cannot be null or empty");
>
> s/AccessDescription/accessDescriptions/
OK.
-------------
PR: https://git.openjdk.org/jdk/pull/11137
