You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2021/09/25 14:21:34 UTC

[httpcomponents-client] 01/01: More consistent handling of request scheme and authority by protocol interceptors

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch 5.1.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 44d131a2c89059d05ac35c0dedf08dd8eca2524a
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Sep 24 23:18:02 2021 +0200

    More consistent handling of request scheme and authority by protocol interceptors
---
 .../client5/http/impl/async/AsyncProtocolExec.java | 31 +++++++++++--------
 .../http/impl/classic/InternalHttpClient.java      | 18 +++--------
 .../hc/client5/http/impl/classic/ProtocolExec.java | 36 +++++++++++++---------
 3 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
index fe0d73b..880d9e5 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
@@ -111,7 +111,7 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
         }
 
         final HttpRoute route = scope.route;
-        final HttpHost target = route.getTargetHost();
+        final HttpHost routeTarget = route.getTargetHost();
         final HttpHost proxy = route.getProxyHost();
         final HttpClientContext clientContext = scope.clientContext;
 
@@ -119,7 +119,7 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
         if (proxy != null && !route.isTunnelled()) {
             final BasicRequestBuilder requestBuilder = BasicRequestBuilder.copy(userRequest);
             if (requestBuilder.getAuthority() == null) {
-                requestBuilder.setAuthority(new URIAuthority(target));
+                requestBuilder.setAuthority(new URIAuthority(routeTarget));
             }
             requestBuilder.setAbsoluteRequestUri(true);
             request = requestBuilder.build();
@@ -127,12 +127,18 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
             request = userRequest;
         }
 
+        // Ensure the request has a scheme and an authority
+        if (request.getScheme() == null) {
+            request.setScheme(routeTarget.getSchemeName());
+        }
+        if (request.getAuthority() == null) {
+            request.setAuthority(new URIAuthority(routeTarget));
+        }
+
         final URIAuthority authority = request.getAuthority();
