Rework a bit the HttpClient SNI support

src/main/asciidoc/dataobjects.adoc

@@ -826,6 +826,10 @@ Set true when the client wants to skip frame masking.
  You may want to set it true on server by server websocket communication: In this case you are by passing RFC6455 protocol.
  It's false as default.
 +++
+|[[sni]]`sni`|`Boolean`|
++++
+Set wether the client should present the request host with the Server Name Indication when using TLS/SSL connections.
++++
 |[[soLinger]]`soLinger`|`Number (int)`|
 +++
 Set whether SO_linger keep alive is enabled
@@ -1841,10 +1845,6 @@ Set the host name to be used by the client request.
 +++
 Set the port to be used by the client request.
 +++
-|[[serverName]]`serverName`|`String`|
-+++
-Set the SNI server name to be used by the client connection.
-+++
 |[[ssl]]`ssl`|`Boolean`|
 +++
 Set whether SSL/TLS is enabled

src/main/generated/io/vertx/core/http/HttpClientOptionsConverter.java

@@ -95,6 +95,9 @@ public static void fromJson(JsonObject json, HttpClientOptions obj) {
     if (json.getValue("sendUnmaskedFrames") instanceof Boolean) {
       obj.setSendUnmaskedFrames((Boolean)json.getValue("sendUnmaskedFrames"));
     }
+    if (json.getValue("sni") instanceof Boolean) {
+      obj.setSni((Boolean)json.getValue("sni"));
+    }
     if (json.getValue("tryUseCompression") instanceof Boolean) {
       obj.setTryUseCompression((Boolean)json.getValue("tryUseCompression"));
     }
@@ -135,6 +138,7 @@ public static void toJson(HttpClientOptions obj, JsonObject json) {
       json.put("protocolVersion", obj.getProtocolVersion().name());
     }
     json.put("sendUnmaskedFrames", obj.isSendUnmaskedFrames());
+    json.put("sni", obj.isSni());
     json.put("tryUseCompression", obj.isTryUseCompression());
     json.put("verifyHost", obj.isVerifyHost());
   }

src/main/java/io/vertx/core/http/HttpClientOptions.java

