From 4d740a50dade9f96a2b38d5f22b4b13cb0c7ffa2 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Mon, 5 Jun 2017 12:35:04 -0700
Subject: [PATCH] netty: Eagerly create SslContext

Creating the SslContext can throw, generally due to broken ALPN. We want
that to propagate to the caller of build(), instead of within the
channel where it could easily cause hangs.

We still delay creation until actual build() time, since TLS is not
guaranteed to work and the application may be configuring plaintext or
similar later before calling build() where SslContext is unnecessary.

The only externally-visible change should be the exception handling.
I'd add a test, but the things throwing are static and trying to inject
them would be pretty messy.

Fixes #2599
---
 .../java/io/grpc/netty/NettyChannelBuilder.java    | 14 +++++++-------
 .../io/grpc/netty/NettyChannelBuilderTest.java     | 13 ++++---------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
index 5c19a28f2dc..07877085887 100644
--- a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
+++ b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
@@ -394,13 +394,6 @@ private static ProtocolNegotiator createProtocolNegotiatorByType(
       case PLAINTEXT_UPGRADE:
         return ProtocolNegotiators.plaintextUpgrade();
       case TLS:
-        if (sslContext == null) {
-          try {
-            sslContext = GrpcSslContexts.forClient().build();
-          } catch (SSLException ex) {
-            throw new RuntimeException(ex);
-          }
-        }
         return ProtocolNegotiators.tls(sslContext, authority);
       default:
         throw new IllegalArgumentException("Unsupported negotiationType: " + negotiationType);
@@ -472,6 +465,13 @@ private static final class NettyTransportFactory implements ClientTransportFacto
       this.channelType = channelType;
       this.negotiationType = negotiationType;
       this.channelOptions = new HashMap<ChannelOption<?>, Object>(channelOptions);
+      if (negotiationType == NegotiationType.TLS && sslContext == null) {
+        try {
+          sslContext = GrpcSslContexts.forClient().build();
+        } catch (SSLException ex) {
+          throw new RuntimeException(ex);
+        }
+      }
       this.sslContext = sslContext;
 
       if (transportCreationParamsFilterFactory == null) {
diff --git a/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java b/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java
index 9f03113dbe5..38d324a3878 100644
--- a/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java
+++ b/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java
@@ -158,16 +158,11 @@ public void createProtocolNegotiator_plaintextUpgrade() {
 
   @Test
   public void createProtocolNegotiator_tlsWithNoContext() {
-    ProtocolNegotiator negotiator = NettyChannelBuilder.createProtocolNegotiator(
+    thrown.expect(NullPointerException.class);
+    NettyChannelBuilder.createProtocolNegotiator(
         "authority:1234",
         NegotiationType.TLS,
         noSslContext);
-
-    assertTrue(negotiator instanceof ProtocolNegotiators.TlsNegotiator);
-    ProtocolNegotiators.TlsNegotiator n = (TlsNegotiator) negotiator;
-
-    assertEquals("authority", n.getHost());
-    assertEquals(1234, n.getPort());
   }
 
   @Test
@@ -185,11 +180,11 @@ public void createProtocolNegotiator_tlsWithClientContext() throws SSLException
   }
 
   @Test
-  public void createProtocolNegotiator_tlsWithAuthorityFallback() {
+  public void createProtocolNegotiator_tlsWithAuthorityFallback() throws SSLException {
     ProtocolNegotiator negotiator = NettyChannelBuilder.createProtocolNegotiator(
         "bad_authority",
         NegotiationType.TLS,
-        noSslContext);
+        GrpcSslContexts.forClient().build());
 
     assertTrue(negotiator instanceof ProtocolNegotiators.TlsNegotiator);
     ProtocolNegotiators.TlsNegotiator n = (TlsNegotiator) negotiator;