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 2015/06/21 12:43:49 UTC
svn commit: r1686698 - in /httpcomponents/httpclient/trunk/httpclient/src:
main/java/org/apache/http/client/ main/java/org/apache/http/impl/client/
main/java/org/apache/http/impl/execchain/
test/java/org/apache/http/impl/client/ test/java/org/apache/ht...
Author: olegk
Date: Sun Jun 21 10:43:48 2015
New Revision: 1686698
URL: http://svn.apache.org/r1686698
Log:
RFC 7231: revised redirect handling
Removed:
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/LaxRedirectStrategy.java
Modified:
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/RedirectStrategy.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRedirectExec.java
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/RedirectStrategy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/RedirectStrategy.java?rev=1686698&r1=1686697&r2=1686698&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/RedirectStrategy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/RedirectStrategy.java Sun Jun 21 10:43:48 2015
@@ -27,9 +27,9 @@
package org.apache.http.client;
+import org.apache.http.HttpException;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
-import org.apache.http.ProtocolException;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.protocol.HttpContext;
@@ -60,7 +60,7 @@ public interface RedirectStrategy {
boolean isRedirected(
HttpRequest request,
HttpResponse response,
- HttpContext context) throws ProtocolException;
+ HttpContext context) throws HttpException;
/**
* Determines the redirect location given the response from the target
@@ -76,6 +76,6 @@ public interface RedirectStrategy {
HttpUriRequest getRedirect(
HttpRequest request,
HttpResponse response,
- HttpContext context) throws ProtocolException;
+ HttpContext context) throws HttpException;
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java?rev=1686698&r1=1686697&r2=1686698&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java Sun Jun 21 10:43:48 2015
@@ -34,6 +34,8 @@ import java.util.Locale;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.Header;
+import org.apache.http.HttpException;
+import org.apache.http.HttpHeaders;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
@@ -44,7 +46,6 @@ import org.apache.http.client.CircularRe
import org.apache.http.client.RedirectStrategy;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpHead;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.methods.RequestBuilder;
import org.apache.http.client.protocol.HttpClientContext;
@@ -62,12 +63,7 @@ import org.apache.http.util.TextUtils;
* {@code 307 Temporary Redirect} status codes will result in an automatic redirect of
* HEAD and GET methods only. POST and PUT methods will not be automatically redirected
* as requiring user confirmation.
- * <p>
- * The restriction on automatic redirection of POST methods can be relaxed by using
- * {@link LaxRedirectStrategy} instead of {@link DefaultRedirectStrategy}.
- * </p>
*
- * @see LaxRedirectStrategy
* @since 4.1
*/
@Immutable
@@ -77,14 +73,6 @@ public class DefaultRedirectStrategy imp
public static final DefaultRedirectStrategy INSTANCE = new DefaultRedirectStrategy();
- /**
- * Redirectable methods.
- */
- private static final String[] REDIRECT_METHODS = new String[] {
- HttpGet.METHOD_NAME,
- HttpHead.METHOD_NAME
- };
-
public DefaultRedirectStrategy() {
super();
}
@@ -97,26 +85,25 @@ public class DefaultRedirectStrategy imp
Args.notNull(request, "HTTP request");
Args.notNull(response, "HTTP response");
+ if (!response.containsHeader(HttpHeaders.LOCATION)) {
+ return false;
+ }
final int statusCode = response.getStatusLine().getStatusCode();
- final String method = request.getRequestLine().getMethod();
- final Header locationHeader = response.getFirstHeader("location");
switch (statusCode) {
- case HttpStatus.SC_MOVED_TEMPORARILY:
- return isRedirectable(method) && locationHeader != null;
- case HttpStatus.SC_MOVED_PERMANENTLY:
- case HttpStatus.SC_TEMPORARY_REDIRECT:
- return isRedirectable(method);
- case HttpStatus.SC_SEE_OTHER:
- return true;
- default:
- return false;
- } //end of switch
+ case HttpStatus.SC_MOVED_PERMANENTLY:
+ case HttpStatus.SC_MOVED_TEMPORARILY:
+ case HttpStatus.SC_SEE_OTHER:
+ case HttpStatus.SC_TEMPORARY_REDIRECT:
+ return true;
+ default:
+ return false;
+ }
}
public URI getLocationURI(
final HttpRequest request,
final HttpResponse response,
- final HttpContext context) throws ProtocolException {
+ final HttpContext context) throws HttpException {
Args.notNull(request, "HTTP request");
Args.notNull(response, "HTTP response");
Args.notNull(context, "HTTP context");
@@ -126,10 +113,7 @@ public class DefaultRedirectStrategy imp
//get the location header to find out where to redirect to
final Header locationHeader = response.getFirstHeader("location");
if (locationHeader == null) {
- // got a redirect response, but no location header
- throw new ProtocolException(
- "Received redirect response " + response.getStatusLine()
- + " but no location header");
+ throw new HttpException("Redirect location is missing");
}
final String location = locationHeader.getValue();
if (this.log.isDebugEnabled()) {
@@ -194,36 +178,24 @@ public class DefaultRedirectStrategy imp
}
}
- /**
- * @since 4.2
- */
- protected boolean isRedirectable(final String method) {
- for (final String m: REDIRECT_METHODS) {
- if (m.equalsIgnoreCase(method)) {
- return true;
- }
- }
- return false;
- }
-
@Override
public HttpUriRequest getRedirect(
final HttpRequest request,
final HttpResponse response,
- final HttpContext context) throws ProtocolException {
+ final HttpContext context) throws HttpException {
final URI uri = getLocationURI(request, response, context);
- final String method = request.getRequestLine().getMethod();
- if (method.equalsIgnoreCase(HttpHead.METHOD_NAME)) {
- return new HttpHead(uri);
- } else if (method.equalsIgnoreCase(HttpGet.METHOD_NAME)) {
- return new HttpGet(uri);
- } else {
- final int status = response.getStatusLine().getStatusCode();
- if (status == HttpStatus.SC_TEMPORARY_REDIRECT) {
+ final int statusCode = response.getStatusLine().getStatusCode();
+ switch (statusCode) {
+ case HttpStatus.SC_MOVED_PERMANENTLY:
+ case HttpStatus.SC_MOVED_TEMPORARILY:
+ case HttpStatus.SC_SEE_OTHER:
+ final String method = request.getRequestLine().getMethod();
+ if (method.equalsIgnoreCase("POST")) {
+ return new HttpGet(uri);
+ }
+ case HttpStatus.SC_TEMPORARY_REDIRECT:
+ default:
return RequestBuilder.copy(request).setUri(uri).build();
- } else {
- return new HttpGet(uri);
- }
}
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java?rev=1686698&r1=1686697&r2=1686698&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java Sun Jun 21 10:43:48 2015
@@ -111,7 +111,7 @@ public class RedirectExec implements Cli
currentRoute, currentRequest, context, execAware);
try {
if (config.isRedirectsEnabled() &&
- this.redirectStrategy.isRedirected(currentRequest, response, context)) {
+ this.redirectStrategy.isRedirected(currentRequest.getOriginal(), response, context)) {
if (redirectCount >= maxRedirects) {
throw new RedirectException("Maximum redirects ("+ maxRedirects + ") exceeded");
@@ -119,7 +119,7 @@ public class RedirectExec implements Cli
redirectCount++;
final HttpRequest redirect = this.redirectStrategy.getRedirect(
- currentRequest, response, context);
+ currentRequest.getOriginal(), response, context);
if (!redirect.headerIterator().hasNext()) {
final HttpRequest original = request.getOriginal();
redirect.setHeaders(original.getAllHeaders());
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java?rev=1686698&r1=1686697&r2=1686698&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java Sun Jun 21 10:43:48 2015
@@ -31,17 +31,17 @@ import java.util.List;
import org.apache.http.HttpEntity;
import org.apache.http.HttpEntityEnclosingRequest;
+import org.apache.http.HttpException;
+import org.apache.http.HttpHeaders;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.HttpVersion;
import org.apache.http.ProtocolException;
import org.apache.http.client.config.RequestConfig;
-import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpHead;
import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.methods.HttpTrace;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.protocol.HttpClientContext;
@@ -56,26 +56,15 @@ import org.junit.Test;
public class TestDefaultRedirectStrategy {
@Test
- public void testIsRedirectable() {
- final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
- Assert.assertTrue(redirectStrategy.isRedirectable(HttpGet.METHOD_NAME));
- Assert.assertTrue(redirectStrategy.isRedirectable(HttpHead.METHOD_NAME));
- Assert.assertFalse(redirectStrategy.isRedirectable(HttpPut.METHOD_NAME));
- Assert.assertFalse(redirectStrategy.isRedirectable(HttpPost.METHOD_NAME));
- Assert.assertFalse(redirectStrategy.isRedirectable(HttpDelete.METHOD_NAME));
- }
-
- @Test
public void testIsRedirectedMovedTemporary() throws Exception {
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
final HttpClientContext context = HttpClientContext.create();
final HttpGet httpget = new HttpGet("http://localhost/");
final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
- response.addHeader("Location", "http://localhost/stuff");
+ Assert.assertFalse(redirectStrategy.isRedirected(httpget, response, context));
+ response.setHeader(HttpHeaders.LOCATION, "http://localhost/blah");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
- final HttpPost httppost = new HttpPost("http://localhost/");
- Assert.assertFalse(redirectStrategy.isRedirected(httppost, response, context));
}
@Test
@@ -95,9 +84,9 @@ public class TestDefaultRedirectStrategy
final HttpGet httpget = new HttpGet("http://localhost/");
final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_PERMANENTLY, "Redirect");
+ Assert.assertFalse(redirectStrategy.isRedirected(httpget, response, context));
+ response.setHeader(HttpHeaders.LOCATION, "http://localhost/blah");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
- final HttpPost httppost = new HttpPost("http://localhost/");
- Assert.assertFalse(redirectStrategy.isRedirected(httppost, response, context));
}
@Test
@@ -107,9 +96,9 @@ public class TestDefaultRedirectStrategy
final HttpGet httpget = new HttpGet("http://localhost/");
final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_TEMPORARY_REDIRECT, "Redirect");
+ Assert.assertFalse(redirectStrategy.isRedirected(httpget, response, context));
+ response.setHeader(HttpHeaders.LOCATION, "http://localhost/blah");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
- final HttpPost httppost = new HttpPost("http://localhost/");
- Assert.assertFalse(redirectStrategy.isRedirected(httppost, response, context));
}
@Test
@@ -119,9 +108,9 @@ public class TestDefaultRedirectStrategy
final HttpGet httpget = new HttpGet("http://localhost/");
final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_SEE_OTHER, "Redirect");
+ Assert.assertFalse(redirectStrategy.isRedirected(httpget, response, context));
+ response.setHeader(HttpHeaders.LOCATION, "http://localhost/blah");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
- final HttpPost httppost = new HttpPost("http://localhost/");
- Assert.assertTrue(redirectStrategy.isRedirected(httppost, response, context));
}
@Test
@@ -166,7 +155,7 @@ public class TestDefaultRedirectStrategy
Assert.assertEquals(URI.create("http://localhost/stuff"), uri);
}
- @Test(expected=ProtocolException.class)
+ @Test(expected=HttpException.class)
public void testGetLocationUriMissingHeader() throws Exception {
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
final HttpClientContext context = HttpClientContext.create();
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java?rev=1686698&r1=1686697&r2=1686698&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java Sun Jun 21 10:43:48 2015
@@ -549,27 +549,6 @@ public class TestRedirects extends Local
}
@Test
- public void testPostNoRedirect() throws Exception {
- this.serverBootstrap.registerHandler("*", new BasicRedirectService());
-
- final HttpHost target = start();
-
- final HttpClientContext context = HttpClientContext.create();
-
- final HttpPost httppost = new HttpPost("/oldlocation/");
- httppost.setEntity(new StringEntity("stuff"));
-
- final HttpResponse response = this.httpclient.execute(target, httppost, context);
- EntityUtils.consume(response.getEntity());
-
- final HttpRequest reqWrapper = context.getRequest();
-
- Assert.assertEquals(HttpStatus.SC_MOVED_TEMPORARILY, response.getStatusLine().getStatusCode());
- Assert.assertEquals("/oldlocation/", reqWrapper.getRequestLine().getUri());
- Assert.assertEquals("POST", reqWrapper.getRequestLine().getMethod());
- }
-
- @Test
public void testPostRedirectSeeOther() throws Exception {
this.serverBootstrap.registerHandler("*", new BasicRedirectService(HttpStatus.SC_SEE_OTHER));
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRedirectExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRedirectExec.java?rev=1686698&r1=1686697&r2=1686698&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRedirectExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRedirectExec.java Sun Jun 21 10:43:48 2015
@@ -120,11 +120,11 @@ public class TestRedirectExec {
Mockito.<HttpClientContext>any(),
Mockito.<HttpExecutionAware>any())).thenReturn(response2);
Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
Mockito.when(redirectStrategy.getRedirect(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(redirect);
Mockito.when(httpRoutePlanner.determineRoute(
@@ -216,11 +216,11 @@ public class TestRedirectExec {
Mockito.<HttpClientContext>any(),
Mockito.<HttpExecutionAware>any())).thenReturn(response2);
Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
Mockito.when(redirectStrategy.getRedirect(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(redirect);
Mockito.when(httpRoutePlanner.determineRoute(
@@ -261,11 +261,11 @@ public class TestRedirectExec {
Mockito.<HttpClientContext>any(),
Mockito.<HttpExecutionAware>any())).thenReturn(response2);
Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
Mockito.when(redirectStrategy.getRedirect(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(redirect);
Mockito.when(httpRoutePlanner.determineRoute(
@@ -332,11 +332,11 @@ public class TestRedirectExec {
Mockito.<HttpClientContext>any(),
Mockito.<HttpExecutionAware>any())).thenReturn(response1);
Mockito.when(redirectStrategy.isRedirected(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
Mockito.doThrow(new ProtocolException("Oppsie")).when(redirectStrategy).getRedirect(
- Mockito.same(request),
+ Mockito.same(get),
Mockito.same(response1),
Mockito.<HttpClientContext>any());