Bump actions/cache from 4 to 6

.github/workflows/maven.yml

@@ -43,7 +43,7 @@ jobs:
 
     steps:
     - uses: actions/checkout@v7
-    - uses: actions/cache@v4
+    - uses: actions/cache@v6
       with:
         path: ~/.m2/repository
         key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}

httpclient5-testing/pom.xml

@@ -126,7 +126,7 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
-          <excludedGroups>slow,time-out,stale-conn</excludedGroups>
+          <excludedGroups>slow,time-out,stale-conn,cancellation</excludedGroups>
         </configuration>
       </plugin>
     </plugins>

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1Async.java

@@ -46,6 +46,7 @@
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.URIScheme;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
@@ -139,6 +140,7 @@ void testSharedPool() throws Exception {
         Assertions.assertEquals(2048, body3.length());
     }
 
+    @Tag("cancellation")
     @Test
     void testRequestCancellation() throws Exception {
         configureServer(bootstrap -> bootstrap.register("/random/*", AsyncRandomHandler::new));

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java

@@ -65,7 +65,7 @@
 import org.apache.hc.core5.net.URIBuilder;
 import org.apache.hc.core5.util.TimeValue;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -316,7 +316,8 @@ void testAbsoluteRequestURIWithFragment() throws Exception {
         Assertions.assertEquals(uri, location);
     }
 
-    @Test @Disabled("Fails intermittently with GitHub Actions")
+    @Tag("cancellation")
+    @Test
     void testRequestCancellation() throws Exception {
         startServer();
         final HttpHost target = startServer();

httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java

@@ -217,7 +217,8 @@ public AsyncDataConsumer handleResponse(
                 if (needAuthentication(
                         targetAuthExchange,
                         proxyAuthExchange,
-                        proxy != null ? proxy : target,
+                        // A 407 received through a tunnel comes from the origin, not from the proxy
+                        route.isTunnelled() ? null : proxy,
                         target,
                         pathPrefix,
                         response,
@@ -319,11 +320,11 @@ private boolean needAuthentication(
                 }
             }
 
-            final boolean proxyAuthRequested = authenticator.isChallenged(
+            final boolean proxyAuthRequested = proxy != null && authenticator.isChallenged(
                     proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
-            final boolean proxyMutualAuthRequired = authenticator.isChallengeExpected(proxyAuthExchange);
+            final boolean proxyMutualAuthRequired = proxy != null && authenticator.isChallengeExpected(proxyAuthExchange);
 
-            if (authCacheKeeper != null) {
+            if (proxy != null && authCacheKeeper != null) {
                 if (proxyAuthRequested) {
                     authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context);
                 } else {

httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java

@@ -207,7 +207,8 @@ public ClassicHttpResponse execute(
                 if (needAuthentication(
                         targetAuthExchange,
                         proxyAuthExchange,
-                        proxy != null ? proxy : target,
+                        // A 407 received through a tunnel comes from the origin, not from the proxy
+                        route.isTunnelled() ? null : proxy,
                         target,
                         pathPrefix,
                         response,
@@ -280,11 +281,11 @@ private boolean needAuthentication(
                 }
             }
 
-            final boolean proxyAuthRequested = authenticator.isChallenged(
+            final boolean proxyAuthRequested = proxy != null && authenticator.isChallenged(
                     proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
-            final boolean proxyMutualAuthRequired = authenticator.isChallengeExpected(proxyAuthExchange);
+            final boolean proxyMutualAuthRequired = proxy != null && authenticator.isChallengeExpected(proxyAuthExchange);
 
-            if (authCacheKeeper != null) {
+            if (proxy != null && authCacheKeeper != null) {
                 if (proxyAuthRequested) {
                     authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context);
                 } else {

httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java

@@ -491,7 +491,8 @@ public void connect(final ConnectionEndpoint endpoint, final TimeValue timeout,
             this.connectionOperator.connect(
                     connection,
                     firstHop,
-                    route.getTargetName(),
+                    // The target name applies to the target only, never to an intermediate proxy
+                    route.getProxyHost() != null ? null : route.getTargetName(),
                     route.getLocalSocketAddress(),
                     connectTimeout,
                     socketConfig,

httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java

@@ -565,7 +565,8 @@ public void connect(final ConnectionEndpoint endpoint, final TimeValue timeout,
         this.connectionOperator.connect(
                 conn,
                 firstHop,
-                route.getTargetName(),
+                // The target name applies to the target only, never to an intermediate proxy
+                route.getProxyHost() != null ? null : route.getTargetName(),
                 unixDomainSocket,
                 route.getLocalSocketAddress(),
                 connectTimeout,

httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java

@@ -523,7 +523,8 @@ public Future<AsyncConnectionEndpoint> connect(
                 connectionInitiator,
                 firstHop,
                 unixDomainSocket,
-                route.getTargetName(),
+                // The target name applies to the target only, never to an intermediate proxy
+                route.getProxyHost() != null ? null : route.getTargetName(),
                 route.getLocalSocketAddress(),
                 connectTimeout,
                 connectAttachment,

httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java

@@ -99,6 +99,10 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate
         this.sslBufferManagement = sslBufferManagement != null ? sslBufferManagement : SSLBufferMode.STATIC;
         this.hostnameVerificationPolicy = hostnameVerificationPolicy != null ? hostnameVerificationPolicy :
                 (hostnameVerifier != null ? HostnameVerificationPolicy.BOTH : HostnameVerificationPolicy.BUILTIN);
+        Args.check(this.hostnameVerificationPolicy != HostnameVerificationPolicy.CLIENT
+                        && this.hostnameVerificationPolicy != HostnameVerificationPolicy.BOTH
+                        || hostnameVerifier != null,
+                "Hostname verifier is required for hostname verification policy " + this.hostnameVerificationPolicy);
         this.hostnameVerifier = hostnameVerifier;
     }
 

httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java

@@ -69,7 +69,8 @@ public ConscryptClientTlsStrategy(
             final String[] supportedCipherSuites,
             final SSLBufferMode sslBufferManagement,
             final HostnameVerifier hostnameVerifier) {
-        this(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, HostnameVerificationPolicy.CLIENT, hostnameVerifier);
+        this(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement,
+                hostnameVerifier != null ? HostnameVerificationPolicy.CLIENT : HostnameVerificationPolicy.BUILTIN, hostnameVerifier);
     }
 
     /**

httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java

@@ -101,7 +101,8 @@ public DefaultClientTlsStrategy(
             final SSLBufferMode sslBufferManagement,
             final HostnameVerifier hostnameVerifier,
             final Factory<SSLEngine, TlsDetails> tlsDetailsFactory) {
-        super(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, HostnameVerificationPolicy.CLIENT, hostnameVerifier);
+        super(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement,
+                hostnameVerifier != null ? HostnameVerificationPolicy.CLIENT : HostnameVerificationPolicy.BUILTIN, hostnameVerifier);
         this.tlsDetailsFactory = tlsDetailsFactory;
     }
 

httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java

@@ -27,18 +27,22 @@
 package org.apache.hc.client5.http.impl.async;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.hc.client5.http.AuthenticationStrategy;
 import org.apache.hc.client5.http.HttpRoute;
+import org.apache.hc.client5.http.RouteInfo;
 import org.apache.hc.client5.http.async.AsyncExecCallback;
 import org.apache.hc.client5.http.async.AsyncExecChain;
 import org.apache.hc.client5.http.async.AsyncExecRuntime;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.ChallengeType;
 import org.apache.hc.client5.http.auth.StandardAuthScheme;
+import org.apache.hc.client5.http.impl.auth.BasicAuthCache;
 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;
@@ -157,4 +161,111 @@ public void failed(final Exception cause) {
         final AuthExchange authExchange = context.getAuthExchange(target);
         Assertions.assertEquals("/blah/", authExchange.getPathPrefix());
     }
+
+    private static final class RecordingCallback implements AsyncExecCallback {
+
+        private final List<Integer> codes = new ArrayList<>();
+        private final AtomicInteger completed = new AtomicInteger(0);
+
+        @Override
+        public AsyncDataConsumer handleResponse(final HttpResponse response, final EntityDetails entityDetails) {
+            codes.add(response.getCode());
+            return Mockito.mock(AsyncDataConsumer.class);
+        }
+
+        @Override
+        public void handleInformationResponse(final HttpResponse response) {
+        }
+
+        @Override
+        public void completed() {
+            completed.incrementAndGet();
+        }
+
+        @Override
+        public void failed(final Exception cause) {
+            Assertions.fail(cause);
+        }
+
+    }
+
+    private void stubChain(final AtomicInteger proceedCount, final List<HttpRequest> requests, final int... codes)
+            throws Exception {
+        Mockito.doAnswer(invocation -> {
+            requests.add(invocation.getArgument(0, HttpRequest.class));
+            final AsyncExecCallback cb = invocation.getArgument(3, AsyncExecCallback.class);
+            final int code = codes[Math.min(proceedCount.getAndIncrement(), codes.length - 1)];
+            final HttpResponse resp = new BasicHttpResponse(code, "Huh?");
+            if (code == 407) {
+                resp.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
+            }
+            cb.handleResponse(resp, (EntityDetails) null);
+            cb.completed();
+            return null;
+        }).when(chain).proceed(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+    }
+
+    @Test
+    void testProxyChallengeThroughTunnelWithAuthCacheIsReturnedToCaller() throws Exception {
+        final AsyncProtocolExec cachingExec = new AsyncProtocolExec(targetAuthStrategy, proxyAuthStrategy, null, false);
+        final HttpHost httpsTarget = new HttpHost("https", "foo", 443);
+        final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+        final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true,
+                RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED);
+        final BasicAuthCache authCache = new BasicAuthCache();
+        final HttpClientContext context = HttpClientContext.create();
+        context.setAuthCache(authCache);
+        context.setCredentialsProvider(CredentialsProviderBuilder.create()
+                .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+                .build());
+        Mockito.when(execRuntime.isEndpointConnected()).thenReturn(true);
+        final AtomicInteger proceedCount = new AtomicInteger(0);
+        final List<HttpRequest> requests = new ArrayList<>();
+        stubChain(proceedCount, requests, 407);
+
+        final HttpRequest request = BasicRequestBuilder.get("https://foo/test").build();
+        final AsyncExecChain.Scope scope = new AsyncExecChain.Scope("test", route, request,
+                Mockito.mock(CancellableDependency.class), context, execRuntime, null, new AtomicInteger(1));
+        final RecordingCallback callback = new RecordingCallback();
+
+        cachingExec.execute(request, (AsyncEntityProducer) null, scope, chain, callback);
+
+        Assertions.assertEquals(Collections.singletonList(407), callback.codes);
+        Assertions.assertEquals(1, callback.completed.get());
+        Assertions.assertEquals(1, proceedCount.get());
+        Assertions.assertNull(requests.get(0).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION));
+        Mockito.verifyNoInteractions(proxyAuthStrategy);
+        Assertions.assertEquals(AuthExchange.State.UNCHALLENGED, context.getAuthExchange(proxy).getState());
+        Assertions.assertNull(authCache.get(proxy));
+    }
+
+    @Test
+    void testProxyChallengeOnPlainRouteStillAuthenticates() throws Exception {
+        final HttpHost proxy = new HttpHost("http", "proxy", 8080);
+        final HttpRoute route = new HttpRoute(target, null, proxy, false);
+        final HttpClientContext context = HttpClientContext.create();
+        context.setCredentialsProvider(CredentialsProviderBuilder.create()
+                .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray())
+                .build());
+        Mockito.when(proxyAuthStrategy.select(
+                        Mockito.eq(ChallengeType.PROXY),
+                        Mockito.any(),
+                        Mockito.<HttpClientContext>any()))
+                .thenReturn(Collections.singletonList(new BasicScheme()));
+        Mockito.when(execRuntime.isEndpointConnected()).thenReturn(true);
+        final AtomicInteger proceedCount = new AtomicInteger(0);
+        final List<HttpRequest> requests = new ArrayList<>();
+        stubChain(proceedCount, requests, 407, 200);
+
+        final HttpRequest request = BasicRequestBuilder.get("http://foo/test").build();
+        final AsyncExecChain.Scope scope = new AsyncExecChain.Scope("test", route, request,
+                Mockito.mock(CancellableDependency.class), context, execRuntime, null, new AtomicInteger(1));
+        final RecordingCallback callback = new RecordingCallback();
+
+        protocolExec.execute(request, (AsyncEntityProducer) null, scope, chain, callback);
+
+        Assertions.assertEquals(2, proceedCount.get());
+        Assertions.assertEquals(Collections.singletonList(200), callback.codes);
+        Assertions.assertNotNull(requests.get(1).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION));
+    }
 }