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 2012/10/10 22:51:30 UTC
svn commit: r1396793 - in /httpcomponents/httpclient/trunk: RELEASE_NOTES.txt
httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java
httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java
Author: olegk
Date: Wed Oct 10 20:51:30 2012
New Revision: 1396793
URL: http://svn.apache.org/viewvc?rev=1396793&view=rev
Log:
HTTPCLIENT-1248: Default and lax redirect strategies should not convert requests redirected with 307 status to GET method
Modified:
httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1396793&r1=1396792&r2=1396793&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Wed Oct 10 20:51:30 2012
@@ -1,6 +1,10 @@
Changes since 4.2.1
-------------------
+* [HTTPCLIENT-1248]: Default and lax redirect strategies should not convert requests redirected
+ with 307 status to GET method.
+ Contributed by Oleg Kalnichevski <olegk at apache.org>
+
* [HTTPCLIENT-1215] BasicAuthCache does not take default ports into consideration when
looking up cached authentication details by HttpHost key.
Contributed by Oleg Kalnichevski <olegk at apache.org>
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=1396793&r1=1396792&r2=1396793&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 Wed Oct 10 20:51:30 2012
@@ -35,6 +35,7 @@ import org.apache.http.annotation.Immuta
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.Header;
+import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
@@ -42,8 +43,15 @@ import org.apache.http.HttpStatus;
import org.apache.http.ProtocolException;
import org.apache.http.client.CircularRedirectException;
import org.apache.http.client.RedirectStrategy;
+import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpOptions;
+import org.apache.http.client.methods.HttpPatch;
+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.params.ClientPNames;
import org.apache.http.client.utils.URIUtils;
@@ -210,9 +218,35 @@ public class DefaultRedirectStrategy imp
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 {
+ int status = response.getStatusLine().getStatusCode();
+ if (status == HttpStatus.SC_TEMPORARY_REDIRECT) {
+ if (method.equalsIgnoreCase(HttpPost.METHOD_NAME)) {
+ return copyEntity(new HttpPost(uri), request);
+ } else if (method.equalsIgnoreCase(HttpPut.METHOD_NAME)) {
+ return copyEntity(new HttpPut(uri), request);
+ } else if (method.equalsIgnoreCase(HttpDelete.METHOD_NAME)) {
+ return new HttpDelete(uri);
+ } else if (method.equalsIgnoreCase(HttpTrace.METHOD_NAME)) {
+ return new HttpTrace(uri);
+ } else if (method.equalsIgnoreCase(HttpOptions.METHOD_NAME)) {
+ return new HttpOptions(uri);
+ } else if (method.equalsIgnoreCase(HttpPatch.METHOD_NAME)) {
+ return copyEntity(new HttpPatch(uri), request);
+ }
+ }
return new HttpGet(uri);
}
}
+ private HttpUriRequest copyEntity(
+ final HttpEntityEnclosingRequestBase redirect, final HttpRequest original) {
+ if (original instanceof HttpEntityEnclosingRequest) {
+ redirect.setEntity(((HttpEntityEnclosingRequest) original).getEntity());
+ }
+ return redirect;
+ }
+
}
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=1396793&r1=1396792&r2=1396793&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 Wed Oct 10 20:51:30 2012
@@ -28,6 +28,8 @@ package org.apache.http.impl.client;
import java.net.URI;
import java.util.List;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
@@ -38,8 +40,10 @@ import org.apache.http.client.methods.Ht
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.params.ClientPNames;
+import org.apache.http.entity.BasicHttpEntity;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.protocol.BasicHttpContext;
import org.apache.http.protocol.ExecutionContext;
@@ -48,10 +52,10 @@ import org.junit.Assert;
import org.junit.Test;
public class TestDefaultRedirectStrategy {
-
+
@Test
public void testIsRedirectable() {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ 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));
@@ -61,10 +65,10 @@ public class TestDefaultRedirectStrategy
@Test
public void testIsRedirectedMovedTemporary() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/stuff");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
@@ -74,20 +78,20 @@ public class TestDefaultRedirectStrategy
@Test
public void testIsRedirectedMovedTemporaryNoLocation() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
Assert.assertFalse(redirectStrategy.isRedirected(httpget, response, context));
}
@Test
public void testIsRedirectedMovedPermanently() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_PERMANENTLY, "Redirect");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
HttpPost httppost = new HttpPost("http://localhost/");
@@ -96,10 +100,10 @@ public class TestDefaultRedirectStrategy
@Test
public void testIsRedirectedTemporaryRedirect() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_TEMPORARY_REDIRECT, "Redirect");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
HttpPost httppost = new HttpPost("http://localhost/");
@@ -108,10 +112,10 @@ public class TestDefaultRedirectStrategy
@Test
public void testIsRedirectedSeeOther() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_SEE_OTHER, "Redirect");
Assert.assertTrue(redirectStrategy.isRedirected(httpget, response, context));
HttpPost httppost = new HttpPost("http://localhost/");
@@ -120,7 +124,7 @@ public class TestDefaultRedirectStrategy
@Test
public void testIsRedirectedUnknownStatus() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 333, "Redirect");
@@ -131,10 +135,10 @@ public class TestDefaultRedirectStrategy
@Test
public void testIsRedirectedInvalidInput() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_SEE_OTHER, "Redirect");
try {
redirectStrategy.isRedirected(null, response, context);
@@ -150,10 +154,10 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUri() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/stuff");
URI uri = redirectStrategy.getLocationURI(httpget, response, context);
@@ -162,20 +166,20 @@ public class TestDefaultRedirectStrategy
@Test(expected=ProtocolException.class)
public void testGetLocationUriMissingHeader() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
redirectStrategy.getLocationURI(httpget, response, context);
}
@Test(expected=ProtocolException.class)
public void testGetLocationUriInvalidLocation() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/not valid");
redirectStrategy.getLocationURI(httpget, response, context);
@@ -183,11 +187,11 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUriRelative() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "/stuff");
URI uri = redirectStrategy.getLocationURI(httpget, response, context);
@@ -196,10 +200,10 @@ public class TestDefaultRedirectStrategy
@Test(expected=IllegalStateException.class)
public void testGetLocationUriRelativeMissingTargetHost() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "/stuff");
URI uri = redirectStrategy.getLocationURI(httpget, response, context);
@@ -208,11 +212,11 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUriRelativeWithFragment() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "/stuff#fragment");
URI uri = redirectStrategy.getLocationURI(httpget, response, context);
@@ -221,11 +225,11 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUriAbsoluteWithFragment() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/stuff#fragment");
URI uri = redirectStrategy.getLocationURI(httpget, response, context);
@@ -234,11 +238,11 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUriNormalized() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/././stuff/../morestuff");
URI uri = redirectStrategy.getLocationURI(httpget, response, context);
@@ -247,12 +251,12 @@ public class TestDefaultRedirectStrategy
@Test(expected=ProtocolException.class)
public void testGetLocationUriRelativeLocationNotAllowed() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
httpget.getParams().setParameter(ClientPNames.REJECT_RELATIVE_REDIRECT, Boolean.TRUE);
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "/stuff");
redirectStrategy.getLocationURI(httpget, response, context);
@@ -260,19 +264,19 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUriAllowCircularRedirects() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
httpget.getParams().setParameter(ClientPNames.ALLOW_CIRCULAR_REDIRECTS, Boolean.TRUE);
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/stuff");
URI uri = URI.create("http://localhost/stuff");
Assert.assertEquals(uri, redirectStrategy.getLocationURI(httpget, response, context));
Assert.assertEquals(uri, redirectStrategy.getLocationURI(httpget, response, context));
Assert.assertEquals(uri, redirectStrategy.getLocationURI(httpget, response, context));
-
+
RedirectLocations redirectLocations = (RedirectLocations) context.getAttribute(
DefaultRedirectStrategy.REDIRECT_LOCATIONS);
Assert.assertNotNull(redirectLocations);
@@ -287,12 +291,12 @@ public class TestDefaultRedirectStrategy
@Test(expected=ProtocolException.class)
public void testGetLocationUriDisallowCircularRedirects() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, new HttpHost("localhost"));
HttpGet httpget = new HttpGet("http://localhost/");
httpget.getParams().setParameter(ClientPNames.ALLOW_CIRCULAR_REDIRECTS, Boolean.FALSE);
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/stuff");
URI uri = URI.create("http://localhost/stuff");
@@ -302,10 +306,10 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetLocationUriInvalidInput() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
HttpContext context = new BasicHttpContext();
HttpGet httpget = new HttpGet("http://localhost/");
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_MOVED_TEMPORARILY, "Redirect");
response.addHeader("Location", "http://localhost/stuff");
try {
@@ -327,8 +331,8 @@ public class TestDefaultRedirectStrategy
@Test
public void testGetRedirectRequest() throws Exception {
- DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
- HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
HttpStatus.SC_SEE_OTHER, "Redirect");
response.addHeader("Location", "http://localhost/stuff");
HttpContext context1 = new BasicHttpContext();
@@ -345,4 +349,25 @@ public class TestDefaultRedirectStrategy
Assert.assertEquals("HEAD", redirect3.getMethod());
}
+ @Test
+ public void testGetRedirectRequestForTemporaryRedirect() throws Exception {
+ DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+ HttpStatus.SC_TEMPORARY_REDIRECT, "Temporary Redirect");
+ response.addHeader("Location", "http://localhost/stuff");
+ HttpContext context1 = new BasicHttpContext();
+ HttpUriRequest redirect1 = redirectStrategy.getRedirect(
+ new HttpTrace("http://localhost/"), response, context1);
+ Assert.assertEquals("TRACE", redirect1.getMethod());
+ HttpContext context2 = new BasicHttpContext();
+ HttpPost httppost = new HttpPost("http://localhost/");
+ HttpEntity entity = new BasicHttpEntity();
+ httppost.setEntity(entity);
+ HttpUriRequest redirect2 = redirectStrategy.getRedirect(
+ httppost, response, context2);
+ Assert.assertEquals("POST", redirect2.getMethod());
+ Assert.assertTrue(redirect2 instanceof HttpEntityEnclosingRequest);
+ Assert.assertSame(entity, ((HttpEntityEnclosingRequest) redirect2).getEntity());
+ }
+
}
Re: svn commit: r1396793
Posted by sebb <se...@gmail.com>.
On 12 October 2012 20:24, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Fri, 2012-10-12 at 03:09 +0100, sebb wrote:
>> On 11 October 2012 15:47, Oleg Kalnichevski <ol...@apache.org> wrote:
>> > On Wed, 2012-10-10 at 22:11 +0100, sebb wrote:
>> >> On 10 October 2012 21:51, <ol...@apache.org> wrote:
>> >> > Author: olegk
>> >> > Date: Wed Oct 10 20:51:30 2012
>> >> > New Revision: 1396793
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1396793&view=rev
>> >> > Log:
>> >> > HTTPCLIENT-1248: Default and lax redirect strategies should not convert requests redirected with 307 status to GET method
>> >> >
>> >
>> > ...
>> >
>> >> The above code converts unknown methods into GET.
>> >> If a new method is added, that will be incorrect.
>> >>
>> >> It might be useful to know when this is happening; maybe an assert
>> >> would be useful here?
>> >> Or a log message?
>> >>
>> >> Or is it possible to write a clone/copy method that works for all
>> >> possible methods?
>> >> Maybe methods should know how to clone themselves?
>> >>
>> >
>> > This is a good point. I reworked the method to make use of the new
>> > RequestBuilder class to create a mutable copy of the original request
>> > prior to setting the request URI.
>> >
>> > http://svn.apache.org/viewvc?rev=1397085&view=rev
>> >
>> > Please review
>>
>> Looks much better and less fragile.
>>
>> Not quite sure why getMethod should default to POST/GET if the method
>> is null - why should the method ever be null? Isn't that a bug?
>>
>
> I think these are most commonly used methods and as such are reasonable
> defaults if someone explicitly sets request method to null. I believe it
> is should be ok to default to POSt for entity closing requests and GET
> otherwise.
>
Well, yes, if a null method is allowed then the defaults are fine.
But I question whether one should allow a null method in the first place.
Seems to me that could hide bugs, and is a bit confusing unless the
default is universally applied.
I think it would be safer to throw NPE or IAE.
>> Also, method names are case-significant, so should probably not upcase.
>>
>
> Good catch. Corrected.
Thanks - it was only recently I saw the comment on the Tomcat list.
> Oleg
>
>> Per the Tomcat issues list:
>>
>> >>
>> RFCs 2616, 2068, and 1945 all agree that method name is case-sensitive:
>> http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.1
>> <<
>> > Oleg
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>> > For additional commands, e-mail: dev-help@hc.apache.org
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>> For additional commands, e-mail: dev-help@hc.apache.org
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1396793
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Fri, 2012-10-12 at 03:09 +0100, sebb wrote:
> On 11 October 2012 15:47, Oleg Kalnichevski <ol...@apache.org> wrote:
> > On Wed, 2012-10-10 at 22:11 +0100, sebb wrote:
> >> On 10 October 2012 21:51, <ol...@apache.org> wrote:
> >> > Author: olegk
> >> > Date: Wed Oct 10 20:51:30 2012
> >> > New Revision: 1396793
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1396793&view=rev
> >> > Log:
> >> > HTTPCLIENT-1248: Default and lax redirect strategies should not convert requests redirected with 307 status to GET method
> >> >
> >
> > ...
> >
> >> The above code converts unknown methods into GET.
> >> If a new method is added, that will be incorrect.
> >>
> >> It might be useful to know when this is happening; maybe an assert
> >> would be useful here?
> >> Or a log message?
> >>
> >> Or is it possible to write a clone/copy method that works for all
> >> possible methods?
> >> Maybe methods should know how to clone themselves?
> >>
> >
> > This is a good point. I reworked the method to make use of the new
> > RequestBuilder class to create a mutable copy of the original request
> > prior to setting the request URI.
> >
> > http://svn.apache.org/viewvc?rev=1397085&view=rev
> >
> > Please review
>
> Looks much better and less fragile.
>
> Not quite sure why getMethod should default to POST/GET if the method
> is null - why should the method ever be null? Isn't that a bug?
>
I think these are most commonly used methods and as such are reasonable
defaults if someone explicitly sets request method to null. I believe it
is should be ok to default to POSt for entity closing requests and GET
otherwise.
> Also, method names are case-significant, so should probably not upcase.
>
Good catch. Corrected.
Oleg
> Per the Tomcat issues list:
>
> >>
> RFCs 2616, 2068, and 1945 all agree that method name is case-sensitive:
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.1
> <<
> > Oleg
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> > For additional commands, e-mail: dev-help@hc.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1396793
Posted by sebb <se...@gmail.com>.
On 11 October 2012 15:47, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Wed, 2012-10-10 at 22:11 +0100, sebb wrote:
>> On 10 October 2012 21:51, <ol...@apache.org> wrote:
>> > Author: olegk
>> > Date: Wed Oct 10 20:51:30 2012
>> > New Revision: 1396793
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1396793&view=rev
>> > Log:
>> > HTTPCLIENT-1248: Default and lax redirect strategies should not convert requests redirected with 307 status to GET method
>> >
>
> ...
>
>> The above code converts unknown methods into GET.
>> If a new method is added, that will be incorrect.
>>
>> It might be useful to know when this is happening; maybe an assert
>> would be useful here?
>> Or a log message?
>>
>> Or is it possible to write a clone/copy method that works for all
>> possible methods?
>> Maybe methods should know how to clone themselves?
>>
>
> This is a good point. I reworked the method to make use of the new
> RequestBuilder class to create a mutable copy of the original request
> prior to setting the request URI.
>
> http://svn.apache.org/viewvc?rev=1397085&view=rev
>
> Please review
Looks much better and less fragile.
Not quite sure why getMethod should default to POST/GET if the method
is null - why should the method ever be null? Isn't that a bug?
Also, method names are case-significant, so should probably not upcase.
Per the Tomcat issues list:
>>
RFCs 2616, 2068, and 1945 all agree that method name is case-sensitive:
http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.1
<<
> Oleg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1396793
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Wed, 2012-10-10 at 22:11 +0100, sebb wrote:
> On 10 October 2012 21:51, <ol...@apache.org> wrote:
> > Author: olegk
> > Date: Wed Oct 10 20:51:30 2012
> > New Revision: 1396793
> >
> > URL: http://svn.apache.org/viewvc?rev=1396793&view=rev
> > Log:
> > HTTPCLIENT-1248: Default and lax redirect strategies should not convert requests redirected with 307 status to GET method
> >
...
> The above code converts unknown methods into GET.
> If a new method is added, that will be incorrect.
>
> It might be useful to know when this is happening; maybe an assert
> would be useful here?
> Or a log message?
>
> Or is it possible to write a clone/copy method that works for all
> possible methods?
> Maybe methods should know how to clone themselves?
>
This is a good point. I reworked the method to make use of the new
RequestBuilder class to create a mutable copy of the original request
prior to setting the request URI.
http://svn.apache.org/viewvc?rev=1397085&view=rev
Please review
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1396793 - in /httpcomponents/httpclient/trunk:
RELEASE_NOTES.txt httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java
httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java
Posted by sebb <se...@gmail.com>.
On 10 October 2012 21:51, <ol...@apache.org> wrote:
> Author: olegk
> Date: Wed Oct 10 20:51:30 2012
> New Revision: 1396793
>
> URL: http://svn.apache.org/viewvc?rev=1396793&view=rev
> Log:
> HTTPCLIENT-1248: Default and lax redirect strategies should not convert requests redirected with 307 status to GET method
>
> Modified:
> httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
> httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRedirectStrategy.java
> httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java
>
> Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1396793&r1=1396792&r2=1396793&view=diff
> ==============================================================================
> --- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
> +++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Wed Oct 10 20:51:30 2012
> @@ -1,6 +1,10 @@
> Changes since 4.2.1
> -------------------
>
> +* [HTTPCLIENT-1248]: Default and lax redirect strategies should not convert requests redirected
> + with 307 status to GET method.
> + Contributed by Oleg Kalnichevski <olegk at apache.org>
> +
> * [HTTPCLIENT-1215] BasicAuthCache does not take default ports into consideration when
> looking up cached authentication details by HttpHost key.
> Contributed by Oleg Kalnichevski <olegk at apache.org>
>
> 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=1396793&r1=1396792&r2=1396793&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 Wed Oct 10 20:51:30 2012
> @@ -35,6 +35,7 @@ import org.apache.http.annotation.Immuta
> import org.apache.commons.logging.Log;
> import org.apache.commons.logging.LogFactory;
> import org.apache.http.Header;
> +import org.apache.http.HttpEntityEnclosingRequest;
> import org.apache.http.HttpHost;
> import org.apache.http.HttpRequest;
> import org.apache.http.HttpResponse;
> @@ -42,8 +43,15 @@ import org.apache.http.HttpStatus;
> import org.apache.http.ProtocolException;
> import org.apache.http.client.CircularRedirectException;
> import org.apache.http.client.RedirectStrategy;
> +import org.apache.http.client.methods.HttpDelete;
> +import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
> import org.apache.http.client.methods.HttpGet;
> import org.apache.http.client.methods.HttpHead;
> +import org.apache.http.client.methods.HttpOptions;
> +import org.apache.http.client.methods.HttpPatch;
> +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.params.ClientPNames;
> import org.apache.http.client.utils.URIUtils;
> @@ -210,9 +218,35 @@ public class DefaultRedirectStrategy imp
> 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 {
> + int status = response.getStatusLine().getStatusCode();
> + if (status == HttpStatus.SC_TEMPORARY_REDIRECT) {
> + if (method.equalsIgnoreCase(HttpPost.METHOD_NAME)) {
> + return copyEntity(new HttpPost(uri), request);
> + } else if (method.equalsIgnoreCase(HttpPut.METHOD_NAME)) {
> + return copyEntity(new HttpPut(uri), request);
> + } else if (method.equalsIgnoreCase(HttpDelete.METHOD_NAME)) {
> + return new HttpDelete(uri);
> + } else if (method.equalsIgnoreCase(HttpTrace.METHOD_NAME)) {
> + return new HttpTrace(uri);
> + } else if (method.equalsIgnoreCase(HttpOptions.METHOD_NAME)) {
> + return new HttpOptions(uri);
> + } else if (method.equalsIgnoreCase(HttpPatch.METHOD_NAME)) {
> + return copyEntity(new HttpPatch(uri), request);
> + }
> + }
> return new HttpGet(uri);
The above code converts unknown methods into GET.
If a new method is added, that will be incorrect.
It might be useful to know when this is happening; maybe an assert
would be useful here?
Or a log message?
Or is it possible to write a clone/copy method that works for all
possible methods?
Maybe methods should know how to clone themselves?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org