Code review of JDK-8072385, Only the first DNSName entry is checked for endpoint identification

[Jamil Nimeh]

Hi Xuelei, this looks good to me.

--Jamil

On 3/9/2015 10:52 PM, Xuelei Fan wrote:
> On 3/10/2015 12:27 PM, Jamil Nimeh wrote:
>> Hi Xuelei, I can't be an official reviewer, but I did look over the code
>> and it looks pretty good to me.
> I think it is OK to push if you reviewed the code.
>
>> I did have a couple questions though:
>>
>>    * Line 1570
>>        o Since the getSubjectAltNames() method is generic enough to take
>>          any int type it might be worthwhile to put some comments into
>>          the header stating that only those subjectAltName types that use
>>          String data should be passed into this function (see follow-on
>>          question below).
> Good point.  I added a note comment and updated the webrev.
>
>>    * Line 1580
>>        o If, for some reason, a maintainer were to ever pass in a
>>          different altname value, like 4 (X400Address), wouldn't the code
>>          cause a ClassCastException when trying to cast the returned
>>          byte[] to a String?  Would it be worthwhile to either do a
>>          instanceof check on the returned value before the cast or catch
>>          the CCE before it goes up the stack?  Or is it your desire to
>>          let the CCE be thrown?  I ask only because earlier in
>>          isIdentityEquivalent() you take the time to catch parsing
>>          exceptions and handle it with a log message rather than let it
>>          unwind the stack.  Would it be worthwhile to do something
>>          similar with a CCE down in getSubjectAltNames()?
>>
> It's better to check the instance for a good coding experiences.
>
> It's safe here as for subjectAltName type of DNS and IPAddress, the data
> type is String.  See the spec of
> X509Certificate.getSubjectAlternativeNames().
>
> We have the similar code in sun/security/util/HostnameChecker.java.
>
> Thanks,
> Xuelei
>
>> But even as-is, this still looks good to me.
>>
>> --Jamil
>>
>> On 03/09/2015 05:24 PM, Xuelei Fan wrote:
>>> Ping ...
>>>
>>> wbrev: http://cr.openjdk.java.net/~xuelei/8072385/webrev.00/
>>>
>>> On 3/4/2015 10:51 PM, Xuelei Fan wrote:
>>>> Hi,
>>>>
>>>> Please review the fix for:
>>>>     https://bugs.openjdk.java.net/browse/JDK-8072385
>>>>
>>>> In SunJSSE implementation, during endpoint identification, there is a
>>>> bug in SubjectAlternativeName matching that only the fist DNSName are
>>>> checked. As may impact some business where host-name alias are used.
>>>>
>>>> The patch is attached.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>