Xuelei,
Thanks for addressing my comments. This looks fine to me.
-Chris.
On 04/09/12 06:01, Xuelei Fan wrote:
webrev: http://cr.openjdk.java.net/~xuelei/7068321/webrev_spec.10/
On 9/3/2012 9:22 PM, Chris Hegarty wrote:
Xuelei,
This looks very good. Just a few minor comments:
- SNIServerName hexes could be UPPERCASE, since it is a constant.
Yes.
- Trivially, SNIHostName(String) calls IDN.toASCII(hostname) twice
It is not clear to me from this constructor whether it should
pass a hostname "as understood by the client", or an encoded
hostname. The method description seems to be at odds with the
implementation ( at least from me reading ). Maybe this could
be a little clearer by saying "as understood by the client".
Good suggestion. The spec is updated to make it clear that the hostname
can be a user-friendly Internationalized Domain Name (IDN).
- SNIHostName.hostname seems simply be a String version of the
SNIServerName.encoding. Also, there is no way of returning the
original hostname as passed in the constructor.
Generally, the encoded hostname (byte array) is come from the client
requested server name indication. In such a case, it is unknown whether
it is encoded in UTF-8 per RFC4366 (old spec for SNI), or ASCII per
RFC6066. We are not always able to easily get the original hostname
because of the encoding tolerant.
SNIHostName.getName() is mainly used in SNIMatcher to decide whether a
hostname is recognizable.
Revised the spec,and change the name of getName() method to getAsciiName
to make it more clear.
- SNIHostName.equals, Is it possible to craft a concrete SNIServerName
implementation that would be considered equal to a SNIHostName?
No, it is a coding error. Thanks!
It would seem that hostname may not be considered in the equality.
- There is scope for null parameter checking in the implementation
to use j.u.Objects.requireNonNull(Object,String)
Good, it makes some things easier.
- Is it possible to change SNIStandardTypes to use an enum, similar to
java.net.SocketOption & java.net.StandardSocketOptions, rather
than an int. It would still be extendable, but more "Java like".
We also need to parse unknown server name types. Using integer is more
straightforward.
Thanks,
Xuelei
-Chris.
On 03/09/2012 03:05, Xuelei Fan wrote:
Hi,
This is the spec review for JEP 114 [1].
webrev: http://cr.openjdk.java.net/~xuelei/7068321/webrev_spec.10/
Network team, per RFC 6066, the host_name in TLS SNI extension need to
be encoded in ASCII. In SNIHostName, to get the ASCII-Compatible
Encoding (ACE), java.net.IDN is used to convert from general String and
UTF-8 encoded byte array to ASCII string. We need expertise in
networking, would you please review the spec of SNIHostName?
Thanks,
Xuelei
[1]: http://openjdk.java.net/jeps/114
