From 36ba72c1d7509ce9482426c9a9a64efd8e282877 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Fri, 12 Sep 2025 22:25:59 +0000 Subject: [PATCH] Fix SOCKS --- .../netty/channel/ChannelManager.java | 5 +- .../asynchttpclient/proxy/SocksProxyTest.java | 252 ++++++++++++++++++ ...cksProxyTestcontainersIntegrationTest.java | 223 ++++++++++++++++ client/src/test/resources/dante/Dockerfile | 19 ++ client/src/test/resources/dante/sockd.conf | 23 ++ 5 files changed, 520 insertions(+), 2 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/proxy/SocksProxyTest.java create mode 100644 client/src/test/java/org/asynchttpclient/proxy/SocksProxyTestcontainersIntegrationTest.java create mode 100644 client/src/test/resources/dante/Dockerfile create mode 100644 client/src/test/resources/dante/sockd.conf diff --git a/client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java b/client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java index e9f4d111e..8d13361ae 100755 --- a/client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java +++ b/client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java @@ -485,7 +485,8 @@ public SslHandler addSslHandler(ChannelPipeline pipeline, Uri uri, String virtua } SslHandler sslHandler = createSslHandler(peerHost, peerPort); - if (hasSocksProxyHandler) { + // Check if SOCKS handler actually exists in the pipeline before trying to add after it + if (hasSocksProxyHandler && pipeline.get(SOCKS_HANDLER) != null) { pipeline.addAfter(SOCKS_HANDLER, SSL_HANDLER, sslHandler); } else { pipeline.addFirst(SSL_HANDLER, sslHandler); @@ -614,4 +615,4 @@ public ClientStats getClientStats() { public boolean isOpen() { return channelPool.isOpen(); } -} +} \ No newline at end of file diff --git a/client/src/test/java/org/asynchttpclient/proxy/SocksProxyTest.java b/client/src/test/java/org/asynchttpclient/proxy/SocksProxyTest.java new file mode 100644 index 000000000..e1870721a --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/proxy/SocksProxyTest.java @@ -0,0 +1,252 @@ +/* + * Copyright (c) 2024 AsyncHttpClient Project. All rights reserved. + * + * 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 org.asynchttpclient.proxy; + +import io.github.artsok.RepeatedIfExceptionsTest; +import org.asynchttpclient.AbstractBasicTest; +import org.asynchttpclient.AsyncHttpClient; +import org.asynchttpclient.Response; +import org.asynchttpclient.testserver.SocksProxy; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * Tests for SOCKS proxy support with both HTTP and HTTPS. + */ +public class SocksProxyTest extends AbstractBasicTest { + + @Override + public AbstractHandler configureHandler() throws Exception { + return new ProxyTest.ProxyHandler(); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testSocks4ProxyWithHttp() throws Exception { + // Start SOCKS proxy in background thread + Thread socksProxyThread = new Thread(() -> { + try { + new SocksProxy(60000); + } catch (Exception e) { + logger.error("Failed to establish SocksProxy", e); + } + }); + socksProxyThread.start(); + + // Give the proxy time to start + Thread.sleep(1000); + + try (AsyncHttpClient client = asyncHttpClient()) { + String target = "http://localhost:" + port1 + '/'; + Future f = client.prepareGet(target) + .setProxyServer(new ProxyServer.Builder("localhost", 8000).setProxyType(ProxyType.SOCKS_V4)) + .execute(); + + Response response = f.get(60, TimeUnit.SECONDS); + assertNotNull(response); + assertEquals(200, response.getStatusCode()); + } + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testSocks5ProxyWithHttp() throws Exception { + // Start SOCKS proxy in background thread + Thread socksProxyThread = new Thread(() -> { + try { + new SocksProxy(60000); + } catch (Exception e) { + logger.error("Failed to establish SocksProxy", e); + } + }); + socksProxyThread.start(); + + // Give the proxy time to start + Thread.sleep(1000); + + try (AsyncHttpClient client = asyncHttpClient()) { + String target = "http://localhost:" + port1 + '/'; + Future f = client.prepareGet(target) + .setProxyServer(new ProxyServer.Builder("localhost", 8000).setProxyType(ProxyType.SOCKS_V5)) + .execute(); + + Response response = f.get(60, TimeUnit.SECONDS); + assertNotNull(response); + assertEquals(200, response.getStatusCode()); + } + } + + @Test + public void testSocks5ProxyWithHttpsDoesNotThrowException() throws Exception { + // This test specifically verifies that HTTPS requests through SOCKS5 proxy + // do not throw NoSuchElementException: socks anymore + + // Start SOCKS proxy in background thread + Thread socksProxyThread = new Thread(() -> { + try { + new SocksProxy(10000); // shorter time for test + } catch (Exception e) { + logger.error("Failed to establish SocksProxy", e); + } + }); + socksProxyThread.start(); + + // Give the proxy time to start + Thread.sleep(1000); + + try (AsyncHttpClient client = asyncHttpClient(config() + .setProxyServer(new ProxyServer.Builder("localhost", 8000).setProxyType(ProxyType.SOCKS_V5)) + .setConnectTimeout(Duration.ofMillis(5000)) + .setRequestTimeout(Duration.ofMillis(10000)))) { + + // This would previously throw: java.util.NoSuchElementException: socks + // We expect this to fail with connection timeout (since we don't have a real HTTPS target) + // but NOT with NoSuchElementException + + try { + Future f = client.prepareGet("https://httpbin.org/get").execute(); + f.get(8, TimeUnit.SECONDS); + // If we reach here, great! The SOCKS proxy worked + } catch (Exception e) { + // We should NOT see NoSuchElementException: socks anymore + String message = e.getMessage(); + if (message != null && message.contains("socks") && message.contains("NoSuchElementException")) { + throw new AssertionError("NoSuchElementException: socks still occurs", e); + } + // Other exceptions like connection timeout are expected since we don't have a real working SOCKS proxy setup + logger.info("Expected exception (not the SOCKS handler bug): " + e.getClass().getSimpleName() + ": " + message); + } + } + } + + @Test + public void testSocks4ProxyWithHttpsDoesNotThrowException() throws Exception { + // This test specifically verifies that HTTPS requests through SOCKS4 proxy + // do not throw NoSuchElementException: socks anymore + + // Start SOCKS proxy in background thread + Thread socksProxyThread = new Thread(() -> { + try { + new SocksProxy(10000); // shorter time for test + } catch (Exception e) { + logger.error("Failed to establish SocksProxy", e); + } + }); + socksProxyThread.start(); + + // Give the proxy time to start + Thread.sleep(1000); + + try (AsyncHttpClient client = asyncHttpClient(config() + .setProxyServer(new ProxyServer.Builder("localhost", 8000).setProxyType(ProxyType.SOCKS_V4)) + .setConnectTimeout(Duration.ofMillis(5000)) + .setRequestTimeout(Duration.ofMillis(10000)))) { + + // This would previously throw: java.util.NoSuchElementException: socks + // We expect this to fail with connection timeout (since we don't have a real HTTPS target) + // but NOT with NoSuchElementException + + try { + Future f = client.prepareGet("https://httpbin.org/get").execute(); + f.get(8, TimeUnit.SECONDS); + // If we reach here, great! The SOCKS proxy worked + } catch (Exception e) { + // We should NOT see NoSuchElementException: socks anymore + String message = e.getMessage(); + if (message != null && message.contains("socks") && message.contains("NoSuchElementException")) { + throw new AssertionError("NoSuchElementException: socks still occurs", e); + } + // Other exceptions like connection timeout are expected since we don't have a real working SOCKS proxy setup + logger.info("Expected exception (not the SOCKS handler bug): " + e.getClass().getSimpleName() + ": " + message); + } + } + } + + @Test + public void testIssue1913NoSuchElementExceptionSocks5() throws Exception { + // Reproduces the exact issue from GitHub issue #1913 with SOCKS5 + // This uses the exact code pattern from the issue report + var proxyServer = new ProxyServer.Builder("127.0.0.1", 1081) + .setProxyType(ProxyType.SOCKS_V5); + + try (var client = asyncHttpClient(config() + .setProxyServer(proxyServer.build()) + .setConnectTimeout(Duration.ofMillis(2000)) + .setRequestTimeout(Duration.ofMillis(5000)))) { + + // This would previously throw: java.util.NoSuchElementException: socks + // We expect this to fail with connection timeout (since proxy doesn't exist) + // but NOT with NoSuchElementException + + try { + var response = client.prepareGet("https://cloudflare.com/cdn-cgi/trace").execute().get(); + // If we reach here, great! The fix worked and proxy connection succeeded + logger.info("Connection successful: " + response.getStatusCode()); + } catch (Exception e) { + // Check that we don't get the NoSuchElementException: socks anymore + Throwable cause = e.getCause(); + String message = cause != null ? cause.getMessage() : e.getMessage(); + + // This should NOT contain the original error + if (message != null && message.contains("socks") && + (e.toString().contains("NoSuchElementException") || cause != null && cause.toString().contains("NoSuchElementException"))) { + throw new AssertionError("NoSuchElementException: socks still occurs - fix didn't work: " + e.toString()); + } + + // Other exceptions like connection timeout are expected since we don't have a working SOCKS proxy + logger.info("Expected exception (not the SOCKS handler bug): " + e.getClass().getSimpleName() + ": " + message); + } + } + } + + @Test + public void testIssue1913NoSuchElementExceptionSocks4() throws Exception { + // Reproduces the exact issue from GitHub issue #1913 with SOCKS4 + // This uses the exact code pattern from the issue report + var proxyServer = new ProxyServer.Builder("127.0.0.1", 1081) + .setProxyType(ProxyType.SOCKS_V4); + + try (var client = asyncHttpClient(config() + .setProxyServer(proxyServer.build()) + .setConnectTimeout(Duration.ofMillis(2000)) + .setRequestTimeout(Duration.ofMillis(5000)))) { + + try { + var response = client.prepareGet("https://cloudflare.com/cdn-cgi/trace").execute().get(); + logger.info("Connection successful: " + response.getStatusCode()); + } catch (Exception e) { + // Check that we don't get the NoSuchElementException: socks anymore + Throwable cause = e.getCause(); + String message = cause != null ? cause.getMessage() : e.getMessage(); + + if (message != null && message.contains("socks") && + (e.toString().contains("NoSuchElementException") || cause != null && cause.toString().contains("NoSuchElementException"))) { + throw new AssertionError("NoSuchElementException: socks still occurs - fix didn't work: " + e.toString()); + } + + logger.info("Expected exception (not the SOCKS handler bug): " + e.getClass().getSimpleName() + ": " + message); + } + } + } +} diff --git a/client/src/test/java/org/asynchttpclient/proxy/SocksProxyTestcontainersIntegrationTest.java b/client/src/test/java/org/asynchttpclient/proxy/SocksProxyTestcontainersIntegrationTest.java new file mode 100644 index 000000000..4308f388e --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/proxy/SocksProxyTestcontainersIntegrationTest.java @@ -0,0 +1,223 @@ +/* + * Copyright (c) 2025 AsyncHttpClient Project. All rights reserved. + * + * 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 org.asynchttpclient.proxy; + +import io.github.artsok.RepeatedIfExceptionsTest; +import org.asynchttpclient.AsyncHttpClient; +import org.asynchttpclient.AsyncHttpClientConfig; +import org.asynchttpclient.Response; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.DockerClientFactory; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.output.Slf4jLogConsumer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.images.builder.ImageFromDockerfile; +import org.testcontainers.junit.jupiter.Testcontainers; + +import java.nio.file.Path; +import java.time.Duration; +import java.util.concurrent.TimeUnit; + +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; +import static org.asynchttpclient.Dsl.get; +import static org.asynchttpclient.Dsl.proxyServer; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Integration tests for SOCKS proxy support using Dante SOCKS server in TestContainers. + * This validates the fix for GitHub issue #1913. + */ +@Testcontainers +public class SocksProxyTestcontainersIntegrationTest { + + private static final Logger LOGGER = LoggerFactory.getLogger(SocksProxyTestcontainersIntegrationTest.class); + + private static final int SOCKS_PORT = 1080; + + private static final String TARGET_HTTP_URL = "http://httpbin.org/get"; + private static final String TARGET_HTTPS_URL = "https://www.example.com/"; + + private static boolean dockerAvailable = false; + private static GenericContainer socksProxy; + + @BeforeAll + static void checkDockerAvailability() { + try { + dockerAvailable = DockerClientFactory.instance().isDockerAvailable(); + LOGGER.info("Docker availability check: {}", dockerAvailable); + } catch (Exception e) { + LOGGER.warn("Failed to check Docker availability: {}", e.getMessage()); + dockerAvailable = false; + } + // Skip tests if Docker not available, unless force-enabled + if (!dockerAvailable && !"true".equals(System.getProperty("docker.tests"))) { + LOGGER.info("Docker is not available - skipping integration tests. Use -Ddocker.tests=true to force run."); + return; // Don't start container if Docker not available + } + // Allow force-disabling Docker tests + if ("true".equals(System.getProperty("no.docker.tests"))) { + LOGGER.info("Docker tests disabled via -Dno.docker.tests=true"); + return; + } + // Only start container if Docker is available + if (dockerAvailable) { + try { + socksProxy = new GenericContainer<>( + new ImageFromDockerfile() + .withFileFromPath("Dockerfile", Path.of("src/test/resources/dante/Dockerfile")) + .withFileFromPath("sockd.conf", Path.of("src/test/resources/dante/sockd.conf")) + ) + .withExposedPorts(SOCKS_PORT) + .withLogConsumer(new Slf4jLogConsumer(LOGGER).withPrefix("DANTE")) + .waitingFor(Wait.forLogMessage(".*sockd.*", 1) + .withStartupTimeout(Duration.ofMinutes(2))); + socksProxy.start(); + LOGGER.info("Dante SOCKS proxy started successfully on port {}", socksProxy.getMappedPort(SOCKS_PORT)); + } catch (Exception e) { + LOGGER.warn("Failed to start Dante SOCKS proxy container: {}", e.getMessage()); + dockerAvailable = false; // Mark as unavailable if container start fails + } + } + } + + @AfterAll + static void stopContainer() { + if (socksProxy != null && socksProxy.isRunning()) { + socksProxy.stop(); + } + } + + @RepeatedIfExceptionsTest(repeats = 3) + public void testSocks4ProxyToHttpTarget() throws Exception { + assumeTrue(dockerAvailable, "Docker is not available - skipping test"); + LOGGER.info("Testing SOCKS4 proxy to HTTP target"); + AsyncHttpClientConfig config = config() + .setProxyServer(proxyServer("localhost", socksProxy.getMappedPort(SOCKS_PORT)) + .setProxyType(ProxyType.SOCKS_V4) + .build()) + .setConnectTimeout(Duration.ofMillis(10000)) + .setRequestTimeout(Duration.ofMillis(30000)) + .build(); + try (AsyncHttpClient client = asyncHttpClient(config)) { + Response response = client.executeRequest(get(TARGET_HTTP_URL)).get(30, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(response.getResponseBody().contains("httpbin")); + LOGGER.info("SOCKS4 proxy to HTTP target test passed"); + } + } + + @RepeatedIfExceptionsTest(repeats = 3) + public void testSocks5ProxyToHttpTarget() throws Exception { + assumeTrue(dockerAvailable, "Docker is not available - skipping test"); + LOGGER.info("Testing SOCKS5 proxy to HTTP target"); + AsyncHttpClientConfig config = config() + .setProxyServer(proxyServer("localhost", socksProxy.getMappedPort(SOCKS_PORT)) + .setProxyType(ProxyType.SOCKS_V5) + .build()) + .setConnectTimeout(Duration.ofMillis(10000)) + .setRequestTimeout(Duration.ofMillis(30000)) + .build(); + try (AsyncHttpClient client = asyncHttpClient(config)) { + Response response = client.executeRequest(get(TARGET_HTTP_URL)).get(30, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(response.getResponseBody().contains("httpbin")); + LOGGER.info("SOCKS5 proxy to HTTP target test passed"); + } + } + + @RepeatedIfExceptionsTest(repeats = 3) + public void testSocks4ProxyToHttpsTarget() throws Exception { + assumeTrue(dockerAvailable, "Docker is not available - skipping test"); + LOGGER.info("Testing SOCKS4 proxy to HTTPS target - validates issue #1913 fix"); + AsyncHttpClientConfig config = config() + .setProxyServer(proxyServer("localhost", socksProxy.getMappedPort(SOCKS_PORT)) + .setProxyType(ProxyType.SOCKS_V4) + .build()) + .setUseInsecureTrustManager(true) + .setConnectTimeout(Duration.ofMillis(10000)) + .setRequestTimeout(Duration.ofMillis(30000)) + .build(); + try (AsyncHttpClient client = asyncHttpClient(config)) { + Response response = client.executeRequest(get(TARGET_HTTPS_URL)).get(30, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(response.getResponseBody().contains("Example Domain") || + response.getResponseBody().contains("example")); + LOGGER.info("SOCKS4 proxy to HTTPS target test passed - issue #1913 RESOLVED!"); + } + } + + @RepeatedIfExceptionsTest(repeats = 3) + public void testSocks5ProxyToHttpsTarget() throws Exception { + assumeTrue(dockerAvailable, "Docker is not available - skipping test"); + LOGGER.info("Testing SOCKS5 proxy to HTTPS target - validates issue #1913 fix"); + AsyncHttpClientConfig config = config() + .setProxyServer(proxyServer("localhost", socksProxy.getMappedPort(SOCKS_PORT)) + .setProxyType(ProxyType.SOCKS_V5) + .build()) + .setUseInsecureTrustManager(true) + .setConnectTimeout(Duration.ofMillis(10000)) + .setRequestTimeout(Duration.ofMillis(30000)) + .build(); + try (AsyncHttpClient client = asyncHttpClient(config)) { + Response response = client.executeRequest(get(TARGET_HTTPS_URL)).get(30, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(response.getResponseBody().contains("Example Domain") || + response.getResponseBody().contains("example")); + LOGGER.info("SOCKS5 proxy to HTTPS target test passed - issue #1913 RESOLVED!"); + } + } + + @RepeatedIfExceptionsTest(repeats = 3) + public void testIssue1913ReproductionWithRealProxy() throws Exception { + assumeTrue(dockerAvailable, "Docker is not available - skipping test"); + LOGGER.info("Testing exact issue #1913 reproduction with real SOCKS proxy"); + + // This reproduces the exact scenario from the GitHub issue but with a real working proxy + var proxyServer = proxyServer("localhost", socksProxy.getMappedPort(SOCKS_PORT)) + .setProxyType(ProxyType.SOCKS_V5); + + try (var client = asyncHttpClient(config() + .setProxyServer(proxyServer) + .setUseInsecureTrustManager(true) + .setConnectTimeout(Duration.ofMillis(10000)) + .setRequestTimeout(Duration.ofMillis(30000)))) { + + // This would previously throw: java.util.NoSuchElementException: socks + var response = client.prepareGet("https://www.example.com/").execute().get(30, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(response.getResponseBody().contains("Example Domain") || + response.getResponseBody().contains("example")); + LOGGER.info("Issue #1913 reproduction test PASSED - NoSuchElementException: socks is FIXED!"); + } + } + + @Test + public void testDockerInfrastructureReady() { + assumeTrue(dockerAvailable, "Docker is not available - skipping test"); + LOGGER.info("Docker infrastructure test - validating Dante SOCKS container is ready"); + LOGGER.info("Dante SOCKS proxy available at: localhost:{}", socksProxy.getMappedPort(SOCKS_PORT)); + assertTrue(socksProxy.isRunning(), "Dante SOCKS container should be running"); + assertTrue(socksProxy.getMappedPort(SOCKS_PORT) > 0, "SOCKS port should be mapped"); + LOGGER.info("Dante SOCKS infrastructure is ready and accessible"); + } +} diff --git a/client/src/test/resources/dante/Dockerfile b/client/src/test/resources/dante/Dockerfile new file mode 100644 index 000000000..a98658439 --- /dev/null +++ b/client/src/test/resources/dante/Dockerfile @@ -0,0 +1,19 @@ +FROM ubuntu:22.04 + +# Install Dante SOCKS server +RUN apt-get update && \ + apt-get install -y dante-server && \ + rm -rf /var/lib/apt/lists/* + +# Copy dante configuration +COPY sockd.conf /etc/sockd.conf + +# Create run directory +RUN mkdir -p /var/run/sockd && \ + chmod 755 /var/run/sockd + +# Expose SOCKS port +EXPOSE 1080 + +# Run dante server (sockd binary is in /usr/sbin) +CMD ["/usr/sbin/sockd", "-f", "/etc/sockd.conf", "-D"] diff --git a/client/src/test/resources/dante/sockd.conf b/client/src/test/resources/dante/sockd.conf new file mode 100644 index 000000000..e4f7ed0fd --- /dev/null +++ b/client/src/test/resources/dante/sockd.conf @@ -0,0 +1,23 @@ +# Basic SOCKS proxy configuration for testing +# Allow all connections and methods for testing purposes + +# Server configuration - listen on all interfaces +internal: 0.0.0.0 port = 1080 +external: eth0 + +# Authentication method - no authentication for testing +socksmethod: none + +# Clients allowed to connect (all for testing) +client pass { + from: 0.0.0.0/0 to: 0.0.0.0/0 + log: error +} + +# Rules for SOCKS requests +socks pass { + from: 0.0.0.0/0 to: 0.0.0.0/0 + protocol: tcp udp + method: none + log: error +}