ProtocolNegotiators.TlsNegotiator overwrites existing SSLParameters

[vietj]

Please answer these questions before submitting your issue.

What version of gRPC are you using?

1.6.1

What JVM are you using (java -version)?

java version "9"
Java(TM) SE Runtime Environment (build 9+181)
Java HotSpot(TM) 64-Bit Server VM (build 9+181, mixed mode)

What did you do?

If possible, provide a recipe for reproducing the error.

I'm using a Netty ManagedChannelBuilder and I set it to use an SslContext configured for h2. Netty's SslContextBuilder will configure the Java9SslEngineWrapper that calls the SSLParameters#setApplicationProtocols with the {h2} array value to enable the client to use the ALPN extension. The ALPN extension is not sent by the client because the SSLParameters is overwritten by ProtocolNegotiators.TlsNegotiator and the existing SSLParameters are lost.

What did you expect to see?

I expect that the current SSLParameters to be preserved.

What did you see instead?

The client is not able to perform negotiate the h2 protocol during the TLS handshake.

Advocated fix

I installed locally a version of io.grpc:grpc-netty with the following change to ProtocolNegotiators#TlsNegotiator and it worked fine:

    @Override
    public Handler newHandler(GrpcHttp2ConnectionHandler handler) {
      ChannelHandler sslBootstrap = new ChannelHandlerAdapter() {
        @Override
        public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
          SSLEngine sslEngine = sslContext.newEngine(ctx.alloc(), host, port);
          SSLParameters sslParams = sslEngine.getSSLParameters();
          sslParams.setEndpointIdentificationAlgorithm("HTTPS");
          sslEngine.setSSLParameters(sslParams);
          ctx.pipeline().replace(this, null, new SslHandler(sslEngine, false));
        }
      };
      return new BufferUntilTlsNegotiatedHandler(sslBootstrap, handler);
    }

This implementation is more conservative and modifies the current SSLParameters which preserve the application protocols for Java 9.