fix ssl configuration issue for tcp and http clients (#369)

* make sure https is used in the uri factory if default secure * remove ssl if absolute http is used in a redirect * align server/client ssl conf accessor to expose SslProvider * expose removeSslSupport in SslProvider * fix BootstrapHandlers.findConfiguration for deferred confs
reactor · Jun 1, 2018 · 777ce3f · 777ce3f
1 parent 78d7706
commit 777ce3f
Show file tree

Hide file tree

Showing 15 changed files with 77 additions and 83 deletions.
diff --git a/src/main/java/reactor/netty/channel/BootstrapHandlers.java b/src/main/java/reactor/netty/channel/BootstrapHandlers.java
@@ -136,6 +136,9 @@ public static <C> C findConfiguration(Class<C> clazz,
 				if (clazz.isInstance(aRph.consumer)) {
 					return (C) aRph.consumer;
 				}
+				if (clazz.isInstance(aRph.deferredConsumer)) {
+					return (C) aRph.deferredConsumer;
+				}
 			}
 		}
 		return null;

diff --git a/src/main/java/reactor/netty/http/client/HttpClientConnect.java b/src/main/java/reactor/netty/http/client/HttpClientConnect.java
@@ -168,8 +168,8 @@ public ProxyProvider proxyProvider() {
 
 		@Nullable
 		@Override
-		public SslContext sslContext() {
-			return defaultClient.sslContext();
+		public SslProvider sslProvider() {
+			return defaultClient.sslProvider();
 		}
 	}
 
@@ -195,8 +195,17 @@ static final class MonoHttpConnect extends Mono<Connection> {
 		public void subscribe(CoreSubscriber<? super Connection> actual) {
 			final Bootstrap b = bootstrap.clone();
 
-			HttpClientHandler handler = new HttpClientHandler(b.config()
-			                                                   .remoteAddress(), configuration);
+			SslProvider ssl = SslProvider.findSslSupport(b);
+			if (ssl != null) {
+				SslProvider.updateSslSupport(b,
+						SslProvider.addHandlerConfigurator(ssl,
+								DEFAULT_HOSTNAME_VERIFICATION));
+			}
+
+			HttpClientHandler handler = new HttpClientHandler(configuration, b.config()
+			                                                                  .remoteAddress(), ssl);
+
+			b.remoteAddress(handler);
 
 			BootstrapHandlers.updateConfiguration(b,
 					NettyPipeline.HttpInitializer,
@@ -213,23 +222,25 @@ public void subscribe(CoreSubscriber<? super Connection> actual) {
 
 					});
 
-			b.remoteAddress(handler);
-
-			SslProvider ssl = SslProvider.findSslSupport(b);
-			if (ssl != null) {
-				SslProvider.updateSslSupport(b,
-						SslProvider.addHandlerConfigurator(ssl,
-								DEFAULT_HOSTNAME_VERIFICATION));
-			}
-
 			Mono.<Connection>create(sink -> {
 				Bootstrap finalBootstrap;
 				//append secure handler if needed
 				if (handler.activeURI.isSecure()) {
-					finalBootstrap = SslProvider.updateSslSupport(b.clone(), DEFAULT_HTTP_SSL_PROVIDER);
+					if (ssl == null) {
+						finalBootstrap = SslProvider.updateSslSupport(b.clone(),
+								DEFAULT_HTTP_SSL_PROVIDER);
+					}
+					else {
+						finalBootstrap = b.clone();
+					}
 				}
 				else {
-					finalBootstrap = b.clone();
+					if (ssl != null) {
+						finalBootstrap = SslProvider.removeSslSupport(b.clone());
+					}
+					else {
+						finalBootstrap = b.clone();
+					}
 				}
 
 				BootstrapHandlers.connectionObserver(finalBootstrap,
@@ -337,7 +348,7 @@ static final class HttpClientHandler extends SocketAddress
 		volatile Supplier<String>[] redirectedFrom;
 
 		@SuppressWarnings("unchecked")
-		HttpClientHandler(@Nullable SocketAddress address, HttpClientConfiguration configuration) {
+		HttpClientHandler(HttpClientConfiguration configuration, @Nullable SocketAddress address, @Nullable SslProvider sslProvider) {
 			this.method = configuration.method;
 			this.compress = configuration.acceptGzip;
 			this.followRedirect = configuration.followRedirect;
@@ -380,7 +391,7 @@ static final class HttpClientHandler extends SocketAddress
 			}
 
 			this.uriEndpointFactory =
-					new UriEndpointFactory(addressSupplier, false, URI_ADDRESS_MAPPER);
+					new UriEndpointFactory(addressSupplier, sslProvider != null, URI_ADDRESS_MAPPER);
 
 			this.websocketProtocols = configuration.websocketSubprotocols;
 			this.handler = configuration.body;

diff --git a/src/main/java/reactor/netty/tcp/SslProvider.java b/src/main/java/reactor/netty/tcp/SslProvider.java
@@ -106,7 +106,7 @@ public static Bootstrap updateSslSupport(Bootstrap b, SslProvider sslProvider) {
 	 */
 	@Nullable
 	public static SslProvider findSslSupport(Bootstrap b) {
-		SslSupportConsumer ssl = BootstrapHandlers.findConfiguration(SslSupportConsumer.class, b.config().handler());
+		DeferredSslSupport ssl = BootstrapHandlers.findConfiguration(DeferredSslSupport.class, b.config().handler());
 
 		if (ssl == null) {
 			return null;
@@ -131,6 +131,18 @@ public static SslProvider findSslSupport(ServerBootstrap b) {
 		return ssl.sslProvider;
 	}
 
+	/**
+	 * Remove Ssl support in the given client bootstrap
+	 *
+	 * @param b a bootstrap to search and remove
+	 *
+	 * @return passed {@link Bootstrap}
+	 */
+	public static Bootstrap removeSslSupport(Bootstrap b) {
+		BootstrapHandlers.removeConfiguration(b, NettyPipeline.SslHandler);
+		return b;
+	}
+
 	final SslContext                   sslContext;
 	final long                         handshakeTimeoutMillis;
 	final long                         closeNotifyFlushTimeoutMillis;
@@ -429,29 +441,6 @@ public interface SslContextSpec {
 
 	}
 
-	@Nullable
-	static SslContext findSslContext(Bootstrap b) {
-		SslSupportConsumer c =
-				BootstrapHandlers.findConfiguration(SslSupportConsumer.class,
-						b.config().handler());
-
-		return c != null ? c.sslProvider.getSslContext() : null;
-	}
-
-	@Nullable
-	static SslContext findSslContext(ServerBootstrap b) {
-		SslSupportConsumer c =
-				BootstrapHandlers.findConfiguration(SslSupportConsumer.class,
-						b.config().childHandler());
-
-		return c != null ? c.sslProvider.getSslContext() : null;
-	}
-
-	static Bootstrap removeSslSupport(Bootstrap b) {
-		BootstrapHandlers.removeConfiguration(b, NettyPipeline.SslHandler);
-		return b;
-	}
-
 	static ServerBootstrap removeSslSupport(ServerBootstrap b) {
 		BootstrapHandlers.removeConfiguration(b, NettyPipeline.SslHandler);
 		return b;

diff --git a/src/main/java/reactor/netty/tcp/TcpClient.java b/src/main/java/reactor/netty/tcp/TcpClient.java
@@ -328,7 +328,7 @@ public final boolean hasProxy(){
 	 * @return true if that {@link TcpClient} secured via SSL transport
 	 */
 	public final boolean isSecure(){
-		return sslContext() != null;
+		return sslProvider() != null;
 	}
 
 
@@ -506,14 +506,14 @@ public final TcpClient secure(SslContext sslContext) {
 	}
 
 	/**
-	 * Return the current {@link SslContext} if that {@link TcpClient} secured via SSL
+	 * Return the current {@link SslProvider} if that {@link TcpClient} secured via SSL
 	 * transport or null
 	 *
-	 * @return he current {@link SslContext} if that {@link TcpClient} secured via SSL
+	 * @return he current {@link SslProvider} if that {@link TcpClient} secured via SSL
 	 * transport or null
 	 */
 	@Nullable
-	public SslContext sslContext(){
+	public SslProvider sslProvider(){
 		return null;
 	}
 

diff --git a/src/main/java/reactor/netty/tcp/TcpClientConnect.java b/src/main/java/reactor/netty/tcp/TcpClientConnect.java
@@ -45,8 +45,7 @@ public Mono<? extends Connection> connect(Bootstrap b) {
 
 			TcpClientRunOn.configure(b,
 					LoopResources.DEFAULT_NATIVE,
-					TcpResources.get(),
-					SslProvider.findSslContext(b));
+					TcpResources.get());
 		}
 
 		return provider.acquire(b);

diff --git a/src/main/java/reactor/netty/tcp/TcpClientOperator.java b/src/main/java/reactor/netty/tcp/TcpClientOperator.java
@@ -19,7 +19,6 @@
 import javax.annotation.Nullable;
 
 import io.netty.bootstrap.Bootstrap;
-import io.netty.handler.ssl.SslContext;
 import reactor.core.publisher.Mono;
 import reactor.netty.Connection;
 
@@ -46,8 +45,8 @@ public Mono<? extends Connection> connect(Bootstrap b) {
 
 	@Override
 	@Nullable
-	public SslContext sslContext(){
-		return source.sslContext();
+	public SslProvider sslProvider(){
+		return source.sslProvider();
 	}
 
 	@Override

diff --git a/src/main/java/reactor/netty/tcp/TcpClientRunOn.java b/src/main/java/reactor/netty/tcp/TcpClientRunOn.java
@@ -17,12 +17,10 @@
 package reactor.netty.tcp;
 
 import java.util.Objects;
-import javax.annotation.Nullable;
 
 import io.netty.bootstrap.Bootstrap;
 import io.netty.channel.EventLoopGroup;
 import io.netty.handler.ssl.JdkSslContext;
-import io.netty.handler.ssl.SslContext;
 import reactor.netty.resources.LoopResources;
 
 /**
@@ -43,16 +41,19 @@ final class TcpClientRunOn extends TcpClientOperator {
 	public Bootstrap configure() {
 		Bootstrap b = source.configure();
 
-		configure(b, preferNative, loopResources, sslContext());
+		configure(b, preferNative, loopResources);
 
 		return b;
 	}
 
 	static void configure(Bootstrap b,
 			boolean preferNative,
-			LoopResources resources,
-			@Nullable SslContext sslContext) {
-		boolean useNative = preferNative && !(sslContext instanceof JdkSslContext);
+			LoopResources resources) {
+		SslProvider sslProvider =  SslProvider.findSslSupport(b);
+
+		boolean useNative = preferNative &&
+				(sslProvider == null || !(sslProvider.sslContext instanceof JdkSslContext));
+
 		EventLoopGroup elg = resources.onClient(useNative);
 
 		b.group(elg).channel(resources.onChannel(elg));

diff --git a/src/main/java/reactor/netty/tcp/TcpClientSecure.java b/src/main/java/reactor/netty/tcp/TcpClientSecure.java
@@ -21,7 +21,6 @@
 import javax.annotation.Nullable;
 
 import io.netty.bootstrap.Bootstrap;
-import io.netty.handler.ssl.SslContext;
 
 /**
  * @author Stephane Maldini
@@ -54,8 +53,8 @@ public Bootstrap configure() {
 	}
 
 	@Override
-	public SslContext sslContext(){
-		return this.sslProvider.getSslContext();
+	public SslProvider sslProvider(){
+		return this.sslProvider;
 	}
 
 }
diff --git a/src/main/java/reactor/netty/tcp/TcpClientUnsecure.java b/src/main/java/reactor/netty/tcp/TcpClientUnsecure.java
@@ -19,7 +19,6 @@
 import javax.annotation.Nullable;
 
 import io.netty.bootstrap.Bootstrap;
-import io.netty.handler.ssl.SslContext;
 
 /**
  * @author Stephane Maldini
@@ -37,7 +36,7 @@ public Bootstrap configure() {
 
 	@Override
 	@Nullable
-	public SslContext sslContext(){
+	public SslProvider sslProvider(){
 		return null;
 	}
 }
diff --git a/src/main/java/reactor/netty/tcp/TcpServer.java b/src/main/java/reactor/netty/tcp/TcpServer.java
@@ -335,7 +335,7 @@ public final TcpServer host(String host) {
 	 * @return true if that {@link TcpServer} secured via SSL transport
 	 */
 	public final boolean isSecure(){
-		return sslContext() != null;
+		return sslProvider() != null;
 	}
 
 	/**
@@ -501,14 +501,14 @@ public final <T> TcpServer selectorOption(ChannelOption<T> key, T value) {
 	}
 
 	/**
-	 * Return the current {@link SslContext} if that {@link TcpServer} secured via SSL
+	 * Return the current {@link SslProvider} if that {@link TcpServer} secured via SSL
 	 * transport or null
 	 *
-	 * @return he current {@link SslContext} if that {@link TcpServer} secured via SSL
+	 * @return he current {@link SslProvider} if that {@link TcpServer} secured via SSL
 	 * transport or null
 	 */
 	@Nullable
-	public SslContext sslContext() {
+	public SslProvider sslProvider() {
 		return null;
 	}
 

diff --git a/src/main/java/reactor/netty/tcp/TcpServerBind.java b/src/main/java/reactor/netty/tcp/TcpServerBind.java
@@ -49,10 +49,7 @@ public Mono<? extends DisposableServer> bind(ServerBootstrap b) {
 		if (b.config()
 		     .group() == null) {
 
-			TcpServerRunOn.configure(b,
-					LoopResources.DEFAULT_NATIVE,
-					TcpResources.get(),
-					SslProvider.findSslContext(b));
+			TcpServerRunOn.configure(b, LoopResources.DEFAULT_NATIVE, TcpResources.get());
 		}
 
 		return Mono.create(sink -> {

diff --git a/src/main/java/reactor/netty/tcp/TcpServerOperator.java b/src/main/java/reactor/netty/tcp/TcpServerOperator.java
@@ -19,7 +19,6 @@
 import javax.annotation.Nullable;
 
 import io.netty.bootstrap.ServerBootstrap;
-import io.netty.handler.ssl.SslContext;
 import reactor.core.publisher.Mono;
 import reactor.netty.DisposableServer;
 
@@ -46,7 +45,7 @@ public Mono<? extends DisposableServer> bind(ServerBootstrap b) {
 
 	@Override
 	@Nullable
-	public SslContext sslContext(){
-		return source.sslContext();
+	public SslProvider sslProvider(){
+		return source.sslProvider();
 	}
 }
diff --git a/src/main/java/reactor/netty/tcp/TcpServerRunOn.java b/src/main/java/reactor/netty/tcp/TcpServerRunOn.java
@@ -17,12 +17,10 @@
 package reactor.netty.tcp;
 
 import java.util.Objects;
-import javax.annotation.Nullable;
 
 import io.netty.bootstrap.ServerBootstrap;
 import io.netty.channel.EventLoopGroup;
 import io.netty.handler.ssl.JdkSslContext;
-import io.netty.handler.ssl.SslContext;
 import reactor.netty.resources.LoopResources;
 
 /**
@@ -43,16 +41,18 @@ final class TcpServerRunOn extends TcpServerOperator {
 	public ServerBootstrap configure() {
 		ServerBootstrap b = source.configure();
 
-		configure(b, preferNative, loopResources, sslContext());
+		configure(b, preferNative, loopResources);
 
 		return b;
 	}
 
 	static void configure(ServerBootstrap b,
 			boolean preferNative,
-			LoopResources resources,
-			@Nullable SslContext sslContext) {
-		boolean useNative = preferNative && !(sslContext instanceof JdkSslContext);
+			LoopResources resources) {
+		SslProvider sslProvider =  SslProvider.findSslSupport(b);
+
+		boolean useNative = preferNative &&
+				(sslProvider == null || !(sslProvider.sslContext instanceof JdkSslContext));
 		EventLoopGroup selectorGroup = resources.onServerSelect(useNative);
 		EventLoopGroup elg = resources.onServer(useNative);
 

diff --git a/src/main/java/reactor/netty/tcp/TcpServerSecure.java b/src/main/java/reactor/netty/tcp/TcpServerSecure.java
@@ -20,7 +20,6 @@
 import java.util.function.Consumer;
 
 import io.netty.bootstrap.ServerBootstrap;
-import io.netty.handler.ssl.SslContext;
 
 /**
  * @author Stephane Maldini
@@ -45,7 +44,7 @@ public ServerBootstrap configure() {
 	}
 
 	@Override
-	public SslContext sslContext() {
-		return this.sslProvider.getSslContext();
+	public SslProvider sslProvider() {
+		return this.sslProvider;
 	}
 }