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 2018/01/03 18:47:28 UTC
[4/4] httpcomponents-client git commit: HTTPCLIENT-1893: Moved
HttpContext state logic out of DefaultRedirectStrategy to the redirect
execution interceptors; redesigned RedirectLocations class;
refactored classic redirect execution interceptor unit tests
HTTPCLIENT-1893: Moved HttpContext state logic out of DefaultRedirectStrategy to the redirect execution interceptors; redesigned RedirectLocations class; refactored classic redirect execution interceptor unit tests
Project: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/repo
Commit: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/commit/47a0eb8b
Tree: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/tree/47a0eb8b
Diff: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/diff/47a0eb8b
Branch: refs/heads/master
Commit: 47a0eb8b65c2a98b50567bf6e0b4229a08035098
Parents: 71060c7
Author: Oleg Kalnichevski <ol...@apache.org>
Authored: Wed Jan 3 19:36:04 2018 +0100
Committer: Oleg Kalnichevski <ol...@apache.org>
Committed: Wed Jan 3 19:37:19 2018 +0100
----------------------------------------------------------------------
.../sync/TestClientRequestExecution.java | 6 +-
.../hc/client5/testing/sync/TestRedirects.java | 9 +-
.../http/impl/DefaultRedirectStrategy.java | 46 +---
.../http/impl/async/AsyncRedirectExec.java | 63 ++++--
.../client5/http/impl/classic/RedirectExec.java | 64 ++++--
.../http/protocol/HttpClientContext.java | 9 +-
.../http/protocol/RedirectLocations.java | 121 +---------
.../http/impl/TestDefaultRedirectStrategy.java | 49 ----
.../http/impl/classic/TestRedirectExec.java | 226 +++++++++++--------
.../http/protocol/TestRedirectLocation.java | 12 +-
10 files changed, 227 insertions(+), 378 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
----------------------------------------------------------------------
diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
index f89d4a1..d0dd55d 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java
@@ -29,12 +29,12 @@ package org.apache.hc.client5.testing.sync;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.net.URI;
-import java.util.List;
import org.apache.hc.client5.http.HttpRequestRetryHandler;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.protocol.HttpClientContext;
+import org.apache.hc.client5.http.protocol.RedirectLocations;
import org.apache.hc.client5.http.utils.URIUtils;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
@@ -252,8 +252,8 @@ public class TestClientRequestExecution extends LocalServerTestBase {
final HttpRequest request = context.getRequest();
Assert.assertEquals("/stuff", request.getRequestUri());
- final List<URI> redirectLocations = context.getRedirectLocations();
- final URI location = URIUtils.resolve(uri, target, redirectLocations);
+ final RedirectLocations redirectLocations = context.getRedirectLocations();
+ final URI location = URIUtils.resolve(uri, target, redirectLocations.getAll());
Assert.assertEquals(uri, location);
}
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
----------------------------------------------------------------------
diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
index ab8ec57..aedefd6 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
@@ -30,7 +30,6 @@ import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
-import java.util.List;
import org.apache.hc.client5.http.CircularRedirectException;
import org.apache.hc.client5.http.ClientProtocolException;
@@ -42,6 +41,7 @@ import org.apache.hc.client5.http.cookie.BasicCookieStore;
import org.apache.hc.client5.http.cookie.CookieStore;
import org.apache.hc.client5.http.impl.cookie.BasicClientCookie;
import org.apache.hc.client5.http.protocol.HttpClientContext;
+import org.apache.hc.client5.http.protocol.RedirectLocations;
import org.apache.hc.client5.http.utils.URIUtils;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
@@ -282,8 +282,9 @@ public class TestRedirects extends LocalServerTestBase {
Assert.assertEquals(HttpStatus.SC_MULTIPLE_CHOICES, response.getCode());
Assert.assertEquals(URIUtils.create(target, "/oldlocation/"), reqWrapper.getUri());
- final List<URI> redirects = context.getRedirectLocations();
- Assert.assertNull(redirects);
+ final RedirectLocations redirects = context.getRedirectLocations();
+ Assert.assertNotNull(redirects);
+ Assert.assertEquals(0, redirects.size());
}
@Test
@@ -304,7 +305,7 @@ public class TestRedirects extends LocalServerTestBase {
Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
Assert.assertEquals(URIUtils.create(target, "/newlocation/"), reqWrapper.getUri());
- final List<URI> redirects = context.getRedirectLocations();
+ final RedirectLocations redirects = context.getRedirectLocations();
Assert.assertNotNull(redirects);
Assert.assertEquals(1, redirects.size());
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
index 75c3cda..54c9048 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
@@ -30,13 +30,7 @@ package org.apache.hc.client5.http.impl;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Locale;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import org.apache.hc.client5.http.CircularRedirectException;
-import org.apache.hc.client5.http.config.RequestConfig;
-import org.apache.hc.client5.http.protocol.HttpClientContext;
-import org.apache.hc.client5.http.protocol.RedirectLocations;
import org.apache.hc.client5.http.protocol.RedirectStrategy;
import org.apache.hc.client5.http.utils.URIUtils;
import org.apache.hc.core5.annotation.Contract;
@@ -52,8 +46,6 @@ import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.net.URIBuilder;
import org.apache.hc.core5.util.Args;
import org.apache.hc.core5.util.TextUtils;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
/**
* Default implementation of {@link RedirectStrategy}. This strategy honors the restrictions
@@ -64,27 +56,11 @@ import org.apache.logging.log4j.Logger;
*
* @since 4.1
*/
-@Contract(threading = ThreadingBehavior.IMMUTABLE)
+@Contract(threading = ThreadingBehavior.STATELESS)
public class DefaultRedirectStrategy implements RedirectStrategy {
- private final Logger log = LogManager.getLogger(getClass());
-
public static final DefaultRedirectStrategy INSTANCE = new DefaultRedirectStrategy();
- private final ConcurrentMap<String, Boolean> safeMethods;
-
- public DefaultRedirectStrategy(final String... safeMethods) {
- super();
- this.safeMethods = new ConcurrentHashMap<>();
- for (final String safeMethod: safeMethods) {
- this.safeMethods.put(safeMethod.toUpperCase(Locale.ROOT), Boolean.TRUE);
- }
- }
-
- public DefaultRedirectStrategy() {
- this("GET", "HEAD", "OPTIONS", "TRACE");
- }
-
@Override
public boolean isRedirected(
final HttpRequest request,
@@ -117,20 +93,12 @@ public class DefaultRedirectStrategy implements RedirectStrategy {
Args.notNull(response, "HTTP response");
Args.notNull(context, "HTTP context");
- final HttpClientContext clientContext = HttpClientContext.adapt(context);
-
//get the location header to find out where to redirect to
final Header locationHeader = response.getFirstHeader("location");
if (locationHeader == null) {
throw new HttpException("Redirect location is missing");
}
final String location = locationHeader.getValue();
- if (this.log.isDebugEnabled()) {
- this.log.debug("Redirect requested to location '" + location + "'");
- }
-
- final RequestConfig config = clientContext.getRequestConfig();
-
URI uri = createLocationURI(location);
try {
if (!uri.isAbsolute()) {
@@ -141,18 +109,6 @@ public class DefaultRedirectStrategy implements RedirectStrategy {
throw new ProtocolException(ex.getMessage(), ex);
}
- RedirectLocations redirectLocations = (RedirectLocations) clientContext.getAttribute(
- HttpClientContext.REDIRECT_LOCATIONS);
- if (redirectLocations == null) {
- redirectLocations = new RedirectLocations();
- context.setAttribute(HttpClientContext.REDIRECT_LOCATIONS, redirectLocations);
- }
- if (!config.isCircularRedirectsAllowed()) {
- if (redirectLocations.contains(uri)) {
- throw new CircularRedirectException("Circular redirect to '" + uri + "'");
- }
- }
- redirectLocations.add(uri);
return uri;
}
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java
index 4f34bf6..cdd86b5 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java
@@ -28,8 +28,8 @@ package org.apache.hc.client5.http.impl.async;
import java.io.IOException;
import java.net.URI;
-import java.util.List;
+import org.apache.hc.client5.http.CircularRedirectException;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.RedirectException;
import org.apache.hc.client5.http.StandardMethods;
@@ -40,6 +40,7 @@ import org.apache.hc.client5.http.auth.AuthExchange;
import org.apache.hc.client5.http.auth.AuthScheme;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.protocol.HttpClientContext;
+import org.apache.hc.client5.http.protocol.RedirectLocations;
import org.apache.hc.client5.http.protocol.RedirectStrategy;
import org.apache.hc.client5.http.routing.HttpRoutePlanner;
import org.apache.hc.client5.http.utils.URIUtils;
@@ -53,6 +54,7 @@ import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.message.BasicHttpRequest;
import org.apache.hc.core5.http.nio.AsyncDataConsumer;
import org.apache.hc.core5.http.nio.AsyncEntityProducer;
+import org.apache.hc.core5.util.LangUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -75,6 +77,7 @@ class AsyncRedirectExec implements AsyncExecChainHandler {
volatile int redirectCount;
volatile HttpRequest currentRequest;
volatile AsyncEntityProducer currentEntityProducer;
+ volatile RedirectLocations redirectLocations;
volatile AsyncExecChain.Scope currentScope;
volatile boolean reroute;
@@ -107,6 +110,16 @@ class AsyncRedirectExec implements AsyncExecChainHandler {
state.redirectCount++;
final URI redirectUri = redirectStrategy.getLocationURI(request, response, clientContext);
+ if (log.isDebugEnabled()) {
+ log.debug("Redirect requested to location '" + redirectUri + "'");
+ }
+ if (!config.isCircularRedirectsAllowed()) {
+ if (state.redirectLocations.contains(redirectUri)) {
+ throw new CircularRedirectException("Circular redirect to '" + redirectUri + "'");
+ }
+ }
+ state.redirectLocations.add(redirectUri);
+
final int statusCode = response.getCode();
switch (statusCode) {
@@ -128,27 +141,27 @@ class AsyncRedirectExec implements AsyncExecChainHandler {
throw new ProtocolException("Redirect URI does not specify a valid host name: " + redirectUri);
}
- if (!currentRoute.getTargetHost().equals(newTarget)) {
- log.debug("New route required");
- state.reroute = true;
- final AuthExchange targetAuthExchange = clientContext.getAuthExchange(currentRoute.getTargetHost());
- log.debug("Resetting target auth state");
- targetAuthExchange.reset();
- if (currentRoute.getProxyHost() != null) {
- final AuthExchange proxyAuthExchange = clientContext.getAuthExchange(currentRoute.getProxyHost());
- final AuthScheme authScheme = proxyAuthExchange.getAuthScheme();
- if (authScheme != null && authScheme.isConnectionBased()) {
- log.debug("Resetting proxy auth state");
- proxyAuthExchange.reset();
+ state.reroute = false;
+ state.redirectURI = redirectUri;
+
+ if (!LangUtils.equals(currentRoute.getTargetHost(), newTarget)) {
+ final HttpRoute newRoute = routePlanner.determineRoute(newTarget, clientContext);
+ if (!LangUtils.equals(currentRoute, newRoute)) {
+ state.reroute = true;
+ final AuthExchange targetAuthExchange = clientContext.getAuthExchange(currentRoute.getTargetHost());
+ log.debug("Resetting target auth state");
+ targetAuthExchange.reset();
+ if (currentRoute.getProxyHost() != null) {
+ final AuthExchange proxyAuthExchange = clientContext.getAuthExchange(currentRoute.getProxyHost());
+ final AuthScheme authScheme = proxyAuthExchange.getAuthScheme();
+ if (authScheme != null && authScheme.isConnectionBased()) {
+ log.debug("Resetting proxy auth state");
+ proxyAuthExchange.reset();
+ }
}
+ state.currentScope = new AsyncExecChain.Scope(scope.exchangeId, newRoute,
+ scope.originalRequest, scope.future, clientContext, scope.execRuntime);
}
- final HttpRoute newRoute = routePlanner.determineRoute(newTarget, clientContext);
- state.currentScope = new AsyncExecChain.Scope(scope.exchangeId, newRoute,
- scope.originalRequest, scope.future, clientContext, scope.execRuntime);
- state.redirectURI = redirectUri;
- } else {
- state.reroute = false;
- state.redirectURI = redirectUri;
}
}
if (state.redirectURI != null) {
@@ -200,10 +213,13 @@ class AsyncRedirectExec implements AsyncExecChainHandler {
final AsyncExecChain chain,
final AsyncExecCallback asyncExecCallback) throws HttpException, IOException {
final HttpClientContext clientContext = scope.clientContext;
- final List<URI> redirectLocations = clientContext.getRedirectLocations();
- if (redirectLocations != null) {
- redirectLocations.clear();
+ RedirectLocations redirectLocations = clientContext.getRedirectLocations();
+ if (redirectLocations == null) {
+ redirectLocations = new RedirectLocations();
+ clientContext.setAttribute(HttpClientContext.REDIRECT_LOCATIONS, redirectLocations);
}
+ redirectLocations.clear();
+
final RequestConfig config = clientContext.getRequestConfig();
final State state = new State();
@@ -211,6 +227,7 @@ class AsyncRedirectExec implements AsyncExecChainHandler {
state.redirectCount = 0;
state.currentRequest = request;
state.currentEntityProducer = entityProducer;
+ state.redirectLocations = redirectLocations;
state.currentScope = scope;
internalExecute(state, chain, asyncExecCallback);
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java
index 99b02cb..80e5cbf 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java
@@ -29,8 +29,8 @@ package org.apache.hc.client5.http.impl.classic;
import java.io.IOException;
import java.net.URI;
-import java.util.List;
+import org.apache.hc.client5.http.CircularRedirectException;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.RedirectException;
import org.apache.hc.client5.http.StandardMethods;
@@ -42,6 +42,7 @@ import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.RequestBuilder;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.protocol.HttpClientContext;
+import org.apache.hc.client5.http.protocol.RedirectLocations;
import org.apache.hc.client5.http.protocol.RedirectStrategy;
import org.apache.hc.client5.http.routing.HttpRoutePlanner;
import org.apache.hc.client5.http.utils.URIUtils;
@@ -56,6 +57,7 @@ import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.util.Args;
+import org.apache.hc.core5.util.LangUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -97,11 +99,12 @@ final class RedirectExec implements ExecChainHandler {
Args.notNull(scope, "Scope");
final HttpClientContext context = scope.clientContext;
-
- final List<URI> redirectLocations = context.getRedirectLocations();
- if (redirectLocations != null) {
- redirectLocations.clear();
+ RedirectLocations redirectLocations = context.getRedirectLocations();
+ if (redirectLocations == null) {
+ redirectLocations = new RedirectLocations();
+ context.setAttribute(HttpClientContext.REDIRECT_LOCATIONS, redirectLocations);
}
+ redirectLocations.clear();
final RequestConfig config = context.getRequestConfig();
final int maxRedirects = config.getMaxRedirects() > 0 ? config.getMaxRedirects() : 50;
@@ -122,6 +125,16 @@ final class RedirectExec implements ExecChainHandler {
redirectCount++;
final URI redirectUri = this.redirectStrategy.getLocationURI(currentRequest, response, context);
+ if (this.log.isDebugEnabled()) {
+ this.log.debug("Redirect requested to location '" + redirectUri + "'");
+ }
+ if (!config.isCircularRedirectsAllowed()) {
+ if (redirectLocations.contains(redirectUri)) {
+ throw new CircularRedirectException("Circular redirect to '" + redirectUri + "'");
+ }
+ }
+ redirectLocations.add(redirectUri);
+
final ClassicHttpRequest originalRequest = scope.originalRequest;
ClassicHttpRequest redirect = null;
final int statusCode = response.getCode();
@@ -147,28 +160,29 @@ final class RedirectExec implements ExecChainHandler {
redirectUri);
}
- HttpRoute currentRoute = currentScope.route;
- final boolean crossSiteRedirect = !currentRoute.getTargetHost().equals(newTarget);
- if (crossSiteRedirect) {
-
- final AuthExchange targetAuthExchange = context.getAuthExchange(currentRoute.getTargetHost());
- this.log.debug("Resetting target auth state");
- targetAuthExchange.reset();
- if (currentRoute.getProxyHost() != null) {
- final AuthExchange proxyAuthExchange = context.getAuthExchange(currentRoute.getProxyHost());
- final AuthScheme authScheme = proxyAuthExchange.getAuthScheme();
- if (authScheme != null && authScheme.isConnectionBased()) {
- this.log.debug("Resetting proxy auth state");
- proxyAuthExchange.reset();
+ final HttpRoute currentRoute = currentScope.route;
+ if (!LangUtils.equals(currentRoute.getTargetHost(), newTarget)) {
+ final HttpRoute newRoute = this.routePlanner.determineRoute(newTarget, context);
+ if (!LangUtils.equals(currentRoute, newRoute)) {
+ log.debug("New route required");
+ final AuthExchange targetAuthExchange = context.getAuthExchange(currentRoute.getTargetHost());
+ this.log.debug("Resetting target auth state");
+ targetAuthExchange.reset();
+ if (currentRoute.getProxyHost() != null) {
+ final AuthExchange proxyAuthExchange = context.getAuthExchange(currentRoute.getProxyHost());
+ final AuthScheme authScheme = proxyAuthExchange.getAuthScheme();
+ if (authScheme != null && authScheme.isConnectionBased()) {
+ this.log.debug("Resetting proxy auth state");
+ proxyAuthExchange.reset();
+ }
}
+ currentScope = new ExecChain.Scope(
+ currentScope.exchangeId,
+ newRoute,
+ currentScope.originalRequest,
+ currentScope.execRuntime,
+ currentScope.clientContext);
}
- currentRoute = this.routePlanner.determineRoute(newTarget, context);
- currentScope = new ExecChain.Scope(
- currentScope.exchangeId,
- currentRoute,
- currentScope.originalRequest,
- currentScope.execRuntime,
- currentScope.clientContext);
}
if (this.log.isDebugEnabled()) {
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/HttpClientContext.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/HttpClientContext.java b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/HttpClientContext.java
index e717a90..7be4c16 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/HttpClientContext.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/HttpClientContext.java
@@ -27,9 +27,7 @@
package org.apache.hc.client5.http.protocol;
-import java.net.URI;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import org.apache.hc.client5.http.HttpRoute;
@@ -67,7 +65,7 @@ public class HttpClientContext extends HttpCoreContext {
public static final String HTTP_ROUTE = "http.route";
/**
- * Attribute name of a {@link List} object that represents a collection of all
+ * Attribute name of a {@link RedirectLocations} object that represents a collection of all
* redirect locations received in the process of request execution.
*/
public static final String REDIRECT_LOCATIONS = "http.protocol.redirect-locations";
@@ -156,9 +154,8 @@ public class HttpClientContext extends HttpCoreContext {
return getAttribute(HTTP_ROUTE, HttpRoute.class);
}
- @SuppressWarnings("unchecked")
- public List<URI> getRedirectLocations() {
- return getAttribute(REDIRECT_LOCATIONS, List.class);
+ public RedirectLocations getRedirectLocations() {
+ return getAttribute(REDIRECT_LOCATIONS, RedirectLocations.class);
}
public CookieStore getCookieStore() {
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RedirectLocations.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RedirectLocations.java b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RedirectLocations.java
index effcbee..22fb6ff 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RedirectLocations.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RedirectLocations.java
@@ -28,10 +28,8 @@
package org.apache.hc.client5.http.protocol;
import java.net.URI;
-import java.util.AbstractList;
import java.util.ArrayList;
import java.util.HashSet;
-import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -41,7 +39,7 @@ import java.util.Set;
*
* @since 4.0
*/
-public class RedirectLocations extends AbstractList<Object> {
+public final class RedirectLocations {
private final Set<URI> unique;
private final List<URI> all;
@@ -68,23 +66,6 @@ public class RedirectLocations extends AbstractList<Object> {
}
/**
- * Removes a URI from the collection.
- */
- public boolean remove(final URI uri) {
- final boolean removed = this.unique.remove(uri);
- if (removed) {
- final Iterator<URI> it = this.all.iterator();
- while (it.hasNext()) {
- final URI current = it.next();
- if (current.equals(uri)) {
- it.remove();
- }
- }
- }
- return removed;
- }
-
- /**
* Returns all redirect {@link URI}s in the order they were added to the collection.
*
* @return list of all URIs
@@ -106,7 +87,6 @@ public class RedirectLocations extends AbstractList<Object> {
* {@code index < 0 || index >= size()})
* @since 4.3
*/
- @Override
public URI get(final int index) {
return this.all.get(index);
}
@@ -119,106 +99,13 @@ public class RedirectLocations extends AbstractList<Object> {
* @return the number of elements in this list
* @since 4.3
*/
- @Override
public int size() {
return this.all.size();
}
- /**
- * Replaces the URI at the specified position in this list with the
- * specified element (must be a URI).
- *
- * @param index
- * index of the element to replace
- * @param element
- * URI to be stored at the specified position
- * @return the URI previously at the specified position
- * @throws UnsupportedOperationException
- * if the {@code set} operation is not supported by this list
- * @throws ClassCastException
- * if the element is not a {@link URI}
- * @throws NullPointerException
- * if the specified element is null and this list does not
- * permit null elements
- * @throws IndexOutOfBoundsException
- * if the index is out of range (
- * {@code index < 0 || index >= size()})
- * @since 4.3
- */
- @Override
- public Object set(final int index, final Object element) {
- final URI removed = this.all.set(index, (URI) element);
- this.unique.remove(removed);
- this.unique.add((URI) element);
- if (this.all.size() != this.unique.size()) {
- this.unique.addAll(this.all);
- }
- return removed;
- }
-
- /**
- * Inserts the specified element at the specified position in this list
- * (must be a URI). Shifts the URI currently at that position (if any) and
- * any subsequent URIs to the right (adds one to their indices).
- *
- * @param index
- * index at which the specified element is to be inserted
- * @param element
- * URI to be inserted
- * @throws UnsupportedOperationException
- * if the {@code add} operation is not supported by this list
- * @throws ClassCastException
- * if the element is not a {@link URI}
- * @throws NullPointerException
- * if the specified element is null and this list does not
- * permit null elements
- * @throws IndexOutOfBoundsException
- * if the index is out of range (
- * {@code index < 0 || index > size()})
- * @since 4.3
- */
- @Override
- public void add(final int index, final Object element) {
- this.all.add(index, (URI) element);
- this.unique.add((URI) element);
- }
-
- /**
- * Removes the URI at the specified position in this list. Shifts any
- * subsequent URIs to the left (subtracts one from their indices). Returns
- * the URI that was removed from the list.
- *
- * @param index
- * the index of the URI to be removed
- * @return the URI previously at the specified position
- * @throws IndexOutOfBoundsException
- * if the index is out of range (
- * {@code index < 0 || index >= size()})
- * @since 4.3
- */
- @Override
- public URI remove(final int index) {
- final URI removed = this.all.remove(index);
- this.unique.remove(removed);
- if (this.all.size() != this.unique.size()) {
- this.unique.addAll(this.all);
- }
- return removed;
- }
-
- /**
- * Returns {@code true} if this collection contains the specified element.
- * More formally, returns {@code true} if and only if this collection
- * contains at least one element {@code e} such that
- * {@code (o==null ? e==null : o.equals(e))}.
- *
- * @param o element whose presence in this collection is to be tested
- * @return {@code true} if this collection contains the specified
- * element
- */
- @Override
- public boolean contains(final Object o) {
- return this.unique.contains(o);
+ public void clear() {
+ unique.clear();
+ all.clear();
}
}
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java
index 1fc2eb2..6ca7e60 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java
@@ -27,11 +27,9 @@
package org.apache.hc.client5.http.impl;
import java.net.URI;
-import java.util.List;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
-import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
@@ -201,53 +199,6 @@ public class TestDefaultRedirectStrategy {
}
@Test
- public void testGetLocationUriAllowCircularRedirects() throws Exception {
- final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
- final HttpClientContext context = HttpClientContext.create();
- final RequestConfig config = RequestConfig.custom().setCircularRedirectsAllowed(true).build();
- context.setRequestConfig(config);
- final URI uri1 = URI.create("http://localhost/stuff1");
- final URI uri2 = URI.create("http://localhost/stuff2");
- final URI uri3 = URI.create("http://localhost/stuff3");
- final HttpGet httpget1 = new HttpGet("http://localhost/");
- final HttpResponse response1 = new BasicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
- response1.addHeader("Location", uri1.toASCIIString());
- final HttpGet httpget2 = new HttpGet(uri1.toASCIIString());
- final HttpResponse response2 = new BasicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
- response2.addHeader("Location", uri2.toASCIIString());
- final HttpGet httpget3 = new HttpGet(uri2.toASCIIString());
- final HttpResponse response3 = new BasicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
- response3.addHeader("Location", uri3.toASCIIString());
- Assert.assertEquals(uri1, redirectStrategy.getLocationURI(httpget1, response1, context));
- Assert.assertEquals(uri2, redirectStrategy.getLocationURI(httpget2, response2, context));
- Assert.assertEquals(uri3, redirectStrategy.getLocationURI(httpget3, response3, context));
-
- final List<URI> uris = context.getRedirectLocations();
- Assert.assertNotNull(uris);
- Assert.assertTrue(uris.contains(uri1));
- Assert.assertTrue(uris.contains(uri2));
- Assert.assertTrue(uris.contains(uri3));
- Assert.assertEquals(3, uris.size());
- Assert.assertEquals(uri1, uris.get(0));
- Assert.assertEquals(uri2, uris.get(1));
- Assert.assertEquals(uri3, uris.get(2));
- }
-
- @Test(expected=ProtocolException.class)
- public void testGetLocationUriDisallowCircularRedirects() throws Exception {
- final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
- final HttpClientContext context = HttpClientContext.create();
- final HttpGet httpget = new HttpGet("http://localhost/stuff");
- final RequestConfig config = RequestConfig.custom().setCircularRedirectsAllowed(false).build();
- context.setRequestConfig(config);
- final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
- response.addHeader("Location", "http://localhost/stuff");
- final URI uri = URI.create("http://localhost/stuff");
- Assert.assertEquals(uri, redirectStrategy.getLocationURI(httpget, response, context));
- redirectStrategy.getLocationURI(httpget, response, context);
- }
-
- @Test
public void testGetLocationUriInvalidInput() throws Exception {
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
final HttpClientContext context = HttpClientContext.create();
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRedirectExec.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRedirectExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRedirectExec.java
index 7fdb4cc..80cca6a 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRedirectExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRedirectExec.java
@@ -30,8 +30,10 @@ import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
+import java.util.Arrays;
import java.util.List;
+import org.apache.hc.client5.http.CircularRedirectException;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.RedirectException;
import org.apache.hc.client5.http.auth.AuthExchange;
@@ -40,48 +42,52 @@ import org.apache.hc.client5.http.classic.ExecRuntime;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.entity.EntityBuilder;
+import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
import org.apache.hc.client5.http.impl.auth.BasicScheme;
import org.apache.hc.client5.http.impl.auth.NTLMScheme;
import org.apache.hc.client5.http.protocol.HttpClientContext;
+import org.apache.hc.client5.http.protocol.RedirectLocations;
import org.apache.hc.client5.http.protocol.RedirectStrategy;
import org.apache.hc.client5.http.routing.HttpRoutePlanner;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.HttpResponse;
+import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.ProtocolException;
+import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
+import org.mockito.junit.MockitoJUnitRunner;
-@SuppressWarnings({"boxing","static-access"}) // test code
+@RunWith(MockitoJUnitRunner.class)
public class TestRedirectExec {
@Mock
private HttpRoutePlanner httpRoutePlanner;
@Mock
- private RedirectStrategy redirectStrategy;
- @Mock
private ExecChain chain;
@Mock
private ExecRuntime endpoint;
+ private RedirectStrategy redirectStrategy;
private RedirectExec redirectExec;
private HttpHost target;
@Before
public void setup() throws Exception {
- MockitoAnnotations.initMocks(this);
- redirectExec = new RedirectExec(httpRoutePlanner, redirectStrategy);
target = new HttpHost("localhost", 80);
+ redirectStrategy = Mockito.spy(new DefaultRedirectStrategy());
+ redirectExec = new RedirectExec(httpRoutePlanner, redirectStrategy);
}
@Test
@@ -90,19 +96,20 @@ public class TestRedirectExec {
final HttpGet request = new HttpGet("/test");
final HttpClientContext context = HttpClientContext.create();
- final ClassicHttpResponse response1 = Mockito.mock(ClassicHttpResponse.class);
+ final ClassicHttpResponse response1 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY));
+ final URI redirect = new URI("http://localhost:80/redirect");
+ response1.setHeader(HttpHeaders.LOCATION, redirect.toASCIIString());
final InputStream instream1 = Mockito.spy(new ByteArrayInputStream(new byte[] {1, 2, 3}));
final HttpEntity entity1 = EntityBuilder.create()
.setStream(instream1)
.build();
- Mockito.when(response1.getEntity()).thenReturn(entity1);
- final ClassicHttpResponse response2 = Mockito.mock(ClassicHttpResponse.class);
+ response1.setEntity(entity1);
+ final ClassicHttpResponse response2 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_OK));
final InputStream instream2 = Mockito.spy(new ByteArrayInputStream(new byte[] {1, 2, 3}));
final HttpEntity entity2 = EntityBuilder.create()
.setStream(instream2)
.build();
- Mockito.when(response2.getEntity()).thenReturn(entity2);
- final URI redirect = new URI("http://localhost:80/redirect");
+ response2.setEntity(entity2);
Mockito.when(chain.proceed(
Mockito.same(request),
@@ -110,26 +117,12 @@ public class TestRedirectExec {
Mockito.when(chain.proceed(
HttpRequestMatcher.matchesRequestUri(redirect),
Mockito.<ExecChain.Scope>any())).thenReturn(response2);
- Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
- Mockito.when(redirectStrategy.getLocationURI(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(redirect);
- Mockito.when(httpRoutePlanner.determineRoute(
- Mockito.eq(target),
- Mockito.<HttpClientContext>any())).thenReturn(route);
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
redirectExec.execute(request, scope, chain);
- final ArgumentCaptor<ClassicHttpRequest> reqCaptor = ArgumentCaptor.forClass(
- ClassicHttpRequest.class);
- Mockito.verify(chain, Mockito.times(2)).proceed(
- reqCaptor.capture(),
- Mockito.same(scope));
+ final ArgumentCaptor<ClassicHttpRequest> reqCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class);
+ Mockito.verify(chain, Mockito.times(2)).proceed(reqCaptor.capture(), Mockito.same(scope));
final List<ClassicHttpRequest> allValues = reqCaptor.getAllValues();
Assert.assertNotNull(allValues);
@@ -137,7 +130,7 @@ public class TestRedirectExec {
Assert.assertSame(request, allValues.get(0));
Mockito.verify(response1, Mockito.times(1)).close();
- Mockito.verify(instream1, Mockito.times(1)).close();
+ Mockito.verify(instream1, Mockito.times(2)).close();
Mockito.verify(response2, Mockito.never()).close();
Mockito.verify(instream2, Mockito.never()).close();
}
@@ -153,23 +146,11 @@ public class TestRedirectExec {
.build();
context.setRequestConfig(config);
- final ClassicHttpResponse response1 = Mockito.mock(ClassicHttpResponse.class);
+ final ClassicHttpResponse response1 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY));
final URI redirect = new URI("http://localhost:80/redirect");
+ response1.setHeader(HttpHeaders.LOCATION, redirect.toASCIIString());
- Mockito.when(chain.proceed(
- Mockito.<ClassicHttpRequest>any(),
- Mockito.<ExecChain.Scope>any())).thenReturn(response1);
- Mockito.when(redirectStrategy.isRedirected(
- Mockito.<ClassicHttpRequest>any(),
- Mockito.<HttpResponse>any(),
- Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
- Mockito.when(redirectStrategy.getLocationURI(
- Mockito.<ClassicHttpRequest>any(),
- Mockito.<HttpResponse>any(),
- Mockito.<HttpClientContext>any())).thenReturn(redirect);
- Mockito.when(httpRoutePlanner.determineRoute(
- Mockito.eq(target),
- Mockito.<HttpClientContext>any())).thenReturn(route);
+ Mockito.when(chain.proceed(Mockito.<ClassicHttpRequest>any(), Mockito.<ExecChain.Scope>any())).thenReturn(response1);
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
redirectExec.execute(request, scope, chain);
@@ -181,26 +162,12 @@ public class TestRedirectExec {
final HttpGet request = new HttpGet("/test");
final HttpClientContext context = HttpClientContext.create();
- final ClassicHttpResponse response1 = Mockito.mock(ClassicHttpResponse.class);
- final ClassicHttpResponse response2 = Mockito.mock(ClassicHttpResponse.class);
+ final ClassicHttpResponse response1 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY));
final URI redirect = new URI("/redirect");
+ response1.setHeader(HttpHeaders.LOCATION, redirect.toASCIIString());
Mockito.when(chain.proceed(
Mockito.same(request),
Mockito.<ExecChain.Scope>any())).thenReturn(response1);
- Mockito.when(chain.proceed(
- HttpRequestMatcher.matchesRequestUri(redirect),
- Mockito.<ExecChain.Scope>any())).thenReturn(response2);
- Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
- Mockito.when(redirectStrategy.getLocationURI(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(redirect);
- Mockito.when(httpRoutePlanner.determineRoute(
- Mockito.eq(target),
- Mockito.<HttpClientContext>any())).thenReturn(route);
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
redirectExec.execute(request, scope, chain);
@@ -223,27 +190,17 @@ public class TestRedirectExec {
context.setAuthExchange(target, targetAuthExchange);
context.setAuthExchange(proxy, proxyAuthExchange);
- final ClassicHttpResponse response1 = Mockito.mock(ClassicHttpResponse.class);
- final ClassicHttpResponse response2 = Mockito.mock(ClassicHttpResponse.class);
- final HttpHost otherHost = new HttpHost("otherhost", 8888);
+ final ClassicHttpResponse response1 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY));
final URI redirect = new URI("http://otherhost:8888/redirect");
+ response1.setHeader(HttpHeaders.LOCATION, redirect.toASCIIString());
+ final ClassicHttpResponse response2 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_OK));
+ final HttpHost otherHost = new HttpHost("otherhost", 8888);
Mockito.when(chain.proceed(
Mockito.same(request),
Mockito.<ExecChain.Scope>any())).thenReturn(response1);
Mockito.when(chain.proceed(
HttpRequestMatcher.matchesRequestUri(redirect),
Mockito.<ExecChain.Scope>any())).thenReturn(response2);
- Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
- Mockito.when(redirectStrategy.getLocationURI(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(redirect);
- Mockito.when(httpRoutePlanner.determineRoute(
- Mockito.eq(target),
- Mockito.<HttpClientContext>any())).thenReturn(new HttpRoute(target));
Mockito.when(httpRoutePlanner.determineRoute(
Mockito.eq(otherHost),
Mockito.<HttpClientContext>any())).thenReturn(new HttpRoute(otherHost));
@@ -261,24 +218,101 @@ public class TestRedirectExec {
Assert.assertEquals(null, authExchange2.getAuthScheme());
}
+ @Test
+ public void testAllowCircularRedirects() throws Exception {
+ final HttpRoute route = new HttpRoute(target);
+ final HttpClientContext context = HttpClientContext.create();
+ context.setRequestConfig(RequestConfig.custom()
+ .setCircularRedirectsAllowed(true)
+ .build());
+
+ final URI uri = URI.create("http://localhost/");
+ final HttpGet request = new HttpGet(uri);
+
+ final URI uri1 = URI.create("http://localhost/stuff1");
+ final URI uri2 = URI.create("http://localhost/stuff2");
+ final ClassicHttpResponse response1 = new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY);
+ response1.addHeader("Location", uri1.toASCIIString());
+ final ClassicHttpResponse response2 = new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY);
+ response2.addHeader("Location", uri2.toASCIIString());
+ final ClassicHttpResponse response3 = new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY);
+ response3.addHeader("Location", uri1.toASCIIString());
+ final ClassicHttpResponse response4 = new BasicClassicHttpResponse(HttpStatus.SC_OK);
+
+ Mockito.when(chain.proceed(
+ HttpRequestMatcher.matchesRequestUri(uri),
+ Mockito.<ExecChain.Scope>any())).thenReturn(response1);
+ Mockito.when(chain.proceed(
+ HttpRequestMatcher.matchesRequestUri(uri1),
+ Mockito.<ExecChain.Scope>any())).thenReturn(response2, response4);
+ Mockito.when(chain.proceed(
+ HttpRequestMatcher.matchesRequestUri(uri2),
+ Mockito.<ExecChain.Scope>any())).thenReturn(response3);
+ Mockito.when(httpRoutePlanner.determineRoute(
+ Mockito.eq(new HttpHost("localhost")),
+ Mockito.<HttpClientContext>any())).thenReturn(route);
+
+ final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
+ redirectExec.execute(request, scope, chain);
+
+ final RedirectLocations uris = context.getRedirectLocations();
+ Assert.assertNotNull(uris);
+ Assert.assertEquals(Arrays.asList(uri1, uri2, uri1), uris.getAll());
+ }
+
+ @Test(expected=CircularRedirectException.class)
+ public void testGetLocationUriDisallowCircularRedirects() throws Exception {
+ final HttpRoute route = new HttpRoute(target);
+ final HttpClientContext context = HttpClientContext.create();
+ context.setRequestConfig(RequestConfig.custom()
+ .setCircularRedirectsAllowed(false)
+ .build());
+
+ final URI uri = URI.create("http://localhost/");
+ final HttpGet request = new HttpGet(uri);
+
+ final URI uri1 = URI.create("http://localhost/stuff1");
+ final URI uri2 = URI.create("http://localhost/stuff2");
+ final ClassicHttpResponse response1 = new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY);
+ response1.addHeader("Location", uri1.toASCIIString());
+ final ClassicHttpResponse response2 = new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY);
+ response2.addHeader("Location", uri2.toASCIIString());
+ final ClassicHttpResponse response3 = new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY);
+ response3.addHeader("Location", uri1.toASCIIString());
+ Mockito.when(httpRoutePlanner.determineRoute(
+ Mockito.eq(new HttpHost("localhost")),
+ Mockito.<HttpClientContext>any())).thenReturn(route);
+
+ Mockito.when(chain.proceed(
+ HttpRequestMatcher.matchesRequestUri(uri),
+ Mockito.<ExecChain.Scope>any())).thenReturn(response1);
+ Mockito.when(chain.proceed(
+ HttpRequestMatcher.matchesRequestUri(uri1),
+ Mockito.<ExecChain.Scope>any())).thenReturn(response2);
+ Mockito.when(chain.proceed(
+ HttpRequestMatcher.matchesRequestUri(uri2),
+ Mockito.<ExecChain.Scope>any())).thenReturn(response3);
+
+ final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
+ redirectExec.execute(request, scope, chain);
+ }
+
@Test(expected = RuntimeException.class)
public void testRedirectRuntimeException() throws Exception {
final HttpRoute route = new HttpRoute(target);
final HttpGet request = new HttpGet("/test");
final HttpClientContext context = HttpClientContext.create();
- final ClassicHttpResponse response1 = Mockito.mock(ClassicHttpResponse.class);
+ final ClassicHttpResponse response1 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY));
+ final URI redirect = new URI("http://localhost:80/redirect");
+ response1.setHeader(HttpHeaders.LOCATION, redirect.toASCIIString());
Mockito.when(chain.proceed(
Mockito.same(request),
Mockito.<ExecChain.Scope>any())).thenReturn(response1);
- Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
- Mockito.doThrow(new RuntimeException("Oppsie")).when(redirectStrategy.getLocationURI(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any()));
+ Mockito.doThrow(new RuntimeException("Oppsie")).when(redirectStrategy).getLocationURI(
+ Mockito.<ClassicHttpRequest>any(),
+ Mockito.<ClassicHttpResponse>any(),
+ Mockito.<HttpClientContext>any());
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
try {
@@ -295,29 +329,27 @@ public class TestRedirectExec {
final HttpGet request = new HttpGet("/test");
final HttpClientContext context = HttpClientContext.create();
- final ClassicHttpResponse response1 = Mockito.mock(ClassicHttpResponse.class);
+ final ClassicHttpResponse response1 = Mockito.spy(new BasicClassicHttpResponse(HttpStatus.SC_MOVED_TEMPORARILY));
+ final URI redirect = new URI("http://localhost:80/redirect");
+ response1.setHeader(HttpHeaders.LOCATION, redirect.toASCIIString());
final InputStream instream1 = Mockito.spy(new ByteArrayInputStream(new byte[] {1, 2, 3}));
final HttpEntity entity1 = EntityBuilder.create()
.setStream(instream1)
.build();
- Mockito.when(response1.getEntity()).thenReturn(entity1);
+ response1.setEntity(entity1);
Mockito.when(chain.proceed(
Mockito.same(request),
Mockito.<ExecChain.Scope>any())).thenReturn(response1);
- Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
- Mockito.same(response1),
- Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
Mockito.doThrow(new ProtocolException("Oppsie")).when(redirectStrategy).getLocationURI(
- Mockito.same(request),
- Mockito.same(response1),
+ Mockito.<ClassicHttpRequest>any(),
+ Mockito.<ClassicHttpResponse>any(),
Mockito.<HttpClientContext>any());
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
try {
redirectExec.execute(request, scope, chain);
} catch (final Exception ex) {
- Mockito.verify(instream1).close();
+ Mockito.verify(instream1, Mockito.times(2)).close();
Mockito.verify(response1).close();
throw ex;
}
@@ -325,17 +357,21 @@ public class TestRedirectExec {
private static class HttpRequestMatcher implements ArgumentMatcher<ClassicHttpRequest> {
- private final URI requestUri;
+ private final URI expectedRequestUri;
HttpRequestMatcher(final URI requestUri) {
super();
- this.requestUri = requestUri;
+ this.expectedRequestUri = requestUri;
}
@Override
public boolean matches(final ClassicHttpRequest argument) {
+ if (argument == null) {
+ return false;
+ }
try {
- return requestUri.equals(argument.getUri());
+ final URI requestUri = argument.getUri();
+ return this.expectedRequestUri.equals(requestUri);
} catch (final URISyntaxException ex) {
return false;
}
http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/47a0eb8b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRedirectLocation.java
----------------------------------------------------------------------
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRedirectLocation.java b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRedirectLocation.java
index 95f4693..3923afc 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRedirectLocation.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRedirectLocation.java
@@ -56,7 +56,7 @@ public class TestRedirectLocation {
Assert.assertTrue(locations.contains(uri3));
Assert.assertFalse(locations.contains(new URI("/")));
- List<URI> list = locations.getAll();
+ final List<URI> list = locations.getAll();
Assert.assertNotNull(list);
Assert.assertEquals(5, list.size());
Assert.assertEquals(uri1, list.get(0));
@@ -64,16 +64,6 @@ public class TestRedirectLocation {
Assert.assertEquals(uri2, list.get(2));
Assert.assertEquals(uri3, list.get(3));
Assert.assertEquals(uri3, list.get(4));
-
- Assert.assertTrue(locations.remove(uri3));
- Assert.assertTrue(locations.remove(uri1));
- Assert.assertFalse(locations.remove(new URI("/")));
-
- list = locations.getAll();
- Assert.assertNotNull(list);
- Assert.assertEquals(2, list.size());
- Assert.assertEquals(uri2, list.get(0));
- Assert.assertEquals(uri2, list.get(1));
}
}