From 408af8734026c4e874cf7ef2ff86279cf446f43e Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Tue, 14 Oct 2025 19:12:04 +0200 Subject: [PATCH] HTTPCLIENT-2401: Do not attempt RFC 2817 TLS upgrade on proxied routes RequestUpgrade skips adding Upgrade/Connection when a proxy is present (tunnelled or not); TLS over proxies must use CONNECT. --- .../client5/http/protocol/RequestUpgrade.java | 9 ++- .../http/protocol/TestRequestUpgrade.java | 63 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestUpgrade.java b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestUpgrade.java index c402d4da68..a473545e28 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestUpgrade.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestUpgrade.java @@ -29,6 +29,7 @@ import java.io.IOException; +import org.apache.hc.client5.http.RouteInfo; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -39,6 +40,7 @@ import org.apache.hc.core5.http.HttpRequestInterceptor; import org.apache.hc.core5.http.Method; import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.URIScheme; import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.util.Args; import org.slf4j.Logger; @@ -65,7 +67,12 @@ public void process( final HttpClientContext clientContext = HttpClientContext.cast(context); final RequestConfig requestConfig = clientContext.getRequestConfigOrDefault(); - if (requestConfig.isProtocolUpgradeEnabled()) { + final RouteInfo route = clientContext.getHttpRoute(); + final String scheme = request.getScheme(); + if (scheme != null && !URIScheme.HTTP.same(scheme)) { + return; + } + if (requestConfig.isProtocolUpgradeEnabled() && (route == null || route.getProxyHost() == null)) { final ProtocolVersion version = request.getVersion() != null ? request.getVersion() : clientContext.getProtocolVersion(); if (!request.containsHeader(HttpHeaders.UPGRADE) && !request.containsHeader(HttpHeaders.CONNECTION) && diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestUpgrade.java b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestUpgrade.java index 191841d90e..4e0e08d70d 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestUpgrade.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestUpgrade.java @@ -30,10 +30,13 @@ import javax.net.ssl.SSLSession; import org.apache.hc.client5.http.HeadersMatcher; +import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpVersion; +import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.http.message.BasicHttpRequest; import org.hamcrest.MatcherAssert; @@ -136,4 +139,64 @@ void testDoUpgradeNonSafeMethodsOrTrace() throws Exception { Assertions.assertFalse(trace.containsHeader(HttpHeaders.UPGRADE)); } + @Test + void doesNotAddUpgradeHeadersWhenProxyPresent() throws Exception { + final RequestUpgrade interceptor = new RequestUpgrade(); + + final HttpHost target = new HttpHost("http", "example.com", 80); + final HttpHost proxy = new HttpHost("http", "proxy.local", 8080); + + final HttpClientContext ctx = HttpClientContext.create(); + final HttpRoute route = new HttpRoute(target, null, proxy, false); + ctx.setRoute(route); + ctx.setRequestConfig(RequestConfig.custom() + .setProtocolUpgradeEnabled(true) + .build()); + + final BasicClassicHttpRequest request = new BasicClassicHttpRequest("GET", "/"); + request.setScheme("http"); + + interceptor.process(request, null, ctx); + + Assertions.assertNull(request.getFirstHeader(HttpHeaders.UPGRADE), "Upgrade header must be absent over proxy"); + Assertions.assertNull(request.getFirstHeader(HttpHeaders.CONNECTION), "Connection: upgrade must be absent over proxy"); + } + + @Test + void doesNotAddUpgradeHeadersWhenSchemeIsHttps() throws Exception { + final RequestUpgrade interceptor = new RequestUpgrade(); + + final HttpClientContext ctx = HttpClientContext.create(); + ctx.setRequestConfig(RequestConfig.custom() + .setProtocolUpgradeEnabled(true) + .build()); + + final BasicClassicHttpRequest request = new BasicClassicHttpRequest("GET", "/"); + request.setVersion(HttpVersion.HTTP_1_1); + request.setScheme("https"); + + interceptor.process(request, null, ctx); + + Assertions.assertNull(request.getFirstHeader(HttpHeaders.UPGRADE)); + Assertions.assertNull(request.getFirstHeader(HttpHeaders.CONNECTION)); + } + + @Test + void addsUpgradeHeadersWhenSchemeIsHttp() throws Exception { + final RequestUpgrade interceptor = new RequestUpgrade(); + + final HttpClientContext ctx = HttpClientContext.create(); + ctx.setRequestConfig(RequestConfig.custom() + .setProtocolUpgradeEnabled(true) + .build()); + + final BasicClassicHttpRequest request = new BasicClassicHttpRequest("GET", "/"); + request.setVersion(HttpVersion.HTTP_1_1); + request.setScheme("http"); + + interceptor.process(request, null, ctx); + + Assertions.assertNotNull(request.getFirstHeader(HttpHeaders.UPGRADE)); + Assertions.assertNotNull(request.getFirstHeader(HttpHeaders.CONNECTION)); + } }