From 215571a0bdb08849dcee315d456f4ecc93b89863 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 18 Apr 2024 12:09:13 +0200 Subject: [PATCH] HTTPCLIENT-2326: Propagate original proxy response to the caller in case of HTTP CONNECT request failure --- .../http/impl/TunnelRefusedException.java | 34 +++++++++++++++++-- .../http/impl/async/AsyncConnectExec.java | 8 ++--- .../http/impl/classic/ConnectExec.java | 31 ++++++++++------- .../http/impl/classic/ProxyClient.java | 3 +- .../http/impl/classic/TestConnectExec.java | 4 +-- 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/TunnelRefusedException.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/TunnelRefusedException.java index 8d032fbdd..14ad335cc 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/TunnelRefusedException.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/TunnelRefusedException.java @@ -28,25 +28,53 @@ package org.apache.hc.client5.http.impl; import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.message.BasicHttpResponse; +import org.apache.hc.core5.http.message.StatusLine; +import org.conscrypt.Internal; /** * Signals that the tunnel request was rejected by the proxy host. * * @since 4.0 + * + * @deprecated Do not use/ */ +@Deprecated public class TunnelRefusedException extends HttpException { private static final long serialVersionUID = -8646722842745617323L; - private final String responseMessage; + private final HttpResponse response; + /** + * @deprecated Do not use. + */ + @Deprecated public TunnelRefusedException(final String message, final String responseMessage) { super(message); - this.responseMessage = responseMessage; + this.response = new BasicHttpResponse(500); } + @Internal + public TunnelRefusedException(final HttpResponse response) { + super("CONNECT refused by proxy: " + new StatusLine(response)); + this.response = response; + } + + /** + * @deprecated Use {@link #getResponse()}. + */ + @Deprecated public String getResponseMessage() { - return this.responseMessage; + return "CONNECT refused by proxy: " + new StatusLine(response); + } + + /** + * @since 5.4 + */ + public HttpResponse getResponse() { + return response; } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java index f9fd61e34..eb85133ad 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java @@ -45,7 +45,6 @@ import org.apache.hc.client5.http.async.AsyncExecRuntime; import org.apache.hc.client5.http.auth.AuthExchange; import org.apache.hc.client5.http.auth.ChallengeType; import org.apache.hc.client5.http.config.RequestConfig; -import org.apache.hc.client5.http.impl.TunnelRefusedException; import org.apache.hc.client5.http.impl.auth.AuthCacheKeeper; import org.apache.hc.client5.http.impl.auth.HttpAuthenticator; import org.apache.hc.client5.http.impl.routing.BasicRouteDirector; @@ -121,8 +120,8 @@ public final class AsyncConnectExec implements AsyncExecChainHandler { final RouteTracker tracker; volatile boolean challenged; + volatile HttpResponse response; volatile boolean tunnelRefused; - volatile HttpResponse tunnelResponse; } @@ -295,7 +294,7 @@ public final class AsyncConnectExec implements AsyncExecChainHandler { if (LOG.isDebugEnabled()) { LOG.debug("{} tunnel refused", exchangeId); } - asyncExecCallback.failed(new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(state.tunnelResponse), null)); + asyncExecCallback.completed(); } else { if (LOG.isDebugEnabled()) { LOG.debug("{} tunnel to target created", exchangeId); @@ -456,8 +455,7 @@ public final class AsyncConnectExec implements AsyncExecChainHandler { state.challenged = false; if (status >= HttpStatus.SC_REDIRECTION) { state.tunnelRefused = true; - state.tunnelResponse = response; - entityConsumerRef.set(null); + entityConsumerRef.set(asyncExecCallback.handleResponse(response, entityDetails)); } else if (status == HttpStatus.SC_OK) { asyncExecCallback.completed(); } else { diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java index 03578ad3e..1c5cbab75 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java @@ -40,7 +40,6 @@ import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecChainHandler; import org.apache.hc.client5.http.classic.ExecRuntime; import org.apache.hc.client5.http.config.RequestConfig; -import org.apache.hc.client5.http.impl.TunnelRefusedException; import org.apache.hc.client5.http.impl.auth.AuthCacheKeeper; import org.apache.hc.client5.http.impl.auth.HttpAuthenticator; import org.apache.hc.client5.http.impl.routing.BasicRouteDirector; @@ -52,6 +51,7 @@ import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.ConnectionReuseStrategy; +import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHeaders; @@ -60,6 +60,7 @@ import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.Method; +import org.apache.hc.core5.http.io.entity.ByteArrayEntity; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.StatusLine; @@ -149,11 +150,15 @@ public final class ConnectExec implements ExecChainHandler { tracker.connectProxy(proxy, route.isSecure() && !route.isTunnelled()); break; case HttpRouteDirector.TUNNEL_TARGET: { - final boolean secure = createTunnelToTarget(exchangeId, route, request, execRuntime, context); + final ClassicHttpResponse finalResponse = createTunnelToTarget( + exchangeId, route, request, execRuntime, context); + if (finalResponse != null) { + return finalResponse; + } if (LOG.isDebugEnabled()) { LOG.debug("{} tunnel to target created.", exchangeId); } - tracker.tunnelTarget(secure); + tracker.tunnelTarget(false); } break; case HttpRouteDirector.TUNNEL_PROXY: { @@ -207,7 +212,7 @@ public final class ConnectExec implements ExecChainHandler { * This method does not processChallenge the connection with * information about the tunnel, that is left to the caller. */ - private boolean createTunnelToTarget( + private ClassicHttpResponse createTunnelToTarget( final String exchangeId, final HttpRoute route, final HttpRequest request, @@ -282,16 +287,16 @@ public final class ConnectExec implements ExecChainHandler { final int status = response.getCode(); if (status != HttpStatus.SC_OK) { - EntityUtils.consume(response.getEntity()); - execRuntime.disconnectEndpoint(); - throw new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(response), null); + final HttpEntity entity = response.getEntity(); + if (entity != null) { + response.setEntity(new ByteArrayEntity( + EntityUtils.toByteArray(entity, 4096), + ContentType.parseLenient(entity.getContentType()))); + execRuntime.disconnectEndpoint(); + } + return response; } - - // How to decide on security of the tunnelled connection? - // The socket factory knows only about the segment to the proxy. - // Even if that is secure, the hop to the target may be insecure. - // Leave it to derived classes, consider insecure by default here. - return false; + return null; } /** 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 e112ec8a9..a4657a26a 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 @@ -43,7 +43,6 @@ import org.apache.hc.client5.http.auth.StandardAuthScheme; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy; import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy; -import org.apache.hc.client5.http.impl.TunnelRefusedException; import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; import org.apache.hc.client5.http.impl.auth.BasicSchemeFactory; import org.apache.hc.client5.http.impl.auth.DigestSchemeFactory; @@ -202,7 +201,7 @@ public class ProxyClient { if (status > 299) { EntityUtils.consume(response.getEntity()); conn.close(); - throw new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(response), null); + throw new HttpException("Tunnel refused: " + new StatusLine(response)); } return conn.getSocket(); } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java index 46ea25d7b..a1716d4b2 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java @@ -41,7 +41,6 @@ import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecRuntime; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.entity.EntityBuilder; -import org.apache.hc.client5.http.impl.TunnelRefusedException; import org.apache.hc.client5.http.impl.auth.BasicScheme; import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -218,9 +217,8 @@ public class TestConnectExec { Mockito.any())).thenReturn(response); final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); - Assertions.assertThrows(TunnelRefusedException.class, () -> exec.execute(request, scope, execChain)); + exec.execute(request, scope, execChain); Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint(); - Mockito.verify(execRuntime).discardEndpoint(); } @Test