HTTPCLIENT-860: HttpClient no longer converts redirects of PUT/POST to GET for status codes 301, 302, 307, as required by the HTTP spec

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@794051 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -62,6 +62,10 @@ Important notes
 Bug fixes since 4.0 BETA2 release 
 -------------------
 
+* [HTTPCLIENT-860] HttpClient no longer converts redirects of PUT/POST to GET 
+  for status codes 301, 302, 307, as required by the HTTP spec.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * [HTTPCLIENT-859] CookieIdentityComparator now takes path attribute into 
   consideration when comparing cookies.
   Contributed by Oleg Kalnichevski <olegk at apache.org>

httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectHandler.java

@@ -42,6 +42,8 @@ import org.apache.http.HttpStatus;
 import org.apache.http.ProtocolException;
 import org.apache.http.client.CircularRedirectException;
 import org.apache.http.client.RedirectHandler;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
 import org.apache.http.client.params.ClientPNames;
 import org.apache.http.client.utils.URIUtils;
 import org.apache.http.params.HttpParams;
@@ -70,12 +72,18 @@ public class DefaultRedirectHandler implements RedirectHandler {
         if (response == null) {
             throw new IllegalArgumentException("HTTP response may not be null");
         }
+        
         int statusCode = response.getStatusLine().getStatusCode();
         switch (statusCode) {
         case HttpStatus.SC_MOVED_TEMPORARILY:
         case HttpStatus.SC_MOVED_PERMANENTLY:
-        case HttpStatus.SC_SEE_OTHER:
         case HttpStatus.SC_TEMPORARY_REDIRECT:
+            HttpRequest request = (HttpRequest) context.getAttribute(
+                    ExecutionContext.HTTP_REQUEST);
+            String method = request.getRequestLine().getMethod();
+            return method.equalsIgnoreCase(HttpGet.METHOD_NAME) 
+                || method.equalsIgnoreCase(HttpHead.METHOD_NAME);
+        case HttpStatus.SC_SEE_OTHER:
             return true;
         default:
             return false;

httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java

@@ -64,7 +64,6 @@ import org.apache.http.client.RedirectException;
 import org.apache.http.client.RedirectHandler;
 import org.apache.http.client.UserTokenHandler;
 import org.apache.http.client.methods.AbortableHttpRequest;
-import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.params.ClientPNames;
 import org.apache.http.client.params.HttpClientParams;
 import org.apache.http.client.protocol.ClientContext;
@@ -950,9 +949,8 @@ public class DefaultRequestDirector implements RequestDirector {
                     proxyAuthState.invalidate();
                 }
             }
-            
+
-            HttpGet redirect = new HttpGet(uri);
+            HttpRedirect redirect = new HttpRedirect(request.getMethod(), uri); 
-            
             HttpRequest orig = request.getOriginal();
             redirect.setHeaders(orig.getAllHeaders());
             

httpclient/src/main/java/org/apache/http/impl/client/HttpRedirect.java

@@ -0,0 +1,126 @@
+/*
+ * ====================================================================
+ * 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 java.net.URI;
+
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpRequestBase;
+
+import net.jcip.annotations.NotThreadSafe;
+
+/**
+ * Redirect request (can be either GET or HEAD).
+ *  
+ * @since 4.0
+ */
+@NotThreadSafe
+class HttpRedirect extends HttpRequestBase {
+
+    private String method;
+    
+    public HttpRedirect(final String method, final URI uri) {
+        super();
+        if (method.equalsIgnoreCase(HttpHead.METHOD_NAME)) {
+            this.method = HttpHead.METHOD_NAME;
+        } else {
+            this.method = HttpGet.METHOD_NAME;
+        }
+        setURI(uri);
+    }
+
+    @Override
+    public String getMethod() {
+        return this.method;
+    }
+    
+}
+/*
+ * ====================================================================
+ * 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 java.net.URI;
+
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpRequestBase;
+
+import net.jcip.annotations.NotThreadSafe;
+
+/**
+ * Redirect request (can be either GET or HEAD).
+ *  
+ * @since 4.0
+ */
+@NotThreadSafe
+class HttpRedirect extends HttpRequestBase {
+
+    private String method;
+    
+    public HttpRedirect(final String method, final URI uri) {
+        super();
+        if (method.equalsIgnoreCase(HttpHead.METHOD_NAME)) {
+            this.method = HttpHead.METHOD_NAME;
+        } else {
+            this.method = HttpGet.METHOD_NAME;
+        }
+        setURI(uri);
+    }
+
+    @Override
+    public String getMethod() {
+        return this.method;
+    }
+    
+}

httpclient/src/test/java/org/apache/http/client/protocol/TestRedirects.java

@@ -451,7 +451,7 @@ public class TestRedirects extends BasicServerTestBase {
         }
     }
 
-    public void testPostRedirect() throws Exception {
+    public void testPostNoRedirect() throws Exception {
         int port = this.localServer.getServicePort();
         String host = "localhost";
         this.localServer.register("*", new BasicRedirectService(host, port));
@@ -468,6 +468,32 @@ public class TestRedirects extends BasicServerTestBase {
             e.consumeContent();
         }
         
+        HttpRequest reqWrapper = (HttpRequest) context.getAttribute(
+                ExecutionContext.HTTP_REQUEST);
+
+        assertEquals(HttpStatus.SC_MOVED_TEMPORARILY, response.getStatusLine().getStatusCode());
+        assertEquals("/oldlocation/", reqWrapper.getRequestLine().getUri());
+        assertEquals("POST", reqWrapper.getRequestLine().getMethod());
+    }
+
+    public void testPostRedirectSeeOther() throws Exception {
+        int port = this.localServer.getServicePort();
+        String host = "localhost";
+        this.localServer.register("*", new BasicRedirectService(host, port, 
+                HttpStatus.SC_SEE_OTHER));
+
+        DefaultHttpClient client = new DefaultHttpClient(); 
+        HttpContext context = new BasicHttpContext();
+        
+        HttpPost httppost = new HttpPost("/oldlocation/");
+        httppost.setEntity(new StringEntity("stuff"));
+
+        HttpResponse response = client.execute(getServerHttp(), httppost, context);
+        HttpEntity e = response.getEntity();
+        if (e != null) {
+            e.consumeContent();
+        }
+        
         HttpRequest reqWrapper = (HttpRequest) context.getAttribute(
                 ExecutionContext.HTTP_REQUEST);
 

src/docbkx/httpagent.xml

@@ -172,9 +172,10 @@ httpclient.getConnectionManager().shutdown();
     <section>
         <title>Automcatic redirect handling</title>
         <para>HttpClient handles all types of redirects automatically, except those explicitly
-            prohibited by the HTTP specification as requiring user intervention. Redirects on
+            prohibited by the HTTP specification as requiring user intervention. <literal>See
-                <literal>POST</literal> and <literal>PUT</literal> requests are converted to
+                Other</literal> (status code 303) redirects on <literal>POST</literal> and
-                <literal>GET</literal> requests as required by the HTTP specification.</para>
+                <literal>PUT</literal> requests are converted to <literal>GET</literal> requests as
+            required by the HTTP specification.</para>
     </section>
     <section>
         <title>HTTP client and execution context</title>