hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject 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...
Date Sun, 21 Jun 2015 10:43:49 GMT
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());
 



Mime
View raw message