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 17:00:04 UTC

[httpcomponents-client] branch master updated (fe1a49b -> 542056f)

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

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


    from fe1a49b  More consistent handling of request scheme and authority by protocol interceptors
     new f310cc6  Refactored AuthCache keeping logic into a separate utility class
     new 542056f  Logger cleanup (no functional changes)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../client5/http/impl/async/AsyncConnectExec.java  |  16 ++-
 .../client5/http/impl/async/AsyncProtocolExec.java |  23 ++++-
 .../hc/client5/http/impl/auth/AuthCacheKeeper.java | 102 +++++++++++++++++++
 .../client5/http/impl/auth/HttpAuthenticator.java  | 111 ++++++---------------
 .../hc/client5/http/impl/classic/ConnectExec.java  |  27 +++--
 .../hc/client5/http/impl/classic/ProtocolExec.java |  21 +++-
 .../http/impl/auth/TestHttpAuthenticator.java      |  13 ---
 7 files changed, 206 insertions(+), 107 deletions(-)
 create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthCacheKeeper.java

[httpcomponents-client] 01/02: Refactored AuthCache keeping logic into a separate utility class

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit f310cc64579cdd4fd5f1c7b26c00be1ad65586cc
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Sep 25 18:52:51 2021 +0200

    Refactored AuthCache keeping logic into a separate utility class
---
 .../client5/http/impl/async/AsyncConnectExec.java  |  16 +++-
 .../client5/http/impl/async/AsyncProtocolExec.java |  23 ++++-
 .../hc/client5/http/impl/auth/AuthCacheKeeper.java | 102 +++++++++++++++++++++
 .../client5/http/impl/auth/HttpAuthenticator.java  |  49 +---------
 .../hc/client5/http/impl/classic/ConnectExec.java  |  27 ++++--
 .../hc/client5/http/impl/classic/ProtocolExec.java |  21 ++++-
 .../http/impl/auth/TestHttpAuthenticator.java      |  13 ---
 7 files changed, 176 insertions(+), 75 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 b0288c7..2c2b9bd 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
@@ -41,6 +41,7 @@ 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;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -84,6 +85,7 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
     private final HttpProcessor proxyHttpProcessor;
     private final AuthenticationStrategy proxyAuthStrategy;
     private final HttpAuthenticator authenticator;
