Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot

[Xuelei Fan]

Hi Michael,

It is pretty hard to get the issue solved in SNIHostName in a good
sharp.  Here is my try to state why we should fix the issue in IDN.

In SNIHostName, the following hostname are not accepted as valid hostname:
1. empty hostname
2. hostname ends with a trailing dot
3. hostname does not comply to RFC 3490.

The process in SNIHostName looks like:
1. call IDN.toASCII() to convert a string hostname
2. check that the return value of #1 is an valid hostname (non-empty,
non-end-with-tailing-dot).

At present, the IDN cannot handle the following IDN properly.
1. returns "com" for "com."
   the trailing dot is swallowed.

2. throws StringIndexOutOfBoundsException for "."
    If "." is an valid IDN that comply to RFC 3490, IDN.toASCII() should
be able to handle it; otherwise, IDN.toASCII() should throw IAE as the
specification suggested. However, IDN.toASCII(".") throws
StringIndexOutOfBoundsException, this behavior does not comply the the
specification:

3. throws StringIndexOutOfBoundsException for "example...net"
   As #2.

We can address #1 and #2 in SNIHostName, but the checking is overloaded
as IDN also need to address the issue. And SNIHostName has to know
what's the separators (".", "\u3002, etc) of IDN in order to check the
dot character. It is not a good encapsulation, and involved in too much
about the details of domain name, I think.

It is a little big hard to address #3 in SNIHostName.

Both all of above issue can be easily addressed in IDN.  And once IDN
addressed these issues, the current SNIHostName is able to handle
invalid hostname (empty, trailing dot, etc) correctly.  We won't need to
touch SNIHostName any more.

Please consider it.

The latest webrev is at:
http://cr.openjdk.java.net/~xuelei/8020842/webrev.02/

Thanks,
Xuelei

On 8/10/2013 9:13 AM, Xuelei Fan wrote:
> Hi Michael,
> 
> I plan to address this issue in SNIHostName.  I have filled another two
> the potential bugs in IDN.
> 
> Thank you, and other people, for the feedback.
> 
> Thanks,
> Xuelei
> 
> On 8/9/2013 11:25 PM, Xuelei Fan wrote:
>> On 8/9/2013 7:31 PM, Michael McMahon wrote:
>>> I don't see how this fixes the original problem as the SNIHostName spec
>>> still doesn't like hostnames with a trailing '.'
>>>
>> The bug description did not reflect the IDN specification correctly.  If
>> "com." is a valid IDN, SNIHostName should accept it an an valid
>> hostname.  The host name in SNIHostName is nothing more or less than an
>> standard IDN.
>>
>> I added a comment in the bug: "com." and "." are valid IDN according the
>> IDN and domain name specifications.  I will contact the bug reporter
>> about this point.
>>
>> Xuelei
>>
>>> I'd prefer to check first where that requirement is coming from, if it is
>>> actually necessary, and if not consider removing it from SNIHostName.
>>> If it is necessary, then the check should be implemented in SNIHostName.
>>>
>>> Michael
>>>
>>> On 09/08/13 05:28, Xuelei Fan wrote:
>>>> Thanks for your feedback and suggestions.
>>>>
>>>> Here is the new webrev:
>>>>     http://cr.openjdk.java.net/~xuelei/8020842/webrev.02/
>>>>
>>>> "." is regarded as valid IDN in this update.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 8/9/2013 10:50 AM, Xuelei Fan wrote:
>>>>> On 8/9/2013 10:14 AM, Weijun Wang wrote:
>>>>>>
>>>>>> On 8/9/13 9:37 AM, Xuelei Fan wrote:
>>>>>>> On 8/9/2013 9:22 AM, Weijun Wang wrote:
>>>>>>>> I tried nslookup. Those with ".." inside are illegal,
>>>>>>>>
>>>>>>>> $ nslookup com..
>>>>>>>> nslookup: 'com..' is not a legal name (empty label)
>>>>>>>>
>>>>>>>> but
>>>>>>>>
>>>>>>>> $ nslookup .
>>>>>>>> Server:        192.168.10.1
>>>>>>>> Address:    192.168.10.1#53
>>>>>>>>
>>>>>>>> Non-authoritative answer:
>>>>>>>> *** Can't find .: No answer
>>>>>>>>
>>>>>>> Thanks for the testing.  The behaviors are the same as this fix now.
>>>>>> No exactly. It seems nslookup still regards "." legal but just cannot
>>>>>> find an IP for it.
>>>>>>
>>>>> I'm not sure whether a root domain name can be stand alone.  Root label
>>>>> is not considered as a label in IDN.  I think it is safe to regard that
>>>>> "." is not a valid IDN as it contains no label.  Anyway, it is a corner
>>>>> case.
>>>>>
>>>>> There are many online IDN conversion web services, some of them can
>>>>> convert ".", some of the cannot.  In the present implementation, we
>>>>> cannot recognize ".", and IDN.toASCII(".") throws
>>>>> StringIndexOutOfBoundsException.  With this fix, I was wondering IAE is
>>>>> a better exception for IDN.toASCII(".").
>>>>>
>>>>>>> Learn something new today to use nslookup.
>>>>>>>
>>>>>>>> Also, since this bug was originally about SNIHostName, do you need to
>>>>>>>> add some extra restriction there to reject "oracle.com." things?
>>>>>>>>
>>>>>>> No, we cannot restrict the format of IDN in SNIHostName more than in
>>>>>>> IDN. However, we may need to rethink about the comparing of two
>>>>>>> IDN, for
>>>>>>> example, "example.com." should equal to "example.com".  I want to
>>>>>>> consider it in another bug.
>>>>>> Not sure. Does the spec say IDN and SNIHostName are equivalent sets?
>>>>>> And
>>>>>> it's not one is another's subset?
>>>>>>
>>>>> Per TLS specification, host name in SNI is an IDN.  The spec of
>>>>> SNIHostname says, "hostname is not a valid Internationalized Domain Name
>>>>> (IDN) compliant with the RFC 3490 specification". The spec in
>>>>> SNIHostName has the same means as IDN.  I won't want to add additional
>>>>> restrict beyond the specification of an IDN.
>>>>>
>>>>> Xuelei
>>>>>
>>>>>>> Can I push the changeset?
>>>>>> I think it's better to ask someone in the networking team to make the
>>>>>> suggestion. From what I read Michael in this thread, he does not seem
>>>>>> totally agreed with your code changes (at least not the 00 version).
>>>>>>
>>>>>> Thanks
>>>>>> Max
>>>>>>
>>>>>>> Thanks,
>>>>>>> Xuelei
>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Max
>>>>>>>>
>>>>>>>> On 8/9/13 8:41 AM, Xuelei Fan wrote:
>>>>>>>>> Ping.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Xuelei
>>>>>>>>>
>>>>>>>>> On 8/7/2013 11:17 PM, Xuelei Fan wrote:
>>>>>>>>>> Please review the new update:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/
>>>>>>>>>>
>>>>>>>>>> With this update, "com." is valid (return "com."); "." and
>>>>>>>>>> "example..com" are invalid.  And IAE will be thrown for invalid
>>>>>>>>>> IDN.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Xuelei
>>>>>>>>>>
>>>
>>
>