diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index 73ec21155..10004f15c 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/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; } diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 7ca25f473..12f387e1d 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -22,7 +22,6 @@ import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder; -import org.apache.hc.client5.http.entity.mime.MultipartPartBuilder; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.io.BasicHttpClientConnectionManager; @@ -32,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; @@ -46,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; @@ -93,7 +93,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Properties; import java.util.Arrays; import java.util.HashMap; @@ -205,11 +204,13 @@ private ConnectionConfig createConnectionConfig(Map configuratio } private HttpClientConnectionManager basicConnectionManager(LayeredConnectionSocketFactory sslConnectionSocketFactory, SocketConfig socketConfig, Map configuration) { - RegistryBuilder registryBuilder = RegistryBuilder.create(); - registryBuilder.register("http", PlainConnectionSocketFactory.getSocketFactory()); - registryBuilder.register("https", sslConnectionSocketFactory); + Lookup tlsSocketStrategyLookup = RegistryBuilder.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); diff --git a/client-v2/src/main/java/com/clickhouse/client/api/transport/HttpEndpoint.java b/client-v2/src/main/java/com/clickhouse/client/api/transport/HttpEndpoint.java index 839b167d6..b71226d04 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/transport/HttpEndpoint.java +++ b/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; + } + } } diff --git a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java index d63f9b2cb..c8904708b 100644 --- a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java +++ b/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 serverVersion = client.queryAll("SELECT version()"); return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression); diff --git a/client-v2/src/test/java/com/clickhouse/client/api/ClientBuilderTest.java b/client-v2/src/test/java/com/clickhouse/client/api/ClientBuilderTest.java new file mode 100644 index 000000000..044a37de0 --- /dev/null +++ b/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 endpoints = + (List) endpointsField.get(client); + return endpoints.get(0).getURI().toString(); + } +} diff --git a/client-v2/src/test/java/com/clickhouse/client/api/transport/HttpEndpointTest.java b/client-v2/src/test/java/com/clickhouse/client/api/transport/HttpEndpointTest.java index 8870cae00..cb41de0ad 100644 --- a/client-v2/src/test/java/com/clickhouse/client/api/transport/HttpEndpointTest.java +++ b/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"); + } }