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
This commit is contained in:
Eric Anderson 2017-06-05 12:35:04 -07:00
parent 9c6ea274fe
commit c48610b890
2 changed files with 11 additions and 16 deletions

View File

@ -381,13 +381,6 @@ public final class NettyChannelBuilder
case PLAINTEXT_UPGRADE: case PLAINTEXT_UPGRADE:
return ProtocolNegotiators.plaintextUpgrade(); return ProtocolNegotiators.plaintextUpgrade();
case TLS: case TLS:
if (sslContext == null) {
try {
sslContext = GrpcSslContexts.forClient().build();
} catch (SSLException ex) {
throw new RuntimeException(ex);
}
}
return ProtocolNegotiators.tls(sslContext, authority); return ProtocolNegotiators.tls(sslContext, authority);
default: default:
throw new IllegalArgumentException("Unsupported negotiationType: " + negotiationType); throw new IllegalArgumentException("Unsupported negotiationType: " + negotiationType);
@ -459,6 +452,13 @@ public final class NettyChannelBuilder
this.channelType = channelType; this.channelType = channelType;
this.negotiationType = negotiationType; this.negotiationType = negotiationType;
this.channelOptions = new HashMap<ChannelOption<?>, Object>(channelOptions); 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; this.sslContext = sslContext;
if (transportCreationParamsFilterFactory == null) { if (transportCreationParamsFilterFactory == null) {

View File

@ -143,16 +143,11 @@ public class NettyChannelBuilderTest {
@Test @Test
public void createProtocolNegotiator_tlsWithNoContext() { public void createProtocolNegotiator_tlsWithNoContext() {
ProtocolNegotiator negotiator = NettyChannelBuilder.createProtocolNegotiator( thrown.expect(NullPointerException.class);
NettyChannelBuilder.createProtocolNegotiator(
"authority:1234", "authority:1234",
NegotiationType.TLS, NegotiationType.TLS,
noSslContext); noSslContext);
assertTrue(negotiator instanceof ProtocolNegotiators.TlsNegotiator);
ProtocolNegotiators.TlsNegotiator n = (TlsNegotiator) negotiator;
assertEquals("authority", n.getHost());
assertEquals(1234, n.getPort());
} }
@Test @Test
@ -170,11 +165,11 @@ public class NettyChannelBuilderTest {
} }
@Test @Test
public void createProtocolNegotiator_tlsWithAuthorityFallback() { public void createProtocolNegotiator_tlsWithAuthorityFallback() throws SSLException {
ProtocolNegotiator negotiator = NettyChannelBuilder.createProtocolNegotiator( ProtocolNegotiator negotiator = NettyChannelBuilder.createProtocolNegotiator(
"bad_authority", "bad_authority",
NegotiationType.TLS, NegotiationType.TLS,
noSslContext); GrpcSslContexts.forClient().build());
assertTrue(negotiator instanceof ProtocolNegotiators.TlsNegotiator); assertTrue(negotiator instanceof ProtocolNegotiators.TlsNegotiator);
ProtocolNegotiators.TlsNegotiator n = (TlsNegotiator) negotiator; ProtocolNegotiators.TlsNegotiator n = (TlsNegotiator) negotiator;