From ff87bf0075c92527c77931f93433894046e367c9 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 20 Jan 2025 17:58:57 +0100 Subject: [PATCH] HTTPCLIENT-2357, regression: Classic HttpClient fails to release connect in case of a proxy authentication failure --- .../ApacheHTTPDSquidCompatibilityIT.java | 22 ++++ ...HttpAsyncClientProxyCompatibilityTest.java | 102 ++++++++++++++++++ .../HttpClientProxyCompatibilityTest.java | 96 +++++++++++++++++ .../http/impl/classic/ConnectExec.java | 2 +- .../http/impl/classic/TestConnectExec.java | 2 +- 5 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/async/HttpAsyncClientProxyCompatibilityTest.java create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/sync/HttpClientProxyCompatibilityTest.java diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/ApacheHTTPDSquidCompatibilityIT.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/ApacheHTTPDSquidCompatibilityIT.java index 66ef1cda9..68ed99e54 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/ApacheHTTPDSquidCompatibilityIT.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/ApacheHTTPDSquidCompatibilityIT.java @@ -30,8 +30,10 @@ import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; import org.apache.hc.client5.testing.compatibility.async.CachingHttpAsyncClientCompatibilityTest; import org.apache.hc.client5.testing.compatibility.async.HttpAsyncClientCompatibilityTest; import org.apache.hc.client5.testing.compatibility.async.HttpAsyncClientHttp1CompatibilityTest; +import org.apache.hc.client5.testing.compatibility.async.HttpAsyncClientProxyCompatibilityTest; import org.apache.hc.client5.testing.compatibility.sync.CachingHttpClientCompatibilityTest; import org.apache.hc.client5.testing.compatibility.sync.HttpClientCompatibilityTest; +import org.apache.hc.client5.testing.compatibility.sync.HttpClientProxyCompatibilityTest; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.URIScheme; import org.apache.hc.core5.http2.HttpVersionPolicy; @@ -323,4 +325,24 @@ class ApacheHTTPDSquidCompatibilityIT { } + @Nested + @DisplayName("Classic client: HTTP/1.1, connection via password protected proxy") + class HttpClientProxy extends HttpClientProxyCompatibilityTest { + + public HttpClientProxy() throws Exception { + super(targetInternalTlsHost(), proxyPwProtectedContainerHost()); + } + + } + + @Nested + @DisplayName("Async client: HTTP/1.1, connection via password protected proxy") + class AsyncClientProxy extends HttpAsyncClientProxyCompatibilityTest { + + public AsyncClientProxy() throws Exception { + super(targetInternalTlsHost(), proxyPwProtectedContainerHost()); + } + + } + } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/async/HttpAsyncClientProxyCompatibilityTest.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/async/HttpAsyncClientProxyCompatibilityTest.java new file mode 100644 index 000000000..9231a4c78 --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/async/HttpAsyncClientProxyCompatibilityTest.java @@ -0,0 +1,102 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.testing.compatibility.async; + +import java.util.concurrent.Future; + +import org.apache.hc.client5.http.ContextBuilder; +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +import org.apache.hc.client5.http.auth.AuthScope; +import org.apache.hc.client5.http.auth.Credentials; +import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; +import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; +import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.testing.extension.async.HttpAsyncClientResource; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http2.HttpVersionPolicy; +import org.apache.hc.core5.util.Timeout; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public abstract class HttpAsyncClientProxyCompatibilityTest { + + static final Timeout TIMEOUT = Timeout.ofSeconds(5); + + private final HttpHost target; + private final HttpHost proxy; + @RegisterExtension + private final HttpAsyncClientResource clientResource; + private final BasicCredentialsProvider credentialsProvider; + + public HttpAsyncClientProxyCompatibilityTest(final HttpHost target, final HttpHost proxy) throws Exception { + this.target = target; + this.proxy = proxy; + this.clientResource = new HttpAsyncClientResource(HttpVersionPolicy.FORCE_HTTP_1); + this.clientResource.configure(builder -> builder.setProxy(proxy)); + this.credentialsProvider = new BasicCredentialsProvider(); + } + + CloseableHttpAsyncClient client() { + return clientResource.client(); + } + + HttpClientContext context() { + return ContextBuilder.create() + .useCredentialsProvider(credentialsProvider) + .build(); + } + + void addCredentials(final AuthScope authScope, final Credentials credentials) { + credentialsProvider.setCredentials(authScope, credentials); + } + + @Test + void test_auth_failure_wrong_proxy_credentials() throws Exception { + addCredentials( + new AuthScope(proxy), + new UsernamePasswordCredentials("testuser", "wrong password".toCharArray())); + final CloseableHttpAsyncClient client = client(); + + for (int i = 0; i < 10; i++) { + final HttpClientContext context = context(); + + final SimpleHttpRequest httpGetSecret = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/") + .build(); + final Future future = client.execute(httpGetSecret, context, null); + final SimpleHttpResponse response = future.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + Assertions.assertEquals(HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED, response.getCode()); + } + } + +} diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/sync/HttpClientProxyCompatibilityTest.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/sync/HttpClientProxyCompatibilityTest.java new file mode 100644 index 000000000..bbce32a29 --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/compatibility/sync/HttpClientProxyCompatibilityTest.java @@ -0,0 +1,96 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.testing.compatibility.sync; + +import org.apache.hc.client5.http.ContextBuilder; +import org.apache.hc.client5.http.auth.AuthScope; +import org.apache.hc.client5.http.auth.Credentials; +import org.apache.hc.client5.http.auth.CredentialsStore; +import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; +import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.testing.extension.sync.HttpClientResource; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public abstract class HttpClientProxyCompatibilityTest { + + private final HttpHost target; + private final HttpHost proxy; + @RegisterExtension + private final HttpClientResource clientResource; + private final CredentialsStore credentialsProvider; + + public HttpClientProxyCompatibilityTest(final HttpHost target, final HttpHost proxy) throws Exception { + this.target = target; + this.proxy = proxy; + this.clientResource = new HttpClientResource(); + this.clientResource.configure(builder -> builder.setProxy(proxy)); + this.credentialsProvider = new BasicCredentialsProvider(); + } + + CloseableHttpClient client() { + return clientResource.client(); + } + + HttpClientContext context() { + return ContextBuilder.create() + .useCredentialsProvider(credentialsProvider) + .build(); + } + + void addCredentials(final AuthScope authScope, final Credentials credentials) { + credentialsProvider.setCredentials(authScope, credentials); + } + + @Test + void test_auth_failure_wrong_proxy_credentials() throws Exception { + addCredentials(new AuthScope(proxy), + new UsernamePasswordCredentials("testuser", "wrong password".toCharArray())); + + final CloseableHttpClient client = client(); + + for (int i = 0; i < 10; i++) { + final HttpClientContext context = context(); + + final ClassicHttpRequest request = new HttpGet("/"); + try (ClassicHttpResponse response = client.executeOpen(target, request, context)) { + Assertions.assertEquals(HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED, response.getCode()); + EntityUtils.consume(response.getEntity()); + } + } + } + +} 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 c218797af..de52644f2 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 @@ -295,7 +295,7 @@ public final class ConnectExec implements ExecChainHandler { response.setEntity(new ByteArrayEntity( EntityUtils.toByteArray(entity, 4096), ContentType.parseLenient(entity.getContentType()))); - execRuntime.disconnectEndpoint(); + execRuntime.discardEndpoint(); } return response; } 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 c6e6e912d..1cc9a5504 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,7 +218,7 @@ class TestConnectExec { final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); exec.execute(request, scope, execChain); - Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint(); + Mockito.verify(execRuntime, Mockito.atLeastOnce()).discardEndpoint(); } @Test