RFC 7231: revised redirect handling

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1686698 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Oleg Kalnichevski 2015-06-21 10:43:48 +00:00
parent 12a8eaabab
commit 93525ebb36
7 changed files with 52 additions and 179 deletions

View File

@ -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 @@ boolean isRedirected(
HttpUriRequest getRedirect(
HttpRequest request,
HttpResponse response,
HttpContext context) throws ProtocolException;
HttpContext context) throws HttpException;
}

View File

@ -34,6 +34,8 @@
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.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 @@
* {@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 implements RedirectStrategy {
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 boolean isRedirected(
Args.notNull(request, "HTTP request");
Args.notNull(response, "HTTP response");
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:
if (!response.containsHeader(HttpHeaders.LOCATION)) {
return false;
} //end of switch
}
final int statusCode = response.getStatusLine().getStatusCode();
switch (statusCode) {
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 URI getLocationURI(
//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 @@ protected URI createLocationURI(final String location) throws ProtocolException
}
}
/**
* @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);
}
}
}

View File

@ -1,67 +0,0 @@
/*
* ====================================================================
* 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.http.impl.client;
import org.apache.http.annotation.Immutable;
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;
/**
* Lax {@link org.apache.http.client.RedirectStrategy} implementation
* that automatically redirects all HEAD, GET and POST requests.
* This strategy relaxes restrictions on automatic redirection of
* POST methods imposed by the HTTP specification.
*
* @since 4.2
*/
@Immutable
public class LaxRedirectStrategy extends DefaultRedirectStrategy {
/**
* Redirectable methods.
*/
private static final String[] REDIRECT_METHODS = new String[] {
HttpGet.METHOD_NAME,
HttpPost.METHOD_NAME,
HttpHead.METHOD_NAME,
HttpDelete.METHOD_NAME
};
@Override
protected boolean isRedirectable(final String method) {
for (final String m: REDIRECT_METHODS) {
if (m.equalsIgnoreCase(method)) {
return true;
}
}
return false;
}
}

View File

@ -111,7 +111,7 @@ public CloseableHttpResponse execute(
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 CloseableHttpResponse execute(
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());

View File

@ -31,17 +31,17 @@
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;
@ -55,16 +55,6 @@
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();
@ -72,10 +62,9 @@ public void testIsRedirectedMovedTemporary() throws Exception {
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 void testIsRedirectedMovedPermanently() throws Exception {
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 void testIsRedirectedTemporaryRedirect() throws Exception {
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 void testIsRedirectedSeeOther() throws Exception {
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 void testGetLocationUri() throws Exception {
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();

View File

@ -548,27 +548,6 @@ public void testDifferentRequestSameRedirect() throws Exception {
Assert.assertEquals(host, target);
}
@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));

View File

@ -120,11 +120,11 @@ public void testFundamentals() throws Exception {
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 void testRelativeRedirect() throws Exception {
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 void testCrossSiteRedirect() throws Exception {
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 void testRedirectProtocolException() throws Exception {
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());