HTTPCLIENT-2292: HttpClient ignores socketConfig#getSocksProxyAddress

apache · Sep 15, 2023 · 404a3ab · 404a3ab
1 parent 8da4783
commit 404a3ab
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 67 deletions.
diff --git a/...src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java b/...src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java
@@ -29,7 +29,9 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.net.Proxy;
 import java.net.Socket;
+import java.net.SocketAddress;
 import java.util.Arrays;
 
 import org.apache.hc.client5.http.ConnectExceptionSupport;
@@ -146,13 +148,14 @@ public void connect(
         }
 
         final Timeout soTimeout = socketConfig.getSoTimeout();
-
+        final SocketAddress socksProxyAddress = socketConfig.getSocksProxyAddress();
+        final Proxy proxy = socksProxyAddress != null ? new Proxy(Proxy.Type.SOCKS, socksProxyAddress) : null;
         final int port = this.schemePortResolver.resolve(host);
         for (int i = 0; i < remoteAddresses.length; i++) {
             final InetAddress address = remoteAddresses[i];
             final boolean last = i == remoteAddresses.length - 1;
 
-            Socket sock = sf.createSocket(context);
+            Socket sock = sf.createSocket(proxy, context);
             if (soTimeout != null) {
                 sock.setSoTimeout(soTimeout.toMillisecondsIntBound());
             }

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/ConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/ConnectionSocketFactory.java
@@ -29,9 +29,11 @@
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
+import java.net.Proxy;
 import java.net.Socket;
 
 import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.protocol.HttpContext;
@@ -50,13 +52,21 @@ public interface ConnectionSocketFactory {
      * Creates new, unconnected socket. The socket should subsequently be passed to
      * {@link #connectSocket(TimeValue, Socket, HttpHost, InetSocketAddress, InetSocketAddress,
      *    HttpContext) connectSocket} method.
-     *
-     * @return  a new socket
-     *
-     * @throws IOException if an I/O error occurs while creating the socket
      */
     Socket createSocket(HttpContext context) throws IOException;
 
+    /**
+     * Creates new, unconnected socket via a proxy (generally SOCKS is expected).
+     * The socket should subsequently be passed to {@link #connectSocket(TimeValue, Socket,
+     * HttpHost, InetSocketAddress, InetSocketAddress, HttpContext) connectSocket} method.
+     *
+     * @since 5.2
+     */
+    @Internal
+    default Socket createSocket(Proxy proxy, HttpContext context) throws IOException {
+        return createSocket(context);
+    }
+
     /**
      * Connects the socket to the target host with the given resolved remote address.
      *

diff --git a/...client5/src/main/java/org/apache/hc/client5/http/socket/PlainConnectionSocketFactory.java b/...client5/src/main/java/org/apache/hc/client5/http/socket/PlainConnectionSocketFactory.java
@@ -29,6 +29,7 @@
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
+import java.net.Proxy;
 import java.net.Socket;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
@@ -60,6 +61,11 @@ public PlainConnectionSocketFactory() {
         super();
     }
 
+    @Override
+    public Socket createSocket(final Proxy proxy, final HttpContext context) throws IOException {
+        return proxy != null ? new Socket(proxy) : new Socket();
+    }
+
     @Override
     public Socket createSocket(final HttpContext context) throws IOException {
         return new Socket();

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java
@@ -30,6 +30,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetSocketAddress;
+import java.net.Proxy;
 import java.net.Socket;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
@@ -39,7 +40,6 @@
 import java.util.List;
 import java.util.regex.Pattern;
 
-import javax.net.SocketFactory;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLException;
@@ -200,7 +200,12 @@ protected void prepareSocket(final SSLSocket socket, final HttpContext context)
 
     @Override
     public Socket createSocket(final HttpContext context) throws IOException {
-        return SocketFactory.getDefault().createSocket();
+        return new Socket();
+    }
+
+    @Override
+    public Socket createSocket(final Proxy proxy, final HttpContext context) throws IOException {
+        return new Socket(proxy);
     }
 
     @Override

diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientExecuteSOCKS.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientExecuteSOCKS.java
@@ -27,25 +27,17 @@
 
 package org.apache.hc.client5.http.examples;
 
-import java.io.IOException;
 import java.net.InetSocketAddress;
-import java.net.Proxy;
-import java.net.Socket;
 
 import org.apache.hc.client5.http.classic.methods.HttpGet;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
-import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
 import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.config.Registry;
-import org.apache.hc.core5.http.config.RegistryBuilder;
 import org.apache.hc.core5.http.io.SocketConfig;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.http.message.StatusLine;
-import org.apache.hc.core5.http.protocol.HttpContext;
-import org.apache.hc.core5.util.TimeValue;
 
 /**
  * How to send a request via SOCKS proxy.
@@ -55,10 +47,7 @@
 public class ClientExecuteSOCKS {
 
     public static void main(final String[] args)throws Exception {
-        final Registry<ConnectionSocketFactory> reg = RegistryBuilder.<ConnectionSocketFactory>create()
-                .register("http", new MyConnectionSocketFactory())
-                .build();
-        final InetSocketAddress socksaddr = new InetSocketAddress("mysockshost", 1234);
+        final InetSocketAddress socksaddr = new InetSocketAddress("localhost", 1080);
         final PoolingHttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
                 .setDefaultSocketConfig(SocketConfig.custom()
                         .setSocksProxyAddress(socksaddr)
@@ -82,36 +71,4 @@ public static void main(final String[] args)throws Exception {
         }
     }
 
-    static class MyConnectionSocketFactory implements ConnectionSocketFactory {
-
-        @Override
-        public Socket createSocket(final HttpContext context) throws IOException {
-            final InetSocketAddress socksaddr = (InetSocketAddress) context.getAttribute("socks.address");
-            final Proxy proxy = new Proxy(Proxy.Type.SOCKS, socksaddr);
-            return new Socket(proxy);
-        }
-
-        @Override
-        public Socket connectSocket(
-                final TimeValue connectTimeout,
-                final Socket socket,
-                final HttpHost host,
-                final InetSocketAddress remoteAddress,
-                final InetSocketAddress localAddress,
-                final HttpContext context) throws IOException {
-            final Socket sock;
-            if (socket != null) {
-                sock = socket;
-            } else {
-                sock = createSocket(context);
-            }
-            if (localAddress != null) {
-                sock.bind(localAddress);
-            }
-            sock.connect(remoteAddress, connectTimeout != null ? connectTimeout.toMillisecondsIntBound() : 0);
-            return sock;
-        }
-
-    }
-
 }
diff --git a/...rc/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java b/...rc/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java
@@ -375,7 +375,7 @@ public void testTargetConnect() throws Exception {
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] {remote});
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(plainSocketFactory);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.eq(socket),
                 Mockito.any(),
@@ -389,7 +389,7 @@ public void testTargetConnect() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(null, context);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
                 socket,
                 target,
@@ -403,7 +403,7 @@ public void testTargetConnect() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(2)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(2)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(2)).createSocket(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(2)).createSocket(null, context);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
                 socket,
                 target,
@@ -447,7 +447,7 @@ public void testProxyConnectAndUpgrade() throws Exception {
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.eq(socket),
                 Mockito.any(),
@@ -461,7 +461,7 @@ public void testProxyConnectAndUpgrade() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("someproxy");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(proxy);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(null, context);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
                 socket,
                 proxy,

diff --git a/...t5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java b/...t5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java
@@ -91,7 +91,7 @@ public void testConnect() throws Exception {
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.any(),
                 Mockito.any(),
@@ -141,7 +141,7 @@ public void testConnectTimeout() throws Exception {
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.any(),
                 Mockito.any(),
@@ -166,7 +166,7 @@ public void testConnectFailure() throws Exception {
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.any(),
                 Mockito.any(),
@@ -192,7 +192,7 @@ public void testConnectFailover() throws Exception {
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.any(),
                 Mockito.any(),
@@ -236,7 +236,7 @@ public void testConnectExplicitAddress() throws Exception {
 
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.any(),
                 Mockito.any(),

diff --git a/.../test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java b/.../test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java
@@ -260,7 +260,7 @@ public void testTargetConnect() throws Exception {
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[]{remote});
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(plainSocketFactory);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
+        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.eq(socket),
                 Mockito.any(),
@@ -274,7 +274,7 @@ public void testTargetConnect() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(null, context);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
                 socket,
                 target,
@@ -288,7 +288,7 @@ public void testTargetConnect() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(2)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(2)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(2)).createSocket(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(2)).createSocket(null, context);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
                 socket,
                 target,
@@ -345,7 +345,7 @@ public void testProxyConnectAndUpgrade() throws Exception {
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
         Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainsf);
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslsf);
-        Mockito.when(plainsf.createSocket(Mockito.any())).thenReturn(mockSock);
+        Mockito.when(plainsf.createSocket(Mockito.any(), Mockito.any())).thenReturn(mockSock);
         Mockito.when(plainsf.connectSocket(
                 Mockito.eq(mockSock),
                 Mockito.any(),
@@ -359,7 +359,7 @@ public void testProxyConnectAndUpgrade() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("someproxy");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(proxy);
-        Mockito.verify(plainsf, Mockito.times(1)).createSocket(context);
+        Mockito.verify(plainsf, Mockito.times(1)).createSocket(null, context);
         Mockito.verify(plainsf, Mockito.times(1)).connectSocket(
                 mockSock,
                 proxy,