new webrev: http://cr.openjdk.java.net/~xuelei/8020842/webrev.05/
Added a new test to test illegal hostname in SNIHostName. Xuelei On 8/10/2013 10:49 AM, Xuelei Fan wrote: > 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 >>>>>>>>>>> >>>> >>> >> >