From f162017eb647a0a3c2ceed589f51e44ec486a4da Mon Sep 17 00:00:00 2001 From: Bartosz Radzynski Date: Mon, 15 May 2023 09:26:27 +0200 Subject: [PATCH 1/6] Fix hostname resolution for proxied connections --- src/main/java/io/nats/client/impl/SocketDataPort.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/nats/client/impl/SocketDataPort.java b/src/main/java/io/nats/client/impl/SocketDataPort.java index 94ac40304..645fe014f 100644 --- a/src/main/java/io/nats/client/impl/SocketDataPort.java +++ b/src/main/java/io/nats/client/impl/SocketDataPort.java @@ -74,7 +74,9 @@ public void connect(@NonNull NatsConnection conn, @NonNull NatsUri nuri, long ti socket = connectToFastestIp(options, host, port, (int) timeout); } else { socket = createSocket(options); - socket.connect(new InetSocketAddress(host, port), (int) timeout); + InetSocketAddress inetSocketAddress = options.isNoResolveHostnames() ? + InetSocketAddress.createUnresolved(host, port) : new InetSocketAddress(host, port); + socket.connect(inetSocketAddress, (int) timeout); } if (options.getSocketReadTimeoutMillis() > 0) { From 465b0e6d262af7214da0f07ed5a112cca78142b4 Mon Sep 17 00:00:00 2001 From: Bartosz Radzynski Date: Mon, 15 May 2023 16:58:45 +0200 Subject: [PATCH 2/6] Do not use `InetSocketAddress.createUnresolved` for uris already containing an IP address --- src/main/java/io/nats/client/impl/SocketDataPort.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/nats/client/impl/SocketDataPort.java b/src/main/java/io/nats/client/impl/SocketDataPort.java index 645fe014f..b2cfdf71f 100644 --- a/src/main/java/io/nats/client/impl/SocketDataPort.java +++ b/src/main/java/io/nats/client/impl/SocketDataPort.java @@ -74,7 +74,7 @@ public void connect(@NonNull NatsConnection conn, @NonNull NatsUri nuri, long ti socket = connectToFastestIp(options, host, port, (int) timeout); } else { socket = createSocket(options); - InetSocketAddress inetSocketAddress = options.isNoResolveHostnames() ? + InetSocketAddress inetSocketAddress = (options.isNoResolveHostnames() && !nuri.hostIsIpAddress()) ? InetSocketAddress.createUnresolved(host, port) : new InetSocketAddress(host, port); socket.connect(inetSocketAddress, (int) timeout); } From 8d9679d3ca3898a7da7dae31f38e7c07e66138df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Radzy=C5=84ski?= Date: Fri, 31 Oct 2025 11:47:13 +0100 Subject: [PATCH 3/6] Add unit test --- .../io/nats/client/impl/SocketDataPort.java | 3 +- .../impl/SocketDataPortProxyHostnameTest.java | 247 ++++++++++++++++++ 2 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java diff --git a/src/main/java/io/nats/client/impl/SocketDataPort.java b/src/main/java/io/nats/client/impl/SocketDataPort.java index b2cfdf71f..aaf9c6f6a 100644 --- a/src/main/java/io/nats/client/impl/SocketDataPort.java +++ b/src/main/java/io/nats/client/impl/SocketDataPort.java @@ -74,7 +74,8 @@ public void connect(@NonNull NatsConnection conn, @NonNull NatsUri nuri, long ti socket = connectToFastestIp(options, host, port, (int) timeout); } else { socket = createSocket(options); - InetSocketAddress inetSocketAddress = (options.isNoResolveHostnames() && !nuri.hostIsIpAddress()) ? + boolean shouldCreateUnresolvedInetAddress = (options.isNoResolveHostnames() && !nuri.hostIsIpAddress() && (options.getProxy() != null)); + InetSocketAddress inetSocketAddress = shouldCreateUnresolvedInetAddress ? InetSocketAddress.createUnresolved(host, port) : new InetSocketAddress(host, port); socket.connect(inetSocketAddress, (int) timeout); } diff --git a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java new file mode 100644 index 000000000..e94d600fb --- /dev/null +++ b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java @@ -0,0 +1,247 @@ +// Copyright 2015-2018 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at: +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package io.nats.client.impl; + +import io.nats.client.Options; +import io.nats.client.support.NatsUri; +import io.nats.client.utils.TestBase; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.*; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Test for proxy hostname resolution bug fix. + * + * When a proxy is configured with domain name whitelisting, the client should NOT + * resolve the hostname to an IP address before connecting. Instead, it should pass + * the hostname as-is to the proxy so the proxy can enforce domain whitelisting. + */ +public class SocketDataPortProxyHostnameTest extends TestBase { + + /** + * Mock proxy that tracks whether it received the CONNECT request with + * a domain name or an IP address. + */ + static class WhitelistingProxyServer implements Runnable { + private final ServerSocket serverSocket; + private volatile String receivedHost; + private final ExecutorService executor; + + WhitelistingProxyServer(ExecutorService executor) throws IOException { + this.executor = executor; + // Bind to localhost on ephemeral port + this.serverSocket = new ServerSocket(0, 10, InetAddress.getLoopbackAddress()); + } + + public int getPort() { + return serverSocket.getLocalPort(); + } + + public String getReceivedHost() { + return receivedHost; + } + + @Override + public void run() { + try { + while (true) { + Socket clientSocket = serverSocket.accept(); + executor.submit(() -> handleClientConnection(clientSocket)); + } + } catch (IOException e) { + // Expected when shutting down + } + } + + private void handleClientConnection(Socket clientSocket) { + try (Socket client = clientSocket; + InputStream in = client.getInputStream(); + OutputStream out = client.getOutputStream()) { + + // Read CONNECT request line + String connectRequest = readLine(in); + + if (connectRequest != null && connectRequest.startsWith("CONNECT ")) { + // Parse the CONNECT request to extract host and port + // Format: CONNECT host:port HTTP/1.x + String[] parts = connectRequest.split("\\s+"); + if (parts.length >= 2) { + String hostPort = parts[1]; + String[] hostPortParts = hostPort.split(":"); + if (hostPortParts.length >= 1) { + receivedHost = hostPortParts[0]; + } + } + + // Read and discard headers until empty line + String line; + while ((line = readLine(in)) != null && !line.isEmpty()) { + // consume headers + } + + // Send 200 OK response + out.write("HTTP/1.0 200 Connection established\r\n\r\n".getBytes()); + out.flush(); + + // Keep the connection open for a bit so the client can use it + Thread.sleep(100); + } + } catch (IOException | InterruptedException e) { + // Connection closed or error + } + } + + private String readLine(InputStream in) throws IOException { + StringBuilder sb = new StringBuilder(); + int ch; + boolean gotCR = false; + while ((ch = in.read()) != -1) { + if (ch == '\r') { + gotCR = true; + } else if (ch == '\n' && gotCR) { + return sb.deleteCharAt(sb.length() - 1).toString(); + } else { + gotCR = false; + sb.append((char) ch); + } + } + return sb.length() > 0 ? sb.toString() : null; + } + + public void shutdown() throws IOException { + serverSocket.close(); + } + } + + /** + * Test that when a proxy is configured and a domain name (non-IP) is used, + * the SocketDataPort preserves the hostname instead of resolving it to IP. + * This allows proxies with domain whitelisting to work correctly. + */ + @Test + public void testProxyReceivesDomainNameWithNoResolveHostnames() throws Exception { + testProxyHostnameResolution( + true, // enableNoResolveHostnames + "nats://localhost:4222", + false // expectIpAddress + ); + } + + /** + * Test that WITHOUT noResolveHostnames(), the proxy receives an IP address instead + * of the domain name. This demonstrates the bug that was fixed. + * + * When noResolveHostnames() is NOT set and a proxy is configured, the hostname + * gets resolved to an IP address before being sent to the proxy. This breaks + * proxies with domain name whitelisting. + */ + @Test + public void testProxyReceivesIpAddressWithoutNoResolveHostnames() throws Exception { + testProxyHostnameResolution( + false, // disableNoResolveHostnames + "nats://localhost:4222", + true // expectIpAddress + ); + } + + /** + * Helper method to test proxy hostname resolution behavior. + * + * @param useNoResolveHostnames Whether to enable noResolveHostnames() option + * @param targetUri The URI to connect to + * @param expectIpAddress True if expecting proxy to receive an IP, false for hostname + */ + private void testProxyHostnameResolution(boolean useNoResolveHostnames, String targetUri, + boolean expectIpAddress) + throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(3); + WhitelistingProxyServer proxyServer = new WhitelistingProxyServer(executor); + + try { + executor.submit(proxyServer); + + Options.Builder optionsBuilder = new Options.Builder() + .proxy(new Proxy(Proxy.Type.HTTP, new InetSocketAddress("localhost", proxyServer.getPort()))) + .noReconnect(); + + if (useNoResolveHostnames) { + optionsBuilder.noResolveHostnames(); + } + + Options options = optionsBuilder.build(); + MockNatsConnection mockConnection = new MockNatsConnection(options); + SocketDataPort dataPort = new SocketDataPort(); + NatsUri nuri = new NatsUri(targetUri); + + try { + dataPort.connect(mockConnection, nuri, 5_000_000_000L); + } catch (Exception e) { + // Expected - connection will fail since there's no real server + } + + String receivedHost = proxyServer.getReceivedHost(); + if (receivedHost != null) { + if (expectIpAddress) { + assertTrue( + isIpAddress(receivedHost), + "Expected IP address but proxy received: " + receivedHost + ); + } else { + assertFalse( + isIpAddress(receivedHost), + "Expected hostname but proxy received IP: " + receivedHost + ); + } + } + } finally { + safeShutdown(proxyServer, executor); + } + } + + /** + * Safely shutdown the proxy server and executor. + */ + private void safeShutdown(WhitelistingProxyServer proxyServer, ExecutorService executor) { + try { + proxyServer.shutdown(); + } catch (IOException e) { + // ignore + } + executor.shutdown(); + } + + /** + * Check if a string is an IP address (IPv4 or IPv6) + */ + private boolean isIpAddress(String host) { + if (host == null) { + return false; + } + try { + InetAddress.getByName(host); + // If getByName doesn't throw and we only got back an IP, it's likely an IP + // This is a simple heuristic check + return host.matches("^\\d+\\.\\d+\\.\\d+\\.\\d+$") || host.startsWith("["); + } catch (UnknownHostException e) { + return false; + } + } +} From 45da71daef8ab17b02ddde1aef3ee6133d7fc2c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Radzy=C5=84ski?= Date: Wed, 5 Nov 2025 16:08:11 +0100 Subject: [PATCH 4/6] Add enableInetAddressCreateUnresolved option --- src/main/java/io/nats/client/Options.java | 29 +++++++++++++++++++ .../io/nats/client/impl/SocketDataPort.java | 9 ++++-- .../impl/SocketDataPortProxyHostnameTest.java | 16 +++++----- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index 6848308bb..c6699ada0 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -555,6 +555,12 @@ public class Options { */ public static final String PROP_FAST_FALLBACK = PFX + "fast.fallback"; + /** + * Property used to enable InetSocketAddress.createUnresolved for proxied connections. + * {@link Builder#enableInetAddressCreateUnresolved() enableInetAddressCreateUnresolved}. + */ + public static final String PROP_ENABLE_INET_ADDRESS_CREATE_UNRESOLVED = PFX + "inet.address.create.unresolved"; + // ---------------------------------------------------------------------------------------------------- // PROTOCOL CONNECT OPTION CONSTANTS // ---------------------------------------------------------------------------------------------------- @@ -714,6 +720,7 @@ public class Options { private final List> httpRequestInterceptors; private final Proxy proxy; private final boolean enableFastFallback; + private final boolean enableInetAddressCreateUnresolved; static class DefaultThreadFactory implements ThreadFactory { final String name; @@ -865,6 +872,7 @@ public static class Builder { private String tlsAlgorithm = DEFAULT_TLS_ALGORITHM; private String credentialPath; private boolean enableFastFallback = false; + private boolean enableInetAddressCreateUnresolved = false; /** * Constructs a new Builder with the default values. @@ -984,6 +992,7 @@ public Builder properties(Properties props) { booleanProperty(props, PROP_USE_DISPATCHER_WITH_EXECUTOR, b -> this.useDispatcherWithExecutor = b); booleanProperty(props, PROP_FORCE_FLUSH_ON_REQUEST, b -> this.forceFlushOnRequest = b); booleanProperty(props, PROP_FAST_FALLBACK, b -> this.enableFastFallback = b); + booleanProperty(props, PROP_ENABLE_INET_ADDRESS_CREATE_UNRESOLVED, b -> this.enableInetAddressCreateUnresolved = b); classnameProperty(props, PROP_SERVERS_POOL_IMPLEMENTATION_CLASS, o -> this.serverPool = (ServerPool) o); classnameProperty(props, PROP_DISPATCHER_FACTORY_CLASS, o -> this.dispatcherFactory = (DispatcherFactory) o); @@ -1889,6 +1898,16 @@ public Builder enableFastFallback() { return this; } + /** + * Whether to enable InetSocketAddress.createUnresolved for proxied connections. + * This is useful for backward compatibility and when hostname resolution should be deferred. + * @return the Builder for chaining + */ + public Builder enableInetAddressCreateUnresolved() { + this.enableInetAddressCreateUnresolved = true; + return this; + } + /** * Build an Options object from this Builder. * @@ -2114,6 +2133,7 @@ public Builder(Options o) { this.serverPool = o.serverPool; this.dispatcherFactory = o.dispatcherFactory; this.enableFastFallback = o.enableFastFallback; + this.enableInetAddressCreateUnresolved = o.enableInetAddressCreateUnresolved; } } @@ -2186,6 +2206,7 @@ private Options(Builder b) { this.serverPool = b.serverPool; this.dispatcherFactory = b.dispatcherFactory; this.enableFastFallback = b.enableFastFallback; + this.enableInetAddressCreateUnresolved = b.enableInetAddressCreateUnresolved; } // ---------------------------------------------------------------------------------------------------- @@ -2729,6 +2750,14 @@ public boolean isEnableFastFallback() { return enableFastFallback; } + /** + * Whether InetSocketAddress.createUnresolved is enabled for proxied connections + * @return the flag + */ + public boolean isEnableInetAddressCreateUnresolved() { + return enableInetAddressCreateUnresolved; + } + public URI createURIForServer(String serverURI) throws URISyntaxException { return new NatsUri(serverURI).getUri(); } diff --git a/src/main/java/io/nats/client/impl/SocketDataPort.java b/src/main/java/io/nats/client/impl/SocketDataPort.java index aaf9c6f6a..34c84374d 100644 --- a/src/main/java/io/nats/client/impl/SocketDataPort.java +++ b/src/main/java/io/nats/client/impl/SocketDataPort.java @@ -74,9 +74,12 @@ public void connect(@NonNull NatsConnection conn, @NonNull NatsUri nuri, long ti socket = connectToFastestIp(options, host, port, (int) timeout); } else { socket = createSocket(options); - boolean shouldCreateUnresolvedInetAddress = (options.isNoResolveHostnames() && !nuri.hostIsIpAddress() && (options.getProxy() != null)); - InetSocketAddress inetSocketAddress = shouldCreateUnresolvedInetAddress ? - InetSocketAddress.createUnresolved(host, port) : new InetSocketAddress(host, port); + InetSocketAddress inetSocketAddress; + if (options.isEnableInetAddressCreateUnresolved() && !nuri.hostIsIpAddress()) { + inetSocketAddress = InetSocketAddress.createUnresolved(host, port); + } else { + inetSocketAddress = new InetSocketAddress(host, port); + } socket.connect(inetSocketAddress, (int) timeout); } diff --git a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java index e94d600fb..224b9c5c0 100644 --- a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java +++ b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java @@ -137,7 +137,7 @@ public void shutdown() throws IOException { * This allows proxies with domain whitelisting to work correctly. */ @Test - public void testProxyReceivesDomainNameWithNoResolveHostnames() throws Exception { + public void testProxyReceivesDomainNameWithEnableInetAddressCreateUnresolved() throws Exception { testProxyHostnameResolution( true, // enableNoResolveHostnames "nats://localhost:4222", @@ -146,15 +146,15 @@ public void testProxyReceivesDomainNameWithNoResolveHostnames() throws Exception } /** - * Test that WITHOUT noResolveHostnames(), the proxy receives an IP address instead + * Test that WITHOUT isEnableInetAddressCreateUnresolved(), the proxy receives an IP address instead * of the domain name. This demonstrates the bug that was fixed. * - * When noResolveHostnames() is NOT set and a proxy is configured, the hostname + * When isEnableInetAddressCreateUnresolved() is NOT set and a proxy is configured, the hostname * gets resolved to an IP address before being sent to the proxy. This breaks * proxies with domain name whitelisting. */ @Test - public void testProxyReceivesIpAddressWithoutNoResolveHostnames() throws Exception { + public void testProxyReceivesIpAddressWithoutEnableInetAddressCreateUnresolved() throws Exception { testProxyHostnameResolution( false, // disableNoResolveHostnames "nats://localhost:4222", @@ -165,11 +165,11 @@ public void testProxyReceivesIpAddressWithoutNoResolveHostnames() throws Excepti /** * Helper method to test proxy hostname resolution behavior. * - * @param useNoResolveHostnames Whether to enable noResolveHostnames() option + * @param useEnableInetAddressCreateUnresolved Whether to enable isEnableInetAddressCreateUnresolved() option * @param targetUri The URI to connect to * @param expectIpAddress True if expecting proxy to receive an IP, false for hostname */ - private void testProxyHostnameResolution(boolean useNoResolveHostnames, String targetUri, + private void testProxyHostnameResolution(boolean useEnableInetAddressCreateUnresolved, String targetUri, boolean expectIpAddress) throws Exception { ExecutorService executor = Executors.newFixedThreadPool(3); @@ -182,8 +182,8 @@ private void testProxyHostnameResolution(boolean useNoResolveHostnames, String t .proxy(new Proxy(Proxy.Type.HTTP, new InetSocketAddress("localhost", proxyServer.getPort()))) .noReconnect(); - if (useNoResolveHostnames) { - optionsBuilder.noResolveHostnames(); + if (useEnableInetAddressCreateUnresolved) { + optionsBuilder.enableInetAddressCreateUnresolved(); } Options options = optionsBuilder.build(); From 50de4f1e224c2149fac78932c2e6241abfe903fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Radzy=C5=84ski?= Date: Thu, 6 Nov 2025 16:55:52 +0100 Subject: [PATCH 5/6] Fix test on ci --- .../io/nats/client/impl/SocketDataPortProxyHostnameTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java index 224b9c5c0..24ab57e69 100644 --- a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java +++ b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java @@ -98,7 +98,9 @@ private void handleClientConnection(Socket clientSocket) { } // Send 200 OK response - out.write("HTTP/1.0 200 Connection established\r\n\r\n".getBytes()); + out.write("HTTP/1.1 200 Connection Established\r\n".getBytes()); + out.write("Content-Length: 0\r\n".getBytes()); + out.write("\r\n".getBytes()); out.flush(); // Keep the connection open for a bit so the client can use it From d5c83382f06ae1e73564ec8677256966676b204f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Radzy=C5=84ski?= Date: Fri, 7 Nov 2025 08:10:02 +0100 Subject: [PATCH 6/6] Try to fix tests on CI once again --- .../io/nats/client/impl/SocketDataPortProxyHostnameTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java index 24ab57e69..44e5f4510 100644 --- a/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java +++ b/src/test/java/io/nats/client/impl/SocketDataPortProxyHostnameTest.java @@ -104,7 +104,7 @@ private void handleClientConnection(Socket clientSocket) { out.flush(); // Keep the connection open for a bit so the client can use it - Thread.sleep(100); + Thread.sleep(1000); } } catch (IOException | InterruptedException e) { // Connection closed or error