* DNSName.java:
91 if ((endIndex - startIndex) < 1)
No need for inner parentheses.
And, is there really a need to define alphaDigitsAndHyphen? How about just (==
'-' || inside alphaDigits)?
* DNSNameTest.java:
Space cannot appear anywhere, please add a test case like "a c.com".
BTW, I assume you want to reuse this test for other sub-tasks of JDK-8054380? I
see the @summary is more general.
No other comments.
Thanks
Max
> On Nov 22, 2018, at 12:51 AM, Seán Coffey <sean.cof...@oracle.com> wrote:
>
> p.s I've updated the testcase to test the IOException message for presence of
> "DNSName". Webrev updated in place.
>
> Regards,
> Sean.
>
> On 21/11/18 15:42, Seán Coffey wrote:
>> Thanks for the comments Bernd. Comments inline..
>>
>> On 16/11/18 21:27, Bernd Eckenfels wrote:
>>> Hello Sean,
>>>
>>> I was only looking at the inspected DNSName class, it still has some
>>> variations (but start now with DNSNames which is good already):
>>>
>>> 76 throw new IOException("DNSName must not be null or empty");
>>> 78 throw new IOException("DNSNames or NameConstraints with blank
>>> components are not permitted");
>>> 80 throw new IOException("DNSNames or NameConstraints may not begin or
>>> end with a .");
>>> 92 throw new IOException("DNSName SubjectAltNames with empty components
>>> are not permitted");
>>> 96 throw new IOException("DNSName components must begin with a letter or
>>> digit");
>>> 101 throw new IOException("DNSName components must consist of letters,
>>> digits, and hyphens");
>>>
>>> I did not check if those exceptions are catched and rethrown with something
>>> like „while validating the SubjectAltName Extension <num> of certificate
>>> <subject>...“, in my experience it helps if you do not have to find and
>>> review the actual certificates (in this case it might not be a problem if
>>> SAN is only in the end entity). You can probably remove „or
>>> NameConstraints“ and „SubjectAltNames“ from lines 78-92 (this is good if
>>> DNsNa
>> Ok - I've cleaned up the exception messages on line 78, 80, 92.
>> webrev updated in place :
>> http://cr.openjdk.java.net/~coffeys/webrev.8213952.v2/webrev/
>>
>>
>>> me applies to SAN or NameConstrained context and the validation logic does
>>> not know — so it’s not only more unified but also less missleading)
>>>
>>> BTW: probably not inthe scope of this fix but a subtype for validation
>>> errors which have getters for context/location and maybe error code and
>>> getValue() would allow callers to print nicer validation reports without
>>> relying on the message or Stacktraces.
>>
>> That's a nice idea and one that should be followed up in separate
>> enhancement. We could lean on the new `jdk.includeInExceptions` Security
>> property which other component areas have started to use lately.
>>
>> e.g. https://bugs.openjdk.java.net/browse/JDK-8207768
>>
>> regards,
>> Sean.
>>>
>>> Gruss
>>> Bernd
>>> --
>>> http://bernd.eckenfels.net
>>>
>>> Von: Seán Coffey <sean.cof...@oracle.com>
>>> Gesendet: Freitag, November 16, 2018 5:15 PM
>>> An: Bernd Eckenfels; security-dev@openjdk.java.net
>>> Betreff: Re: RFR: 8213952: Relax DNSName restriction as per RFC 1123
>>>
>>> Thanks for the comments Bernd. comments inline..
>>>
>>> On 16/11/18 12:40, Bernd Eckenfels wrote:
>>>> You could also add (a..b, false) and (.a, false), (a., false) to the
>>>> testcases.
>>> good point. test updated.
>>>>
>>>> I noticed that there are different types of Exception messages (DNS name,
>>>> DNSName, DNS Name or name constrained, DNS name and SAN), would be good if
>>>> all of them have the same keyword?
>>> I cleaned up some other references to DNSName in the sun.security.x509
>>> package. I'm not sure what classes you were referencing the above examples
>>> from.
>>>
>>> new webrev : http://cr.openjdk.java.net/~coffeys/webrev.8213952.v2/webrev/
>>>
>>> regards,
>>> Sean.
>>>>
>>>> Gruss
>>>> Bernd
>>>> --
>>>> http://bernd.eckenfels.net
>>>>
>>>> Von: security-dev <security-dev-boun...@openjdk.java.net> im Auftrag von
>>>> Seán Coffey <sean.cof...@oracle.com>
>>>> Gesendet: Freitag, November 16, 2018 12:25 PM
>>>> An: OpenJDK Dev list
>>>> Betreff: RFR: 8213952: Relax DNSName restriction as per RFC 1123
>>>>
>>>> Looking to make an adjustment to DNSName constructor to help comply with
>>>> RFC 1123
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8213952
>>>> http://cr.openjdk.java.net/~coffeys/webrev.8213952/webrev/
>>>>
>>>> regards,
>>>> Sean.
>>>>
>>>
>>
>
>