@@ -144,6 +144,11 @@ public class HttpClientOptions extends ClientOptionsBase {
    */
   public static final int DEFAULT_MAX_REDIRECTS = 16;
 
+  /*
+   * Default sni = false
+   */
+  public static final boolean DEFAULT_SNI = false;
+
   private boolean verifyHost = true;
   private int maxPoolSize;
   private boolean keepAlive;
@@ -168,6 +173,7 @@ public class HttpClientOptions extends ClientOptionsBase {
   private boolean http2ClearTextUpgrade;
   private boolean sendUnmaskedFrames;
   private int maxRedirects;
+  private boolean sni;
 
   /**
    * Default constructor
@@ -207,6 +213,7 @@ public HttpClientOptions(HttpClientOptions other) {
     this.http2ClearTextUpgrade = other.http2ClearTextUpgrade;
     this.sendUnmaskedFrames = other.isSendUnmaskedFrames();
     this.maxRedirects = other.maxRedirects;
+    this.sni = other.sni;
   }
 
   /**
@@ -255,6 +262,7 @@ private void init() {
     http2ClearTextUpgrade = DEFAULT_HTTP2_CLEAR_TEXT_UPGRADE;
     sendUnmaskedFrames = DEFAULT_SEND_UNMASKED_FRAMES;
     maxRedirects = DEFAULT_MAX_REDIRECTS;
+    sni = DEFAULT_SNI;
   }
 
   @Override
@@ -888,6 +896,24 @@ public HttpClientOptions setMaxRedirects(int maxRedirects) {
     return this;
   }
 
+  /**
+   * @return wether the client should use SNI on TLS/SSL connections
+   */
+  public boolean isSni() {
+    return sni;
+  }
+
+  /**
+   * Set wether the client should present the request host with the Server Name Indication when using TLS/SSL connections.
+   *
+   * @param sni true when the client should use SNI on TLS/SSL connections
+   * @return a reference to this, so the API can be used fluently
+   */
+  public HttpClientOptions setSni(boolean sni) {
+    this.sni = sni;
+    return this;
+  }
+
   public HttpClientOptions setMetricsName(String metricsName) {
     return (HttpClientOptions) super.setMetricsName(metricsName);
   }

src/main/java/io/vertx/core/http/RequestOptions.java

@@ -46,12 +46,6 @@ public class RequestOptions {
    */
   public static final String DEFAULT_URI = "";
 
-  /**
-   * SNI default serve name = null
-   */
-  public static final String DEFAULT_SERVER_NAME = null;
-
-  private String serverName;
   private String host;
   private int port;
   private boolean ssl;
@@ -61,7 +55,6 @@ public class RequestOptions {
    * Default constructor
    */
   public RequestOptions() {
-    serverName = DEFAULT_SERVER_NAME;
     host = DEFAULT_HOST;
     port = DEFAULT_PORT;
     ssl = DEFAULT_SSL;
@@ -93,25 +86,6 @@ public RequestOptions(JsonObject json) {
     setURI(json.getString("uri", DEFAULT_URI));
   }
 
-  /**
-   * Get the SNI server name to be used by the client connection.
-   *
-   * @return  the SNI server name
-   */
-  public String getServerName() {
-    return serverName;
-  }
-
-  /**
-   * Set the SNI server name to be used by the client connection.
-   *
-   * @return a reference to this, so the API can be used fluently
-   */
-  public RequestOptions setServerName(String serverName) {
-    this.serverName = serverName;
-    return this;
-  }
-
   /**
    * Get the host name to be used by the client request.
    *

src/main/java/io/vertx/core/http/impl/ConnectionManager.java

@@ -101,13 +101,11 @@ HttpClientMetrics metrics() {
 
   static final class ConnectionKey {
 
-    private final String serverName;
     private final boolean ssl;
     private final int port;
     private final String host;
 
-    public ConnectionKey(String serverName, boolean ssl, int port, String host) {
-      this.serverName = serverName;
+    public ConnectionKey(boolean ssl, int port, String host) {
       this.ssl = ssl;
       this.host = host;
       this.port = port;
@@ -120,7 +118,6 @@ public boolean equals(Object o) {
 
       ConnectionKey that = (ConnectionKey) o;
 
-      if (!Objects.equals(serverName, that.serverName)) return false;
       if (ssl != that.ssl) return false;
       if (port != that.port) return false;
       if (!Objects.equals(host, that.host)) return false;
@@ -131,7 +128,6 @@ public boolean equals(Object o) {
     @Override
     public int hashCode() {
       int result = ssl ? 1 : 0;
-      result = 31 * result + (serverName != null ? serverName.hashCode() : 0);
       result = 31 * result + (host != null ? host.hashCode() : 0);
       result = 31 * result + port;
       return result;
@@ -149,8 +145,9 @@ private class QueueManager {
     private final Map<Channel, HttpClientConnection> connectionMap = new ConcurrentHashMap<>();
     private final Map<ConnectionKey, ConnQueue> queueMap = new ConcurrentHashMap<>();
 
-    ConnQueue getConnQueue(ConnectionKey address, HttpVersion version) {
-      return queueMap.computeIfAbsent(address, targetAddress -> new ConnQueue(version, this, targetAddress));
+    ConnQueue getConnQueue(String peerHost, boolean ssl, int port, String host, HttpVersion version) {
+      ConnectionKey key = new ConnectionKey(ssl, port, ssl && peerHost != null ? peerHost : host);
+      return queueMap.computeIfAbsent(key, targetAddress -> new ConnQueue(version, this, peerHost, host, port, ssl, key));
     }
 
     public void close() {
@@ -165,17 +162,15 @@ public void close() {
   }
 
   public void getConnectionForWebsocket(boolean ssl, int port, String host, Waiter waiter) {
-    ConnectionKey address = new ConnectionKey(null, ssl, port, host);
-    ConnQueue connQueue = wsQM.getConnQueue(address, HttpVersion.HTTP_1_1);
+    ConnQueue connQueue = wsQM.getConnQueue(host, ssl, port, host, HttpVersion.HTTP_1_1);
     connQueue.getConnection(waiter);
   }
 
-  public void getConnectionForRequest(String serverName, boolean ssl, HttpVersion version, int port, String host, Waiter waiter) {
+  public void getConnectionForRequest(HttpVersion version, String peerHost, boolean ssl, int port, String host, Waiter waiter) {
     if (!keepAlive && pipelining) {
       waiter.handleFailure(new IllegalStateException("Cannot have pipelining with no keep alive"));
     } else {
-      ConnectionKey address = new ConnectionKey(serverName, ssl, port, host);
-      ConnQueue connQueue = requestQM.getConnQueue(address, version);
+      ConnQueue connQueue = requestQM.getConnQueue(peerHost, ssl, port, host, version);
       connQueue.getConnection(waiter);
     }
   }
@@ -199,15 +194,23 @@ public void close() {
   public class ConnQueue {
 
     private final QueueManager mgr;
-    private final ConnectionKey address;
+    private final String peerHost;
+    private final boolean ssl;
+    private final int port;
+    private final String host;
+    private final ConnectionKey key;
     private final Queue<Waiter> waiters = new ArrayDeque<>();
     private Pool<HttpClientConnection> pool;
     private int connCount;
     private final int maxSize;
     final Object metric;
 
-    ConnQueue(HttpVersion version, QueueManager mgr, ConnectionKey address) {
-      this.address = address;
+    ConnQueue(HttpVersion version, QueueManager mgr, String peerHost, String host, int port, boolean ssl, ConnectionKey key) {
+      this.key = key;
+      this.host = host;
+      this.port = port;
+      this.ssl = ssl;
+      this.peerHost = peerHost;
       this.mgr = mgr;
       if (version == HttpVersion.HTTP_2) {
         maxSize = options.getHttp2MaxPoolSize();
@@ -216,7 +219,7 @@ public class ConnQueue {
         maxSize = options.getMaxPoolSize();
         pool = (Pool)new Http1xPool(client, ConnectionManager.this.metrics, options, this, mgr.connectionMap, version, options.getMaxPoolSize());
       }
-      this.metric = ConnectionManager.this.metrics.createEndpoint(address.host, address.port, maxSize);
+      this.metric = ConnectionManager.this.metrics.createEndpoint(host, port, maxSize);
     }
 
     public synchronized void getConnection(Waiter waiter) {
@@ -289,7 +292,7 @@ private void createNewConnection(Waiter waiter) {
       Bootstrap bootstrap = new Bootstrap();
       bootstrap.group(context.nettyEventLoop());
       bootstrap.channel(NioSocketChannel.class);
-      connector.connect(this, bootstrap, context, address.serverName, address.ssl, pool.version(), address.host, address.port, waiter);
+      connector.connect(this, bootstrap, context, peerHost, ssl, pool.version(), host, port, waiter);
     }
 
     /**
@@ -318,9 +321,9 @@ public synchronized void connectionClosed() {
         createNewConnection(waiter);
       } else if (connCount == 0) {
         // No waiters and no connections - remove the ConnQueue
-        mgr.queueMap.remove(address);
+        mgr.queueMap.remove(key);
         if (ConnectionManager.this.metrics.isEnabled()) {
-          ConnectionManager.this.metrics.closeEndpoint(address.host, address.port, metric);
+          ConnectionManager.this.metrics.closeEndpoint(host, port, metric);
         }
       }
     }
@@ -412,7 +415,7 @@ protected void connect(
         ConnQueue queue,
         Bootstrap bootstrap,
         ContextImpl context,
-        String serverName,
+        String peerHost,
         boolean ssl,
         HttpVersion version,
         String host,
@@ -435,7 +438,7 @@ protected void connect(
         ChannelPipeline pipeline = ch.pipeline();
         boolean useAlpn = options.isUseAlpn();
         if (useAlpn) {
-          SslHandler sslHandler = new SslHandler(sslHelper.createEngine(client.getVertx(), host, port, serverName));
+          SslHandler sslHandler = new SslHandler(sslHelper.createEngine(client.getVertx(), peerHost, port, options.isSni() ? peerHost : null));
           ch.pipeline().addLast("ssl", sslHandler);
           ch.pipeline().addLast(new ApplicationProtocolNegotiationHandler("http/1.1") {
             @Override
@@ -453,7 +456,7 @@ protected void configurePipeline(ChannelHandlerContext ctx, String protocol) {
           });
         } else {
           if (ssl) {
-            pipeline.addLast("ssl", new SslHandler(sslHelper.createEngine(vertx, host, port, serverName)));
+            pipeline.addLast("ssl", new SslHandler(sslHelper.createEngine(vertx, peerHost, port, options.isSni() ? peerHost : null)));
           }
           if (version == HttpVersion.HTTP_2) {
             if (options.isHttp2ClearTextUpgrade()) {

src/main/java/io/vertx/core/http/impl/HttpClientImpl.java

@@ -93,7 +93,7 @@ public class HttpClientImpl implements HttpClient, MetricsProvider {
         if (uri.getQuery() != null) {
           requestURI += "?" + uri.getQuery();
         }
-        return Future.succeededFuture(createRequest(null, m, uri.getHost(), port, ssl, requestURI, null));
+        return Future.succeededFuture(createRequest(m, uri.getHost(), port, ssl, requestURI, null));
       }
       return null;
     } catch (Exception e) {
@@ -454,12 +454,12 @@ public HttpClientRequest requestAbs(HttpMethod method, String absoluteURI) {
         port = 443;
       }
     }
-    return createRequest(null, method, url.getHost(), port, ssl, url.getFile(), null);
+    return createRequest(method, url.getHost(), port, ssl, url.getFile(), null);
   }
 
   @Override
   public HttpClientRequest request(HttpMethod method, int port, String host, String requestURI) {
-    return createRequest(null, method, host, port, null, requestURI, null);
+    return createRequest(method, host, port, null, requestURI, null);
   }
 
   @Override
@@ -469,7 +469,7 @@ public HttpClientRequest request(HttpMethod method, RequestOptions options, Hand
 
   @Override
   public HttpClientRequest request(HttpMethod method, RequestOptions options) {
-    return createRequest(options.getServerName(), method, options.getHost(), options.getPort(), options.isSsl(), options.getURI(), null);
+    return createRequest(method, options.getHost(), options.getPort(), options.isSsl(), options.getURI(), null);
   }
 
   @Override
@@ -904,8 +904,8 @@ boolean isCancelled() {
     });
   }
 
-  void getConnectionForRequest(String serverName, boolean ssl, int port, String host, Waiter waiter) {
-    connectionManager.getConnectionForRequest(serverName, ssl, options.getProtocolVersion(), port, host, waiter);
+  void getConnectionForRequest(String peerHost, boolean ssl, int port, String host, Waiter waiter) {
+    connectionManager.getConnectionForRequest(options.getProtocolVersion(), peerHost, ssl, port, host, waiter);
   }
 
   /**
@@ -933,11 +933,11 @@ private URL parseUrl(String surl) {
   }
 
   private HttpClient requestNow(HttpMethod method, RequestOptions options, Handler<HttpClientResponse> responseHandler) {
-    createRequest(null, method, options.getHost(), options.getPort(), options.isSsl(), options.getURI(), null).handler(responseHandler).end();
+    createRequest(method, options.getHost(), options.getPort(), options.isSsl(), options.getURI(), null).handler(responseHandler).end();
     return this;
   }
 
-  private HttpClientRequest createRequest(String serverName, HttpMethod method, String host, int port, Boolean ssl, String relativeURI, MultiMap headers) {
+  private HttpClientRequest createRequest(HttpMethod method, String host, int port, Boolean ssl, String relativeURI, MultiMap headers) {
     Objects.requireNonNull(method, "no null method accepted");
     Objects.requireNonNull(host, "no null host accepted");
     Objects.requireNonNull(relativeURI, "no null relativeURI accepted");
@@ -955,11 +955,11 @@ private HttpClientRequest createRequest(String serverName, HttpMethod method, St
         headers.add("Proxy-Authorization", "Basic " + Base64.getEncoder()
             .encodeToString((proxyOptions.getUsername() + ":" + proxyOptions.getPassword()).getBytes()));
       }
-      req = new HttpClientRequestImpl(this, useSSL, serverName, method, proxyOptions.getHost(), proxyOptions.getPort(),
+      req = new HttpClientRequestImpl(this, useSSL, method, proxyOptions.getHost(), proxyOptions.getPort(),
           relativeURI, vertx);
       req.setHost(host + (port != 80 ? ":" + port : ""));
     } else {
-      req = new HttpClientRequestImpl(this, useSSL, serverName, method, host, port, relativeURI, vertx);
+      req = new HttpClientRequestImpl(this, useSSL, method, host, port, relativeURI, vertx);
     }
     if (headers != null) {
       req.headers().setAll(headers);