From a0a38afbeef3a4464ebdf51f0f1699280ac37569 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 25 Apr 2024 10:51:56 +0200 Subject: [PATCH] Made handling of refused tunnel exception consistent between the classic and async protocol handler implementations; Proxy response body no longer gets buffered in memory in order to avoid excessive resource allocation in case of a proxy misbehavior --- .../hc/client5/http/impl/async/AsyncConnectExec.java | 6 ++++-- .../apache/hc/client5/http/impl/classic/ConnectExec.java | 7 ++----- .../apache/hc/client5/http/impl/classic/ProxyClient.java | 7 ++----- .../hc/client5/http/impl/classic/TestConnectExec.java | 4 +--- 4 files changed, 9 insertions(+), 15 deletions(-) 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 40c87b8ee..f9fd61e34 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 @@ -122,6 +122,7 @@ public final class AsyncConnectExec implements AsyncExecChainHandler { volatile boolean challenged; volatile boolean tunnelRefused; + volatile HttpResponse tunnelResponse; } @@ -294,7 +295,7 @@ public final class AsyncConnectExec implements AsyncExecChainHandler { if (LOG.isDebugEnabled()) { LOG.debug("{} tunnel refused", exchangeId); } - asyncExecCallback.failed(new TunnelRefusedException("Tunnel refused", null)); + asyncExecCallback.failed(new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(state.tunnelResponse), null)); } else { if (LOG.isDebugEnabled()) { LOG.debug("{} tunnel to target created", exchangeId); @@ -455,7 +456,8 @@ public final class AsyncConnectExec implements AsyncExecChainHandler { state.challenged = false; if (status >= HttpStatus.SC_REDIRECTION) { state.tunnelRefused = true; - entityConsumerRef.set(asyncExecCallback.handleResponse(response, entityDetails)); + state.tunnelResponse = response; + entityConsumerRef.set(null); } 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 27fa438e7..03578ad3e 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 @@ -282,12 +282,9 @@ public final class ConnectExec implements ExecChainHandler { final int status = response.getCode(); if (status != HttpStatus.SC_OK) { - - // Buffer response content - final HttpEntity entity = response.getEntity(); - final String responseMessage = entity != null ? EntityUtils.toString(entity) : null; + EntityUtils.consume(response.getEntity()); execRuntime.disconnectEndpoint(); - throw new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(response), responseMessage); + throw new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(response), null); } // How to decide on security of the tunnelled connection? 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 29394f2dd..e112ec8a9 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 @@ -200,12 +200,9 @@ public class ProxyClient { final int status = response.getCode(); if (status > 299) { - - // Buffer response content - final HttpEntity entity = response.getEntity(); - final String responseMessage = entity != null ? EntityUtils.toString(entity) : null; + EntityUtils.consume(response.getEntity()); conn.close(); - throw new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(response), responseMessage); + throw new TunnelRefusedException("CONNECT refused by proxy: " + new StatusLine(response), null); } 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 d9118ee0f..46ea25d7b 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 @@ -218,9 +218,7 @@ public class TestConnectExec { Mockito.any())).thenReturn(response); final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); - final TunnelRefusedException exception = Assertions.assertThrows(TunnelRefusedException.class, () -> - exec.execute(request, scope, execChain)); - Assertions.assertEquals("Ka-boom", exception.getResponseMessage()); + Assertions.assertThrows(TunnelRefusedException.class, () -> exec.execute(request, scope, execChain)); Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint(); Mockito.verify(execRuntime).discardEndpoint(); }