(3rd Round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

[Xuelei Fan]

On 8/15/2012 8:31 AM, Brad Wetmore wrote:
> Still looking over the changes, but a few preliminary comments.
> 
> On 8/12/2012 5:50 AM, Xuelei Fan wrote:
>> Hi,
>>
>> Please review the spec of JEP 114, TLS Server Name Indication (SNI)
>> Extension.
>>
>>      http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/
>>
>> Please read the README to help you understanding the the specification:
>>
>>     http://cr.openjdk.java.net./~xuelei/7068321/README_04.txt
> 
> I forgot to mention this previously:
> 
>> Oracle provider now only supports one server name type, "host_name".
>> The value of name is encoded as UTF-8 string.
> 
> RFC 6066 requires ASCII, earlier versions (RFC 4366) have this as UTF-8.
>  Do you want to allow for UTF-8 here?
> 
Good point! I think we may send ASCII, but accept UTF-8. Need to
consider it more during implementation.

>> The major differences comparing with previous webrev are:
>> 1. client mode and server mode will use separated API set.
>>     For client, the related APIs are:
>>       setServerName(String type, String value)
>>       clearServerName(String type)
>>       disableServerName(String type)
>>       enableServerName(String type)
>>       isDisabledServerName(String type)
>>       getServerNames()
> 
> Please read my note on the 2nd round review before reading this,
> specifically the section that starts "I'm not following your confusion".
> 
> So wow, what happened here?  I thought we were so very close on
> finalizing the API, and it was just a matter of tweaking the wording to
> have a null value disable the type from being sent.
> 
>     setServerName(String type, String value) {
>         if type == value
>             throw IAE;
>         if value == null
>             map.remove(type);
>             return;
>         else if value.equals("")
>             throw IAE;
>         else
>             map.put(value);
>     }
> 
> Then in the Handshakers, only those type/values that are present in the
> map are be pulled out for constructing the SNI extensions.
> 
> If you want to go with this new API style with clear/disable/enable, I
> can see where you are coming from, but that was unexpected.  Before I
> review for accuracy, I want to make sure you really want to tweak things
> so extensively.  I think what you had previously fit the bill pretty well.
>
>>     For server side, the related APIs are:
>>       setServerNamePattern(String type, Pattern pattern)
>>       clearServerNamePattern(String type)
>>       getServerNamePatterns()
> 
> and same for this one.
> 
Sorry about that. It's not an easy trad-off for me between fewer APIs
and clearer APIs. The underlying motivation pushed me to extend the APIs
is that we may don't want the developers to find the usage of "null"
from java doc, and remember what the "null" value means. The new APIs
are more clear to me.

Sorry. No main framework changes, hope it is the final revision of the
APIs. ;-)

Thanks,
Xuelei

> I'll look over the rest once I get your answer to the above.
> 
>> 2. close the door to use the generated socket in client mode.
>>
>>     SSLSocketFactory.createSocket(Socket s,
>>         InputStream consumed, boolean autoClose)
>>
>>     The returned socket was set in server mode.
> 
> Brad
>