Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

I just would like to remind that session resumption is a very important use case to support for ALPN. Understood. The ALPN value is tied to a handshake, either already completed and active (getApplicationProtocol()) or still in progress (getHandshakeApplicationProtocol()). Each handshake r

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

298: This test is not actually calling into checkResult on the server side. Ooops! You need to check the output of the wrap() before calling unwrap() as it overwrites the serverResult. You need to put in a similar checkResult() before doing the flip()s. So checks are required before and

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Thanks for the additional review comments. Responses in-line below. Updated webrev: http://cr.openjdk.java.net/~vinnie/8144093/webrev.02/ > On 1 Dec 2015, at 01:32, Bradford Wetmore wrote: > > > On 11/29/2015 4:08 PM, Vincent Ryan wrote: > > > Following on from Brad’s recent email, here is

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Hello Sean, An empty array is allowed: it means do not use ALPN. I’ve updated the exception messages to display the offending length in each case. --- ALPNExtension.java Tue Dec 1 15:22:02 2015 +++ ALPNExtension.java Tue Dec 1 14:56:12 2015 @@ -97,11 +97,13 @@ listLength = s.get

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Hey Vinnie, question on SSLParameters.setApplicationProtocols(String[] protocols) method What happens if you pass an empty array into this method. Shouldn't it throw an IllegalArgumentException ? In ALPNExtension.java : +if (listLength < 2 || listLength + 2 != len) { +

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Hi, On Mon, Nov 30, 2015 at 1:08 AM, Vincent Ryan wrote: > Hello, > > Following on from Brad’s recent email, here is the full webrev of the API > and the implementation classes for ALPN: > http://cr.openjdk.java.net/~vinnie/8144093/webrev.00/ > > In adds the implementation classes (sun/security

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Looks fine to me. Thanks for the update. Xuelei On 12/1/2015 7:08 AM, Vincent Ryan wrote: > Thanks for your review. Comments in-line. > >> On 30 Nov 2015, at 06:30, Xuelei Fan wrote: >> >> Looks fine to me. Just a few minor comments. >> >> ServerHandshaker.java >> = >> The

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

On 11/29/2015 4:08 PM, Vincent Ryan wrote: > Following on from Brad’s recent email, here is the full webrev of the > API and the implementation classes for ALPN: > http://cr.openjdk.java.net/~vinnie/8144093/webrev.00/ > > In adds the implementation classes (sun/security/ssl) to the public API >

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Thanks for the review. I’ve incorporated your comments and Xuelei’s comments in a revised webrev: http://cr.openjdk.java.net/~vinnie/8144093/webrev.01/ > On 30 Nov 2015, at 21:10, Sean Mullan wrote: > > SSLParameters.java > > 649

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Thanks for your review. Comments in-line. > On 30 Nov 2015, at 06:30, Xuelei Fan wrote: > > Looks fine to me. Just a few minor comments. > > ServerHandshaker.java > = > There is a typo of the first line. > - /* Copyright (c) 1996, 2015, ... > + /* > + * Copyright (c) 1996,

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

SSLParameters.java 649 applicationProtocols = protocols.clone(); You should clone the parameters before checking if they are valid. Move this to line 642, and check the validity of the cloned array. Also, use a temporary variable for the clone, so as not to pollute the applicationProt

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Looks fine to me. Just a few minor comments. ServerHandshaker.java = There is a typo of the first line. - /* Copyright (c) 1996, 2015, ... + /* + * Copyright (c) 1996, 2015 ... line 538-546 String negotiatedValue = null; List protocols = clientHelloALPN.g

Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

Hello, Following on from Brad’s recent email, here is the full webrev of the API and the implementation classes for ALPN: http://cr.openjdk.java.net/~vinnie/8144093/webrev.00/ In adds the implementation classes (sun/security/ssl) to the