Merge pull request #2808 from ClickHouse/03/25/26/resolve_underscore_hostname

[client-v2] Implemented proper handling of hostnames with `_` Added DNS resolver

Author: chernser

client-v2/src/main/java/com/clickhouse/client/api/Client.java

@@ -53,8 +53,6 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.lang.reflect.InvocationTargetException;
-import java.net.MalformedURLException;
-import java.net.URL;
 import java.time.Duration;
 import java.time.ZoneId;
 import java.time.temporal.ChronoUnit;
@@ -293,32 +291,10 @@ public Builder() {
          */
         public Builder addEndpoint(String endpoint) {
             try {
-                URL endpointURL = new URL(endpoint);
-
-                String protocolStr = endpointURL.getProtocol();
-                if (!protocolStr.equalsIgnoreCase("https") &&
-                    !protocolStr.equalsIgnoreCase("http")) {
-                    throw new IllegalArgumentException("Only HTTP and HTTPS protocols are supported");
-                }
-
-                boolean secure = protocolStr.equalsIgnoreCase("https");
-                String host = endpointURL.getHost();
-                if (host == null || host.isEmpty()) {
-                    throw new IllegalArgumentException("Host cannot be empty in endpoint: " + endpoint);
-                }
-
-                int port = endpointURL.getPort();
-                if (port <= 0) {
-                    throw new ValidationUtils.SettingsValidationException("port", "Valid port must be specified");
-                }
-
-                String path = endpointURL.getPath();
-                if (path == null || path.isEmpty()) {
-                    path = "/";
-                }
-
-                return addEndpoint(Protocol.HTTP, host, port, secure, path);
-            } catch (MalformedURLException e) {
+                return addEndpoint(new HttpEndpoint(endpoint));
+            } catch (ValidationUtils.SettingsValidationException e) {
+                throw e;
+            } catch (IllegalArgumentException e) {
                 throw new IllegalArgumentException("Endpoint should be a valid URL string, but was " + endpoint, e);
             }
         }
@@ -336,19 +312,19 @@ public Builder addEndpoint(Protocol protocol, String host, int port, boolean sec
         }
 
         public Builder addEndpoint(Protocol protocol, String host, int port, boolean secure, String basePath) {
-            ValidationUtils.checkNonBlank(host, "host");
             ValidationUtils.checkNotNull(protocol, "protocol");
-            ValidationUtils.checkRange(port, 1, ValidationUtils.TCP_PORT_NUMBER_MAX, "port");
             ValidationUtils.checkNotNull(basePath, "basePath");
 
             if (protocol == Protocol.HTTP) {
-                HttpEndpoint httpEndpoint = new HttpEndpoint(host, port, secure, basePath);
-                this.endpoints.add(httpEndpoint);
+                return addEndpoint(new HttpEndpoint(host, port, secure, basePath));
             } else {
                 throw new IllegalArgumentException("Unsupported protocol: " + protocol);
             }
-            return this;
+        }
 
+        private Builder addEndpoint(Endpoint endpoint) {
+            this.endpoints.add(endpoint);
+            return this;
         }
 
 

client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java

@@ -31,10 +31,9 @@
 import org.apache.hc.client5.http.io.HttpClientConnectionManager;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
-import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
 import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory;
-import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
 import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
+import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.ConnectionRequestTimeoutException;
 import org.apache.hc.core5.http.ContentType;
@@ -45,8 +44,10 @@
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.NoHttpResponseException;
+import org.apache.hc.core5.http.URIScheme;
 import org.apache.hc.core5.http.config.CharCodingConfig;
 import org.apache.hc.core5.http.config.Http1Config;
+import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.config.RegistryBuilder;
 import org.apache.hc.core5.http.impl.io.DefaultHttpResponseParserFactory;
 import org.apache.hc.core5.http.io.SocketConfig;
@@ -204,11 +205,13 @@ private ConnectionConfig createConnectionConfig(Map<String, Object> configuratio
     }
 
     private HttpClientConnectionManager basicConnectionManager(LayeredConnectionSocketFactory sslConnectionSocketFactory, SocketConfig socketConfig, Map<String, Object> configuration) {
-        RegistryBuilder<ConnectionSocketFactory> registryBuilder = RegistryBuilder.create();
-        registryBuilder.register("http", PlainConnectionSocketFactory.getSocketFactory());
-        registryBuilder.register("https", sslConnectionSocketFactory);
+        Lookup<TlsSocketStrategy> tlsSocketStrategyLookup = RegistryBuilder.<TlsSocketStrategy>create()
+                .register(URIScheme.HTTPS.id, (socket, target, port, attachment, context) ->
+                        (SSLSocket) sslConnectionSocketFactory.createLayeredSocket(socket, target, port, context))
+                .build();
 
-        BasicHttpClientConnectionManager connManager = new BasicHttpClientConnectionManager(registryBuilder.build());
+        BasicHttpClientConnectionManager connManager = BasicHttpClientConnectionManager.create(
+                null, null, tlsSocketStrategyLookup, null);
         connManager.setConnectionConfig(createConnectionConfig(configuration));
         connManager.setSocketConfig(socketConfig);
 

client-v2/src/main/java/com/clickhouse/client/api/transport/HttpEndpoint.java

@@ -1,8 +1,10 @@
 package com.clickhouse.client.api.transport;
 
 import com.clickhouse.client.api.ClientMisconfigurationException;
+import com.clickhouse.client.api.internal.ValidationUtils;
 
 import java.net.URI;
+import java.net.MalformedURLException;
 import java.net.URL;
 
 public class HttpEndpoint implements Endpoint {
@@ -19,24 +21,28 @@ public class HttpEndpoint implements Endpoint {
 
     private final String path;
 
-    public HttpEndpoint(String host, int port, boolean secure, String path){
-        this.host = host;
-        this.port = port;
-        this.secure = secure;
-        if (path != null && !path.isEmpty()) {
-            // Ensure basePath starts with /
-            this.path = path.startsWith("/") ? path : "/" + path;
-        } else {
-            this.path = "/";
-        }
-        
-        // Use URI constructor to properly handle encoding of path segments
-        // Encode path segments separately to preserve slashes
-        try {
-            this.uri = new URI(secure ? "https" : "http", null, host, port, this.path, null, null);
-        } catch (Exception e) {
-            throw new ClientMisconfigurationException("Failed to create endpoint URL", e);
-        }
+    public HttpEndpoint(String endpoint) {
+        this(parseEndpointUrl(endpoint));
+    }
+
+    public HttpEndpoint(String host, int port, boolean secure, String path) {
+        this(new EndpointDetails(validateHost(host), validatePort(port), secure, normalizePath(path)));
+    }
+
+    private HttpEndpoint(URL endpointUrl) {
+        this(new EndpointDetails(
+                validateHost(endpointUrl.getHost()),
+                validatePort(endpointUrl.getPort()),
+                isSecure(endpointUrl.getProtocol()),
+                decodePath(endpointUrl.getPath())));
+    }
+
+    private HttpEndpoint(EndpointDetails endpointDetails) {
+        this.host = endpointDetails.host;
+        this.port = endpointDetails.port;
+        this.secure = endpointDetails.secure;
+        this.path = endpointDetails.path;
+        this.uri = createUri(endpointDetails.host, endpointDetails.port, endpointDetails.secure, endpointDetails.path);
         this.info = uri.toString();
     }
 
@@ -77,4 +83,71 @@ public boolean equals(Object obj) {
     public int hashCode() {
         return uri.hashCode();
     }
+
+    private static URL parseEndpointUrl(String endpoint) {
+        try {
+            return new URL(endpoint);
+        } catch (MalformedURLException e) {
+            throw new IllegalArgumentException("Failed to parse endpoint URL", e);
+        }
+    }
+
+    private static String validateHost(String host) {
+        ValidationUtils.checkNonBlank(host, "host");
+        return host;
+    }
+
+    private static int validatePort(int port) {
+        if (port <= 0) {
+            throw new ValidationUtils.SettingsValidationException("port", "Valid port must be specified");
+        }
+        ValidationUtils.checkRange(port, 1, ValidationUtils.TCP_PORT_NUMBER_MAX, "port");
+        return port;
+    }
+
+    private static boolean isSecure(String protocol) {
+        if ("https".equalsIgnoreCase(protocol)) {
+            return true;
+        }
+        if ("http".equalsIgnoreCase(protocol)) {
+            return false;
+        }
+        throw new IllegalArgumentException("Only HTTP and HTTPS protocols are supported");
+    }
+
+    private static String normalizePath(String path) {
+        if (path != null && !path.isEmpty()) {
+            return path.startsWith("/") ? path : "/" + path;
+        }
+        return "/";
+    }
+
+    private static String decodePath(String path) {
+        String normalizedPath = normalizePath(path);
+        return URI.create(normalizedPath.replace(" ", "%20")).getPath();
+    }
+
+    private static URI createUri(String host, int port, boolean secure, String path) {
+        try {
+            String scheme = secure ? "https" : "http";
+            String authority = host + ":" + port;
+            return new URI(scheme, authority, path, null, null);
+        } catch (Exception e) {
+            throw new ClientMisconfigurationException("Failed to create endpoint URL", e);
+        }
+    }
+
+    private static final class EndpointDetails {
+        private final String host;
+        private final int port;
+        private final boolean secure;
+        private final String path;
+
+        private EndpointDetails(String host, int port, boolean secure, String path) {
+            this.host = host;
+            this.port = port;
+            this.secure = secure;
+            this.path = path;
+        }
+    }
 }

client-v2/src/test/java/com/clickhouse/client/ClientTests.java

@@ -5,6 +5,7 @@
 import com.clickhouse.client.api.ClientException;
 import com.clickhouse.client.api.ClientFaultCause;
 import com.clickhouse.client.api.ClientMisconfigurationException;
+import com.clickhouse.client.api.ConnectionInitiationException;
 import com.clickhouse.client.api.ConnectionReuseStrategy;
 import com.clickhouse.client.api.ServerException;
 import com.clickhouse.client.api.enums.Protocol;
@@ -30,6 +31,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.net.ConnectException;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -569,6 +571,21 @@ public void testQueryIdGenerator() throws Exception {
         Assert.assertEquals(actualIds, new ArrayList<>(queryIds));
     }
 
+    @Test(groups = {"integration"})
+    public void testHostnameWithUnderscore() throws Exception {
+
+        try (Client client = new Client.Builder().addEndpoint("http://localhost_db:8123")
+                .setUsername("default")
+                .build()) {
+            client.queryAll("SELECT 1");
+            fail("Exception expected");
+        } catch (ClientException e) {
+            Assert.assertTrue(e.getCause() instanceof ConnectionInitiationException);
+            ConnectionInitiationException ce = (ConnectionInitiationException) e.getCause();
+            Assert.assertTrue(ce.getCause() instanceof UnknownHostException);
+        }
+    }
+
     public boolean isVersionMatch(String versionExpression, Client client) {
         List<GenericRecord> serverVersion = client.queryAll("SELECT version()");
         return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression);

client-v2/src/test/java/com/clickhouse/client/api/ClientBuilderTest.java

@@ -0,0 +1,34 @@
+package com.clickhouse.client.api;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.lang.reflect.Field;
+import java.util.List;
+
+public class ClientBuilderTest {
+
+    @Test
+    public void testAddEndpointToleratesUnderscoreHostname() throws Exception {
+        try (Client client = new Client.Builder()
+                .addEndpoint("http://host_with_underscore:8123")
+                .setUsername("default")
+                .setPassword("")
+                .build()) {
+
+            String firstEndpoint = extractFirstEndpointUri(client);
+            Assert.assertEquals(firstEndpoint, "http://host_with_underscore:8123/",
+                    "Endpoint URI should preserve original hostname");
+        }
+    }
+
+    private static String extractFirstEndpointUri(Client client) throws Exception {
+        Field endpointsField = Client.class.getDeclaredField("endpoints");
+        endpointsField.setAccessible(true);
+
+        @SuppressWarnings("unchecked")
+        List<com.clickhouse.client.api.transport.Endpoint> endpoints =
+                (List<com.clickhouse.client.api.transport.Endpoint>) endpointsField.get(client);
+        return endpoints.get(0).getURI().toString();
+    }
+}

client-v2/src/test/java/com/clickhouse/client/api/transport/HttpEndpointTest.java

@@ -160,4 +160,25 @@ public void testUtf8CharactersInPath() {
         Assert.assertTrue(cyrillicEndpoint.getURI().toASCIIString().contains("%"),
                 "Cyrillic path should be percent-encoded in ASCII representation");
     }
+
+    @Test
+    public void testUnderscoreHostIsAcceptedInUri() {
+        HttpEndpoint endpoint = new HttpEndpoint("host_with_underscore", 8123, false, "/");
+        Assert.assertEquals(endpoint.getHost(), "host_with_underscore", "Original host should be preserved");
+        Assert.assertEquals(endpoint.getURI().toString(), "http://host_with_underscore:8123/");
+    }
+
+    @Test
+    public void testUrlEndpointPreservesUnderscoreHost() {
+        HttpEndpoint endpoint = new HttpEndpoint("http://host_with_underscore:8123/");
+        Assert.assertEquals(endpoint.getHost(), "host_with_underscore", "Original host should be preserved");
+        Assert.assertEquals(endpoint.getURI().toString(), "http://host_with_underscore:8123/");
+    }
+
+    @Test
+    public void testUrlEndpointIgnoresQueryAndFragment() {
+        HttpEndpoint endpoint = new HttpEndpoint("http://localhost:8123/sales%20db?ignored=value#fragment");
+        Assert.assertEquals(endpoint.getPath(), "/sales db", "Path should be decoded before URI creation");
+        Assert.assertEquals(endpoint.getURI().toString(), "http://localhost:8123/sales%20db");
+    }
 }