+    private final AuthCacheKeeper authCacheKeeper;
     private final HttpRouteDirector routeDirector;
 
     public AsyncConnectExec(
@@ -93,8 +95,9 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
         Args.notNull(proxyAuthStrategy, "Proxy authentication strategy");
         this.proxyHttpProcessor = proxyHttpProcessor;
         this.proxyAuthStrategy  = proxyAuthStrategy;
-        this.authenticator      = new HttpAuthenticator(LOG);
-        this.routeDirector      = new BasicRouteDirector();
+        this.authenticator = new HttpAuthenticator();
+        this.authCacheKeeper = new AuthCacheKeeper();
+        this.routeDirector = new BasicRouteDirector();
     }
 
     static class State {
@@ -432,8 +435,15 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
         if (config.isAuthenticationEnabled()) {
             final boolean proxyAuthRequested = authenticator.isChallenged(proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
             if (proxyAuthRequested) {
-                return authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
+                authCacheKeeper.updateOnChallenge(proxy, proxyAuthExchange, context);
+            } else {
+                authCacheKeeper.updateOnNoChallenge(proxy, proxyAuthExchange, context);
+            }
+            if (proxyAuthRequested) {
+                final boolean updated = authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
                         proxyAuthStrategy, proxyAuthExchange, context);
+                authCacheKeeper.updateOnResponse(proxy, proxyAuthExchange, context);
+                return updated;
             }
         }
         return false;
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 880d9e5..45312f1 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
@@ -42,6 +42,7 @@ import org.apache.hc.client5.http.auth.CredentialsProvider;
 import org.apache.hc.client5.http.auth.CredentialsStore;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.AuthSupport;
+import org.apache.hc.client5.http.impl.auth.AuthCacheKeeper;
 import org.apache.hc.client5.http.impl.auth.HttpAuthenticator;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.core5.annotation.Contract;
@@ -87,6 +88,7 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
     private final AuthenticationStrategy targetAuthStrategy;
     private final AuthenticationStrategy proxyAuthStrategy;
     private final HttpAuthenticator authenticator;
+    private final AuthCacheKeeper authCacheKeeper;
 
     AsyncProtocolExec(
             final HttpProcessor httpProcessor,
@@ -95,7 +97,8 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
         this.httpProcessor = Args.notNull(httpProcessor, "HTTP protocol processor");
         this.targetAuthStrategy = Args.notNull(targetAuthStrategy, "Target authentication strategy");
         this.proxyAuthStrategy = Args.notNull(proxyAuthStrategy, "Proxy authentication strategy");
-        this.authenticator = new HttpAuthenticator(LOG);
+        this.authenticator = new HttpAuthenticator();
+        this.authCacheKeeper = new AuthCacheKeeper();
     }
 
     @Override
@@ -281,6 +284,11 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
             final HttpHost target = AuthSupport.resolveAuthTarget(request, route);
             final boolean targetAuthRequested = authenticator.isChallenged(
                     target, ChallengeType.TARGET, response, targetAuthExchange, context);
+            if (targetAuthRequested) {
+                authCacheKeeper.updateOnChallenge(target, targetAuthExchange, context);
+            } else {
+                authCacheKeeper.updateOnNoChallenge(target, targetAuthExchange, context);
+            }
 
             HttpHost proxy = route.getProxyHost();
             // if proxy is not set use target host instead
@@ -289,14 +297,23 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
             }
             final boolean proxyAuthRequested = authenticator.isChallenged(
                     proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
+            if (proxyAuthRequested) {
+                authCacheKeeper.updateOnChallenge(proxy, proxyAuthExchange, context);
+            } else {
+                authCacheKeeper.updateOnNoChallenge(proxy, proxyAuthExchange, context);
+            }
 
             if (targetAuthRequested) {
-                return authenticator.updateAuthState(target, ChallengeType.TARGET, response,
+                final boolean updated = authenticator.updateAuthState(target, ChallengeType.TARGET, response,
                         targetAuthStrategy, targetAuthExchange, context);
+                authCacheKeeper.updateOnResponse(target, targetAuthExchange, context);
+                return updated;
             }
             if (proxyAuthRequested) {
-                return authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
+                final boolean updated = authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
                         proxyAuthStrategy, proxyAuthExchange, context);
+                authCacheKeeper.updateOnResponse(proxy, proxyAuthExchange, context);
+                return updated;
             }
         }
         return false;
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthCacheKeeper.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthCacheKeeper.java
new file mode 100644
index 0000000..a748f14
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthCacheKeeper.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
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.http.impl.auth;
+
+import org.apache.hc.client5.http.auth.AuthCache;
+import org.apache.hc.client5.http.auth.AuthExchange;
+import org.apache.hc.client5.http.auth.AuthScheme;
+import org.apache.hc.client5.http.auth.AuthStateCacheable;
+import org.apache.hc.client5.http.protocol.HttpClientContext;
+import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.annotation.ThreadingBehavior;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.protocol.HttpContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Utility class that implements commons aspects of the client side authentication cache keeping.
+ *
+ * @since 5.2
+ */
+@Internal
+@Contract(threading = ThreadingBehavior.STATELESS)
+public final class AuthCacheKeeper {
+
+    private static final Logger LOG = LoggerFactory.getLogger(AuthCacheKeeper.class);
+
+    public AuthCacheKeeper() {
+    }
+
+    public void updateOnChallenge(final HttpHost host, final AuthExchange authExchange, final HttpContext context) {
+        if (authExchange.getState() == AuthExchange.State.SUCCESS) {
+            clearCache(host, HttpClientContext.adapt(context));
+        }
+    }
+
+    public void updateOnNoChallenge(final HttpHost host, final AuthExchange authExchange, final HttpContext context) {
+        if (authExchange.getState() == AuthExchange.State.SUCCESS) {
+            updateCache(host, authExchange.getAuthScheme(), HttpClientContext.adapt(context));
+        }
+    }
+
+    public void updateOnResponse(final HttpHost host, final AuthExchange authExchange, final HttpContext context) {
+        if (authExchange.getState() == AuthExchange.State.FAILURE) {
+            clearCache(host, HttpClientContext.adapt(context));
+        }
+    }
+
+    private void updateCache(final HttpHost host, final AuthScheme authScheme, final HttpClientContext clientContext) {
+        final boolean cacheable = authScheme.getClass().getAnnotation(AuthStateCacheable.class) != null;
+        if (cacheable) {
+            AuthCache authCache = clientContext.getAuthCache();
+            if (authCache == null) {
+                authCache = new BasicAuthCache();
+                clientContext.setAuthCache(authCache);
+            }
+            if (LOG.isDebugEnabled()) {
+                final String exchangeId = clientContext.getExchangeId();
+                LOG.debug("{} Caching '{}' auth scheme for {}", exchangeId, authScheme.getName(), host);
+            }
+            authCache.put(host, authScheme);
+        }
+    }
+
+    private void clearCache(final HttpHost host, final HttpClientContext clientContext) {
+        final AuthCache authCache = clientContext.getAuthCache();
+        if (authCache != null) {
+            if (LOG.isDebugEnabled()) {
+                final String exchangeId = clientContext.getExchangeId();
+                LOG.debug("{} Clearing cached auth scheme for {}", exchangeId, host);
+            }
+            authCache.remove(host);
+        }
+    }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java
index 7987402..67d3c07 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java
@@ -35,18 +35,15 @@ import java.util.Map;
 import java.util.Queue;
 
 import org.apache.hc.client5.http.AuthenticationStrategy;
-import org.apache.hc.client5.http.auth.AuthCache;
 import org.apache.hc.client5.http.auth.AuthChallenge;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.AuthScheme;
-import org.apache.hc.client5.http.auth.AuthStateCacheable;
 import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.auth.ChallengeType;
 import org.apache.hc.client5.http.auth.CredentialsProvider;
 import org.apache.hc.client5.http.auth.MalformedChallengeException;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.core5.annotation.Contract;
-import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.FormattedHeader;
 import org.apache.hc.core5.http.Header;
@@ -77,15 +74,9 @@ public final class HttpAuthenticator {
     private final Logger log;
     private final AuthChallengeParser parser;
 
-    @Internal
-    public HttpAuthenticator(final Logger log) {
-        super();
-        this.log = log != null ? log : DEFAULT_LOGGER;
-        this.parser = new AuthChallengeParser();
-    }
-
     public HttpAuthenticator() {
-        this(null);
+        this.log = DEFAULT_LOGGER;
+        this.parser = new AuthChallengeParser();
     }
 
     /**
@@ -124,9 +115,6 @@ public final class HttpAuthenticator {
             if (log.isDebugEnabled()) {
                 log.debug("{} Authentication required", exchangeId);
             }
-            if (authExchange.getState() == AuthExchange.State.SUCCESS) {
-                clearCache(host, clientContext);
-            }
             return true;
         }
         switch (authExchange.getState()) {
@@ -136,7 +124,6 @@ public final class HttpAuthenticator {
                 log.debug("{} Authentication succeeded", exchangeId);
             }
             authExchange.setState(AuthExchange.State.SUCCESS);
-            updateCache(host, authExchange.getAuthScheme(), clientContext);
             break;
         case SUCCESS:
             break;
@@ -213,7 +200,6 @@ public final class HttpAuthenticator {
             if (log.isDebugEnabled()) {
                 log.debug("{} Response contains no valid authentication challenges", exchangeId);
             }
-            clearCache(host, clientContext);
             authExchange.reset();
             return false;
         }
@@ -242,15 +228,14 @@ public final class HttpAuthenticator {
                             if (log.isWarnEnabled()) {
                                 log.warn("{} {}", exchangeId, ex.getMessage());
                             }
-                            clearCache(host, clientContext);
                             authExchange.reset();
+                            authExchange.setState(AuthExchange.State.FAILURE);
                             return false;
                         }
                         if (authScheme.isChallengeComplete()) {
                             if (log.isDebugEnabled()) {
                                 log.debug("{} Authentication failed", exchangeId);
                             }
-                            clearCache(host, clientContext);
                             authExchange.reset();
                             authExchange.setState(AuthExchange.State.FAILURE);
                             return false;
@@ -376,32 +361,4 @@ public final class HttpAuthenticator {
         }
     }
 
-    private void updateCache(final HttpHost host, final AuthScheme authScheme, final HttpClientContext clientContext) {
-        final boolean cacheable = authScheme.getClass().getAnnotation(AuthStateCacheable.class) != null;
-        if (cacheable) {
-            AuthCache authCache = clientContext.getAuthCache();
-            if (authCache == null) {
-                authCache = new BasicAuthCache();
-                clientContext.setAuthCache(authCache);
-            }
-            if (log.isDebugEnabled()) {
-                final String exchangeId = clientContext.getExchangeId();
-                log.debug("{} Caching '{}' auth scheme for {}", exchangeId, authScheme.getName(), host);
-            }
-            authCache.put(host, authScheme);
-        }
-    }
-
-    private void clearCache(final HttpHost host, final HttpClientContext clientContext) {
-
-        final AuthCache authCache = clientContext.getAuthCache();
-        if (authCache != null) {
-            if (log.isDebugEnabled()) {
-                final String exchangeId = clientContext.getExchangeId();
-                log.debug("{} Clearing cached auth scheme for {}", exchangeId, host);
-            }
-            authCache.remove(host);
-        }
-    }
-
 }
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 357915f..96c0a4e 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
@@ -39,6 +39,7 @@ 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;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -82,6 +83,7 @@ public final class ConnectExec implements ExecChainHandler {
     private final HttpProcessor proxyHttpProcessor;
     private final AuthenticationStrategy proxyAuthStrategy;
     private final HttpAuthenticator authenticator;
+    private final AuthCacheKeeper authCacheKeeper;
     private final HttpRouteDirector routeDirector;
 
     public ConnectExec(
@@ -91,11 +93,12 @@ public final class ConnectExec implements ExecChainHandler {
         Args.notNull(reuseStrategy, "Connection reuse strategy");
         Args.notNull(proxyHttpProcessor, "Proxy HTTP processor");
         Args.notNull(proxyAuthStrategy, "Proxy authentication strategy");
-        this.reuseStrategy      = reuseStrategy;
+        this.reuseStrategy = reuseStrategy;
         this.proxyHttpProcessor = proxyHttpProcessor;
-        this.proxyAuthStrategy  = proxyAuthStrategy;
-        this.authenticator      = new HttpAuthenticator(LOG);
-        this.routeDirector      = new BasicRouteDirector();
+        this.proxyAuthStrategy = proxyAuthStrategy;
+        this.authenticator = new HttpAuthenticator();
+        this.authCacheKeeper = new AuthCacheKeeper();
+        this.routeDirector = new BasicRouteDirector();
     }
 
     @Override
@@ -239,10 +242,18 @@ public final class ConnectExec implements ExecChainHandler {
             }
 
             if (config.isAuthenticationEnabled()) {
-                if (this.authenticator.isChallenged(proxy, ChallengeType.PROXY, response,
-                        proxyAuthExchange, context)) {
-                    if (this.authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
-                            this.proxyAuthStrategy, proxyAuthExchange, context)) {
+                final boolean proxyAuthRequested = authenticator.isChallenged(proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
+                if (proxyAuthRequested) {
+                    authCacheKeeper.updateOnChallenge(proxy, proxyAuthExchange, context);
+                } else {
+                    authCacheKeeper.updateOnNoChallenge(proxy, proxyAuthExchange, context);
+                }
+
+                if (proxyAuthRequested) {
+                    final boolean updated = authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
+                            proxyAuthStrategy, proxyAuthExchange, context);
+                    authCacheKeeper.updateOnResponse(proxy, proxyAuthExchange, context);
+                    if (updated) {
                         // Retry request
                         response = null;
                     }
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 14fcbfb..9355190 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
@@ -41,6 +41,7 @@ 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.AuthSupport;
+import org.apache.hc.client5.http.impl.auth.AuthCacheKeeper;
 import org.apache.hc.client5.http.impl.auth.HttpAuthenticator;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.core5.annotation.Contract;
@@ -86,6 +87,7 @@ public final class ProtocolExec implements ExecChainHandler {
     private final AuthenticationStrategy targetAuthStrategy;
     private final AuthenticationStrategy proxyAuthStrategy;
     private final HttpAuthenticator authenticator;
+    private final AuthCacheKeeper authCacheKeeper;
 
     public ProtocolExec(
             final HttpProcessor httpProcessor,
@@ -95,6 +97,7 @@ public final class ProtocolExec implements ExecChainHandler {
         this.targetAuthStrategy = Args.notNull(targetAuthStrategy, "Target authentication strategy");
         this.proxyAuthStrategy = Args.notNull(proxyAuthStrategy, "Proxy authentication strategy");
         this.authenticator = new HttpAuthenticator();
+        this.authCacheKeeper = new AuthCacheKeeper();
     }
 
     @Override
@@ -247,6 +250,11 @@ public final class ProtocolExec implements ExecChainHandler {
             final HttpHost target = AuthSupport.resolveAuthTarget(request, route);
             final boolean targetAuthRequested = authenticator.isChallenged(
                     target, ChallengeType.TARGET, response, targetAuthExchange, context);
+            if (targetAuthRequested) {
+                authCacheKeeper.updateOnChallenge(target, targetAuthExchange, context);
+            } else {
+                authCacheKeeper.updateOnNoChallenge(target, targetAuthExchange, context);
+            }
 
             HttpHost proxy = route.getProxyHost();
             // if proxy is not set use target host instead
@@ -255,14 +263,23 @@ public final class ProtocolExec implements ExecChainHandler {
             }
             final boolean proxyAuthRequested = authenticator.isChallenged(
                     proxy, ChallengeType.PROXY, response, proxyAuthExchange, context);
+            if (proxyAuthRequested) {
+                authCacheKeeper.updateOnChallenge(proxy, proxyAuthExchange, context);
+            } else {
+                authCacheKeeper.updateOnNoChallenge(proxy, proxyAuthExchange, context);
+            }
 
             if (targetAuthRequested) {
-                return authenticator.updateAuthState(target, ChallengeType.TARGET, response,
+                final boolean updated = authenticator.updateAuthState(target, ChallengeType.TARGET, response,
                         targetAuthStrategy, targetAuthExchange, context);
+                authCacheKeeper.updateOnResponse(target, targetAuthExchange, context);
+                return updated;
             }
             if (proxyAuthRequested) {
-                return authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
+                final boolean updated = authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
                         proxyAuthStrategy, proxyAuthExchange, context);
+                authCacheKeeper.updateOnResponse(proxy, proxyAuthExchange, context);
+                return updated;
             }
         }
         return false;
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestHttpAuthenticator.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestHttpAuthenticator.java
index fd24894..10e6946 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestHttpAuthenticator.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestHttpAuthenticator.java
@@ -29,7 +29,6 @@ package org.apache.hc.client5.http.impl.auth;
 import java.util.LinkedList;
 import java.util.Queue;
 
-import org.apache.hc.client5.http.auth.AuthCache;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.AuthScheme;
 import org.apache.hc.client5.http.auth.AuthSchemeFactory;
@@ -80,7 +79,6 @@ public class TestHttpAuthenticator {
     private HttpHost defaultHost;
     private CredentialsProvider credentialsProvider;
     private Lookup<AuthSchemeFactory> authSchemeRegistry;
-    private AuthCache authCache;
     private HttpAuthenticator httpAuthenticator;
 
     @Before
@@ -98,8 +96,6 @@ public class TestHttpAuthenticator {
             .register(StandardAuthScheme.DIGEST, DigestSchemeFactory.INSTANCE)
             .register(StandardAuthScheme.NTLM, NTLMSchemeFactory.INSTANCE).build();
         this.context.setAttribute(HttpClientContext.AUTHSCHEME_REGISTRY, this.authSchemeRegistry);
-        this.authCache = Mockito.mock(AuthCache.class);
-        this.context.setAttribute(HttpClientContext.AUTH_CACHE, this.authCache);
         this.httpAuthenticator = new HttpAuthenticator();
     }
 
@@ -109,7 +105,6 @@ public class TestHttpAuthenticator {
         response.setHeader(HttpHeaders.WWW_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
         Assert.assertTrue(this.httpAuthenticator.isChallenged(
                 this.defaultHost, ChallengeType.TARGET, response, this.authExchange, this.context));
-        Mockito.verifyNoInteractions(this.authCache);
     }
 
     @Test
@@ -122,8 +117,6 @@ public class TestHttpAuthenticator {
 
         Assert.assertTrue(this.httpAuthenticator.isChallenged(
                 this.defaultHost, ChallengeType.TARGET, response, this.authExchange, this.context));
-
-        Mockito.verify(this.authCache).remove(this.defaultHost);
     }
 
     @Test
@@ -144,8 +137,6 @@ public class TestHttpAuthenticator {
         Assert.assertFalse(this.httpAuthenticator.isChallenged(
                 this.defaultHost, ChallengeType.TARGET, response, this.authExchange, this.context));
         Assert.assertEquals(AuthExchange.State.SUCCESS, this.authExchange.getState());
-
-        Mockito.verify(this.authCache).put(this.defaultHost, this.authScheme);
     }
 
     @Test
@@ -157,8 +148,6 @@ public class TestHttpAuthenticator {
         Assert.assertFalse(this.httpAuthenticator.isChallenged(
                 this.defaultHost, ChallengeType.TARGET, response, this.authExchange, this.context));
         Assert.assertEquals(AuthExchange.State.SUCCESS, this.authExchange.getState());
-
-        Mockito.verify(this.authCache).put(this.defaultHost, this.authScheme);
     }
 
     @Test
@@ -269,8 +258,6 @@ public class TestHttpAuthenticator {
                 host, ChallengeType.TARGET, response, authStrategy, this.authExchange, this.context));
 
         Assert.assertEquals(AuthExchange.State.FAILURE, this.authExchange.getState());
-
-        Mockito.verify(this.authCache).remove(host);
     }
 
     @Test

[httpcomponents-client] 02/02: Logger cleanup (no functional changes)

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 542056f9c460618ee7aac180fd84874e4499c9b7
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Sep 25 18:55:46 2021 +0200

    Logger cleanup (no functional changes)
---
 .../client5/http/impl/auth/HttpAuthenticator.java  | 64 +++++++++++-----------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java
index 67d3c07..e2e642b 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/HttpAuthenticator.java
@@ -69,13 +69,11 @@ import org.slf4j.LoggerFactory;
 @Contract(threading = ThreadingBehavior.STATELESS)
 public final class HttpAuthenticator {
 
-    private static final Logger DEFAULT_LOGGER = LoggerFactory.getLogger(HttpAuthenticator.class);
+    private static final Logger LOG = LoggerFactory.getLogger(HttpAuthenticator.class);
 
-    private final Logger log;
     private final AuthChallengeParser parser;
 
     public HttpAuthenticator() {
-        this.log = DEFAULT_LOGGER;
         this.parser = new AuthChallengeParser();
     }
 
@@ -112,16 +110,16 @@ public final class HttpAuthenticator {
         final String exchangeId = clientContext.getExchangeId();
 
         if (response.getCode() == challengeCode) {
-            if (log.isDebugEnabled()) {
-                log.debug("{} Authentication required", exchangeId);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} Authentication required", exchangeId);
             }
             return true;
         }
         switch (authExchange.getState()) {
         case CHALLENGED:
         case HANDSHAKE:
-            if (log.isDebugEnabled()) {
-                log.debug("{} Authentication succeeded", exchangeId);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} Authentication succeeded", exchangeId);
             }
             authExchange.setState(AuthExchange.State.SUCCESS);
             break;
@@ -157,8 +155,8 @@ public final class HttpAuthenticator {
         final HttpClientContext clientContext = HttpClientContext.adapt(context);
         final String exchangeId = clientContext.getExchangeId();
 
-        if (log.isDebugEnabled()) {
-            log.debug("{} {} requested authentication", exchangeId, host.toHostString());
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("{} {} requested authentication", exchangeId, host.toHostString());
         }
 
         final Header[] headers = response.getHeaders(
@@ -184,8 +182,8 @@ public final class HttpAuthenticator {
             try {
                 authChallenges = parser.parse(challengeType, buffer, cursor);
             } catch (final ParseException ex) {
-                if (log.isWarnEnabled()) {
-                    log.warn("{} Malformed challenge: {}", exchangeId, header.getValue());
+                if (LOG.isWarnEnabled()) {
+                    LOG.warn("{} Malformed challenge: {}", exchangeId, header.getValue());
                 }
                 continue;
             }
@@ -197,8 +195,8 @@ public final class HttpAuthenticator {
             }
         }
         if (challengeMap.isEmpty()) {
-            if (log.isDebugEnabled()) {
-                log.debug("{} Response contains no valid authentication challenges", exchangeId);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} Response contains no valid authentication challenges", exchangeId);
             }
             authExchange.reset();
             return false;
@@ -219,22 +217,22 @@ public final class HttpAuthenticator {
                     final String schemeName = authScheme.getName();
                     final AuthChallenge challenge = challengeMap.get(schemeName.toLowerCase(Locale.ROOT));
                     if (challenge != null) {
-                        if (log.isDebugEnabled()) {
-                            log.debug("{} Authorization challenge processed", exchangeId);
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("{} Authorization challenge processed", exchangeId);
                         }
                         try {
                             authScheme.processChallenge(challenge, context);
                         } catch (final MalformedChallengeException ex) {
-                            if (log.isWarnEnabled()) {
-                                log.warn("{} {}", exchangeId, ex.getMessage());
+                            if (LOG.isWarnEnabled()) {
+                                LOG.warn("{} {}", exchangeId, ex.getMessage());
                             }
                             authExchange.reset();
                             authExchange.setState(AuthExchange.State.FAILURE);
                             return false;
                         }
                         if (authScheme.isChallengeComplete()) {
-                            if (log.isDebugEnabled()) {
-                                log.debug("{} Authentication failed", exchangeId);
+                            if (LOG.isDebugEnabled()) {
+                                LOG.debug("{} Authentication failed", exchangeId);
                             }
                             authExchange.reset();
                             authExchange.setState(AuthExchange.State.FAILURE);
@@ -251,15 +249,15 @@ public final class HttpAuthenticator {
         final List<AuthScheme> preferredSchemes = authStrategy.select(challengeType, challengeMap, context);
         final CredentialsProvider credsProvider = clientContext.getCredentialsProvider();
         if (credsProvider == null) {
-            if (log.isDebugEnabled()) {
-                log.debug("{} Credentials provider not set in the context", exchangeId);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} Credentials provider not set in the context", exchangeId);
             }
             return false;
         }
 
         final Queue<AuthScheme> authOptions = new LinkedList<>();
-        if (log.isDebugEnabled()) {
-            log.debug("{} Selecting authentication options", exchangeId);
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("{} Selecting authentication options", exchangeId);
         }
         for (final AuthScheme authScheme: preferredSchemes) {
             try {
@@ -270,14 +268,14 @@ public final class HttpAuthenticator {
                     authOptions.add(authScheme);
                 }
             } catch (final AuthenticationException | MalformedChallengeException ex) {
-                if (log.isWarnEnabled()) {
-                    log.warn(ex.getMessage());
+                if (LOG.isWarnEnabled()) {
+                    LOG.warn(ex.getMessage());
                 }
             }
         }
         if (!authOptions.isEmpty()) {
-            if (log.isDebugEnabled()) {
-                log.debug("{} Selected authentication options: {}", exchangeId, authOptions);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} Selected authentication options: {}", exchangeId, authOptions);
             }
             authExchange.reset();
             authExchange.setState(AuthExchange.State.CHALLENGED);
@@ -324,8 +322,8 @@ public final class HttpAuthenticator {
                 while (!authOptions.isEmpty()) {
                     authScheme = authOptions.remove();
                     authExchange.select(authScheme);
-                    if (log.isDebugEnabled()) {
-                        log.debug("{} Generating response to an authentication challenge using {} scheme",
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("{} Generating response to an authentication challenge using {} scheme",
                                 exchangeId, authScheme.getName());
                     }
                     try {
@@ -336,8 +334,8 @@ public final class HttpAuthenticator {
                         request.addHeader(header);
                         break;
                     } catch (final AuthenticationException ex) {
-                        if (log.isWarnEnabled()) {
-                            log.warn("{} {} authentication error: {}", exchangeId, authScheme, ex.getMessage());
+                        if (LOG.isWarnEnabled()) {
+                            LOG.warn("{} {} authentication error: {}", exchangeId, authScheme, ex.getMessage());
                         }
                     }
                 }
@@ -354,8 +352,8 @@ public final class HttpAuthenticator {
                         authResponse);
                 request.addHeader(header);
             } catch (final AuthenticationException ex) {
-                if (log.isErrorEnabled()) {
-                    log.error("{} {} authentication error: {}", exchangeId, authScheme, ex.getMessage());
+                if (LOG.isErrorEnabled()) {
+                    LOG.error("{} {} authentication error: {}", exchangeId, authScheme, ex.getMessage());
                 }
             }
         }