[httpcomponents-client] branch 5.4.x updated: HTTPCLIENT-2293 Enforce port specification for CONNECT requests as per RFC 9110 (#482)

[ab...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

abernal pushed a commit to branch 5.4.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git


The following commit(s) were added to refs/heads/5.4.x by this push:
     new 3cac2e675 HTTPCLIENT-2293 Enforce port specification for CONNECT requests as per RFC 9110 (#482)
3cac2e675 is described below

commit 3cac2e6751a145fdf492c7c23c66e6c759ce7193
Author: Arturo Bernal <ab...@apache.org>
AuthorDate: Fri Sep 15 16:41:15 2023 +0200

    HTTPCLIENT-2293 Enforce port specification for CONNECT requests as per RFC 9110 (#482)
    
    - This adheres to RFC 9110, which states: "There is no default port; a client MUST send the port number even if the CONNECT request is based on a URI reference that contains an authority component with an elided port.
---
 .../hc/client5/http/impl/classic/ProxyClient.java  |  10 +-
 .../client5/http/impl/classic/TestProxyClient.java | 113 +++++++++++++++++++++
 2 files changed, 117 insertions(+), 6 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java
index 8b4965cef..3af70fd94 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java
@@ -140,12 +140,10 @@ public class ProxyClient {
         Args.notNull(proxy, "Proxy host");
         Args.notNull(target, "Target host");
         Args.notNull(credentials, "Credentials");
-        HttpHost host = target;
-        if (host.getPort() <= 0) {
-            host = new HttpHost(host.getSchemeName(), host.getHostName(), 80);
-        }
+        Args.check(target.getPort() > 0, "A valid port number must be provided for the tunnel CONNECT request.");
+
         final HttpRoute route = new HttpRoute(
-                host,
+                target,
                 null,
                 proxy, false, TunnelType.TUNNELLED, LayerType.PLAIN);
 
@@ -153,7 +151,7 @@ public class ProxyClient {
         final HttpContext context = new BasicHttpContext();
         ClassicHttpResponse response;
 
-        final ClassicHttpRequest connect = new BasicClassicHttpRequest(Method.CONNECT, proxy, host.toHostString());
+        final ClassicHttpRequest connect = new BasicClassicHttpRequest(Method.CONNECT, proxy, target.toHostString());
 
         final BasicCredentialsProvider credsProvider = new BasicCredentialsProvider();
         credsProvider.setCredentials(new AuthScope(proxy), credentials);
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProxyClient.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProxyClient.java
new file mode 100644
index 000000000..4c14e2c5e
--- /dev/null
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProxyClient.java
@@ -0,0 +1,113 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.http.impl.classic;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.net.Socket;
+
+import org.apache.hc.client5.http.auth.Credentials;
+import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
+import org.apache.hc.client5.http.config.RequestConfig;
+import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
+import org.apache.hc.core5.http.io.HttpConnectionFactory;
+import org.junit.jupiter.api.Test;
+
+class TestProxyClient {
+
+    @Test
+    void testTunnelWithInvalidPort() throws IOException {
+        // Mock dependencies
+        final HttpConnectionFactory<ManagedHttpClientConnection> connFactory = mock(HttpConnectionFactory.class);
+        final ManagedHttpClientConnection managedConnection = mock(ManagedHttpClientConnection.class);
+        when(connFactory.createConnection(null)).thenReturn(managedConnection);
+
+        final HttpRequestExecutor requestExecutor = mock(HttpRequestExecutor.class);
+        final ClassicHttpResponse response = mock(ClassicHttpResponse.class);
+        when(response.getCode()).thenReturn(200);
+        try {
+            when(requestExecutor.execute(any(), any(), any())).thenReturn(response);
+        } catch (final IOException | HttpException e) {
+            fail("Shouldn't fail");
+        }
+
+        final RequestConfig requestConfig = RequestConfig.DEFAULT;
+
+        final ProxyClient client = new ProxyClient(connFactory, null, null, requestConfig);
+
+        final HttpHost proxy = new HttpHost("proxy.example.com", 8080);
+        final HttpHost target = new HttpHost("target.example.com", -1); // Invalid port
+        final Credentials credentials = new UsernamePasswordCredentials("user", "pass".toCharArray());
+
+        assertThrows(IllegalArgumentException.class, () -> client.tunnel(proxy, target, credentials));
+    }
+
+    @Test
+    void testSuccessfulTunnel() throws IOException, HttpException {
+        // Mock dependencies
+        final HttpConnectionFactory<ManagedHttpClientConnection> connFactory = mock(HttpConnectionFactory.class);
+
+        final ManagedHttpClientConnection managedConnection = mock(ManagedHttpClientConnection.class);
+        when(managedConnection.isOpen()).thenReturn(true); // Always return true for isOpen()
+        when(connFactory.createConnection(null)).thenReturn(managedConnection);
+
+        final ClassicHttpResponse mockResponse = mock(ClassicHttpResponse.class);
+        when(mockResponse.getCode()).thenReturn(200); // Successful response
+        when(managedConnection.receiveResponseHeader()).thenReturn(mockResponse);
+
+        final HttpRequestExecutor mockRequestExecutor = mock(HttpRequestExecutor.class);
+        when(mockRequestExecutor.execute(any(), any(), any())).thenReturn(mockResponse);
+
+        final Socket mockSocket = mock(Socket.class);
+        when(managedConnection.getSocket()).thenReturn(mockSocket);
+
+        final RequestConfig requestConfig = RequestConfig.DEFAULT;
+
+        final ProxyClient client = new ProxyClient(connFactory, null, null, requestConfig);
+
+        final HttpHost proxy = new HttpHost("proxy.example.com", 8080);
+        final HttpHost target = new HttpHost("target.example.com", 80); // Valid port
+        final Credentials credentials = new UsernamePasswordCredentials("user", "pass".toCharArray());
+
+        final Socket resultSocket = client.tunnel(proxy, target, credentials);
+        assertNotNull(resultSocket, "Expected a valid socket object");
+        assertEquals(mockSocket, resultSocket, "Expected the mock socket to be returned");
+    }
+
+}
\ No newline at end of file