Also reviewed SSLSocketFactoryImpl.java

BaseSSLSocketImpl.java
----------------------
I don't see where requireCloseNotify is actually being used in the remaining code. If you are removing this functionality, you should probably be calling that out in your CSR.

SSLSessionContextImpl.java
--------------------------
The only change here was to reorder the import of Vector (obsolete) and update the copyright? May I ask why? If this was somehow a netbeans suggestion, there were several other things that could be updated that seem more important. e.g. 37-39 could be made final. 207: unnecessary unboxing.

Brad




On 6/6/2018 5:45 PM, Bradford Wetmore wrote:
Today, I looked over SunJSSE.java, Utilities.java, and module-info.java.  Mostly nits below, some style things.


Utilities.java
--------------

39:  hexDigits can be private.

42:  Extra level of indent.  4 or 8.

39-41:  Nits: When variables are static finals, they are usually written uppercase.

115:  Extra " " and can we get SNI cap'd?

HostName for  server name indication
->
HostName for Server Name Indication

150:  Minor Nit:  You're concating ("+") and then using StringBuilder.append(), instead of simply append chaining which might be slightly more efficient.

167-217:  The code looks ok, but this might be reworked to take better advantage of StringBuilder.append() chaining, rather than the mix of concat and append here now.  e.g. what you did with toHexString(long). I see much of the same code repeated over and over here.  Can be done later.

Brad



On 6/3/2018 9:43 PM, Xuelei Fan wrote:
Hi,

Here it the 2nd full webrev:
   http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.01

and the delta update to the 1st webrev:
   http://cr.openjdk.java.net/~xuelei/8196584/webrev-delta.00/

Xuelei

On 5/25/2018 4:45 PM, Xuelei Fan wrote:
Hi,

I'd like to invite you to review the TLS 1.3 implementation.  I appreciate it if I could have compatibility and specification feedback before May 31, 2018, and implementation feedback before June 7, 2018.

Here is the webrev:
     http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.00

The formal TLS 1.3 specification is not finalized yet, although it had been approved to be a standard.  The implementation is based on the draft version 28:
     https://tools.ietf.org/html/draft-ietf-tls-tls13-28

For the overall description of this enhancement, please refer to JEP 332:
     http://openjdk.java.net/jeps/332

For the compatibility and specification update, please refer to CSR 8202625:
     https://bugs.openjdk.java.net/browse/JDK-8202625

Note that we are using the sandbox for the development right now. For more information, please refer to Bradford's previous email:

http://mail.openjdk.java.net/pipermail/security-dev/2018-May/017139.html

Thanks & Regards,
Xuelei

Reply via email to