-        if (authority != null) {
-            final CredentialsProvider credsProvider = clientContext.getCredentialsProvider();
-            if (credsProvider instanceof CredentialsStore) {
-                AuthSupport.extractFromAuthority(request.getScheme(), authority, (CredentialsStore) credsProvider);
-            }
+        final CredentialsProvider credsProvider = clientContext.getCredentialsProvider();
+        if (credsProvider instanceof CredentialsStore) {
+            AuthSupport.extractFromAuthority(request.getScheme(), authority, (CredentialsStore) credsProvider);
         }
 
         final AtomicBoolean challenged = new AtomicBoolean(false);
@@ -151,8 +157,8 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
         final HttpClientContext clientContext = scope.clientContext;
         final AsyncExecRuntime execRuntime = scope.execRuntime;
 
-        final HttpHost target = route.getTargetHost();
         final HttpHost proxy = route.getProxyHost();
+        final HttpHost target = new HttpHost(request.getScheme(), request.getAuthority());
 
         final AuthExchange targetAuthExchange = clientContext.getAuthExchange(target);
         final AuthExchange proxyAuthExchange = proxy != null ? clientContext.getAuthExchange(proxy) : new AuthExchange();
@@ -251,11 +257,10 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
             @Override
             public void failed(final Exception cause) {
                 if (cause instanceof IOException || cause instanceof RuntimeException) {
-                    if (proxyAuthExchange.isConnectionBased()) {
-                        proxyAuthExchange.reset();
-                    }
-                    if (targetAuthExchange.isConnectionBased()) {
-                        targetAuthExchange.reset();
+                    for (final AuthExchange authExchange : clientContext.getAuthExchanges().values()) {
+                        if (authExchange.isConnectionBased()) {
+                            authExchange.reset();
+                        }
                     }
                 }
                 asyncExecCallback.failed(cause);
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java
index 7138c05..da5a188 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java
@@ -55,7 +55,6 @@ import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
 import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
@@ -63,7 +62,6 @@ import org.apache.hc.core5.http.protocol.BasicHttpContext;
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.io.ModalCloseable;
-import org.apache.hc.core5.net.URIAuthority;
 import org.apache.hc.core5.util.Args;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -118,11 +116,7 @@ class InternalHttpClient extends CloseableHttpClient implements Configurable {
         this.closeables = closeables != null ?  new ConcurrentLinkedQueue<>(closeables) : null;
     }
 
-    private HttpRoute determineRoute(
-            final HttpHost host,
-            final HttpRequest request,
-            final HttpContext context) throws HttpException {
-        final HttpHost target = host != null ? host : RoutingSupport.determineHost(request);
+    private HttpRoute determineRoute(final HttpHost target, final HttpContext context) throws HttpException {
         return this.routePlanner.determineRoute(target, context);
     }
 
@@ -151,12 +145,6 @@ class InternalHttpClient extends CloseableHttpClient implements Configurable {
             final HttpContext context) throws IOException {
         Args.notNull(request, "HTTP request");
         try {
-            if (request.getScheme() == null && target != null) {
-                request.setScheme(target.getSchemeName());
-            }
-            if (request.getAuthority() == null && target != null) {
-                request.setAuthority(new URIAuthority(target));
-            }
             final HttpClientContext localcontext = HttpClientContext.adapt(
                     context != null ? context : new BasicHttpContext());
             RequestConfig config = null;
@@ -167,7 +155,9 @@ class InternalHttpClient extends CloseableHttpClient implements Configurable {
                 localcontext.setRequestConfig(config);
             }
             setupContext(localcontext);
-            final HttpRoute route = determineRoute(target, request, localcontext);
+            final HttpRoute route = determineRoute(
+                    target != null ? target : RoutingSupport.determineHost(request),
+                    localcontext);
             final String exchangeId = ExecSupport.getNextExchangeId();
             if (LOG.isDebugEnabled()) {
                 LOG.debug("{} preparing request execution", exchangeId);
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
index 2edfe4c..14fcbfb 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
@@ -114,17 +114,15 @@ public final class ProtocolExec implements ExecChainHandler {
         final HttpClientContext context = scope.clientContext;
         final ExecRuntime execRuntime = scope.execRuntime;
 
-        final HttpHost target = route.getTargetHost();
+        final HttpHost routeTarget = route.getTargetHost();
         final HttpHost proxy = route.getProxyHost();
-        final AuthExchange targetAuthExchange = context.getAuthExchange(target);
-        final AuthExchange proxyAuthExchange = proxy != null ? context.getAuthExchange(proxy) : new AuthExchange();
 
         try {
             final ClassicHttpRequest request;
             if (proxy != null && !route.isTunnelled()) {
                 final ClassicRequestBuilder requestBuilder = ClassicRequestBuilder.copy(userRequest);
                 if (requestBuilder.getAuthority() == null) {
-                    requestBuilder.setAuthority(new URIAuthority(target));
+                    requestBuilder.setAuthority(new URIAuthority(routeTarget));
                 }
                 requestBuilder.setAbsoluteRequestUri(true);
                 request = requestBuilder.build();
@@ -132,14 +130,25 @@ public final class ProtocolExec implements ExecChainHandler {
                 request = userRequest;
             }
 
+            // Ensure the request has a scheme and an authority
+            if (request.getScheme() == null) {
+                request.setScheme(routeTarget.getSchemeName());
+            }
+            if (request.getAuthority() == null) {
+                request.setAuthority(new URIAuthority(routeTarget));
+            }
+
             final URIAuthority authority = request.getAuthority();
-            if (authority != null) {
-                final CredentialsProvider credsProvider = context.getCredentialsProvider();
-                if (credsProvider instanceof CredentialsStore) {
-                    AuthSupport.extractFromAuthority(request.getScheme(), authority, (CredentialsStore) credsProvider);
-                }
+            final CredentialsProvider credsProvider = context.getCredentialsProvider();
+            if (credsProvider instanceof CredentialsStore) {
+                AuthSupport.extractFromAuthority(request.getScheme(), authority, (CredentialsStore) credsProvider);
             }
 
+            final HttpHost target = new HttpHost(request.getScheme(), request.getAuthority());
+
+            final AuthExchange targetAuthExchange = context.getAuthExchange(target);
+            final AuthExchange proxyAuthExchange = proxy != null ? context.getAuthExchange(proxy) : new AuthExchange();
+
             RequestEntityProxy.enhance(request);
 
             for (;;) {
@@ -217,11 +226,10 @@ public final class ProtocolExec implements ExecChainHandler {
             throw ex;
         } catch (final RuntimeException | IOException ex) {
             execRuntime.discardEndpoint();
-            if (proxyAuthExchange.isConnectionBased()) {
-                proxyAuthExchange.reset();
-            }
-            if (targetAuthExchange.isConnectionBased()) {
-                targetAuthExchange.reset();
+            for (final AuthExchange authExchange : context.getAuthExchanges().values()) {
+                if (authExchange.isConnectionBased()) {
+                    authExchange.reset();
+                }
             }
             throw ex;
         }