Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-16 Thread Vincent Ryan
I’ve made the change to use activated() rather than started() in SSLEngineImpl and SSLSocketImpl and also the suggestions for ServerHandshaker. I’ve also updated the tests to check SSLEngine/SSLSocket.getHandshakeApplicationProtocolSelector. All tests pass. Thanks for all the review comments.

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-15 Thread Xuelei Fan
On 12/15/2016 5:15 PM, Bradford Wetmore wrote: (Xuelei, question for you below) Hi Vinnie, There is no test yet for SSLEngine/SSLSocket: public BiFunction getHandshakeApplicationProtocolSelector() { Tests are passing at 100% with latest jdk.patch.

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-15 Thread Bradford Wetmore
(Xuelei, question for you below) Hi Vinnie, There is no test yet for SSLEngine/SSLSocket: public BiFunction getHandshakeApplicationProtocolSelector() { Tests are passing at 100% with latest jdk.patch. SSLSocketImpl.java == 2672:

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-15 Thread Vincent Ryan
Thanks Brad for those review comments. I’ve make some implementation changes and updated the existing ALPN tests. No public API changes. A new webrev is available at: http://cr.openjdk.java.net/~vinnie/8170282/webrev.02/ > On 9 Dec

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-09 Thread Bradford Wetmore
API looks good. SSLEngineImpl/SSLSocketImpl.java 212/229: I might suggest a more descriptive variable name, like applicationSelector. "selector" is a bit ambiguous. 450/1379: getHandshakeApplicationProtocolSelector()); -> selector); Xuelei wrote: > This

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-09 Thread Xuelei Fan
On 12/8/2016 6:09 PM, David M. Lloyd wrote: > How could it turn out to be incorrect, ... Here is some potentials regarding the cipher suite and application protocol: C1. select cipher suite at first, and then select the application protocol. There is an issue if there is no application protocol

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-09 Thread Vincent Ryan
Thanks for your detailed review Brad. I’ve generated a new webrev: http://cr.openjdk.java.net/~vinnie/8170282/webrev.01/ > On 9 Dec 2016, at 01:34, Bradford Wetmore wrote: > > > Hi Vinnie, > > On 12/8/2016 2:18 PM, Vincent Ryan wrote: >> The Java Servlet

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-09 Thread Simone Bordet
Hi, On Fri, Dec 9, 2016 at 3:09 AM, David M. Lloyd wrote: > On 12/08/2016 07:34 PM, Bradford Wetmore wrote: >> >> Hi, >> >> Vinnie wrote: >>> >>> We understood when we examined these issues last year that the >>> existing ALPN API would be sufficient. However it

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-08 Thread Bradford Wetmore
Hi, Vinnie wrote: > We understood when we examined these issues last year that the > existing ALPN API would be sufficient. However it transpired, > following HTTP2 server implementation efforts, that a particular > use-case was difficult to address without sacrificing performance. A few more

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-08 Thread Bradford Wetmore
Hi Vinnie, On 12/8/2016 2:18 PM, Vincent Ryan wrote: The Java Servlet Expect Group reported that they have identified a specific HTTP2 server use-case that cannot be easily addressed using the existing ALPN APIs. This changeset fixes that problem. It supports a new callback mechanism to

[9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-08 Thread Vincent Ryan
The Java Servlet Expect Group reported that they have identified a specific HTTP2 server use-case that cannot be easily addressed using the existing ALPN APIs. This changeset fixes that problem. It supports a new callback mechanism to allow TLS server applications to set an application protocol