diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 2328854d5..59800bb02 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,9 @@ Changes since release 4.3 BETA1 ------------------- +* [HTTPCLIENT-1359] repeated requests using the same context fail if they redirect. + Contributed by James Leigh + * [HTTPCLIENT-1354] do not quote algorithm parameter in DIGEST auth response. Contributed by Oleg Kalnichevski diff --git a/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java b/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java index 0c75a18ac..25c640d7a 100644 --- a/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java +++ b/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java @@ -42,6 +42,7 @@ import org.apache.http.auth.AuthScheme; import org.apache.http.auth.AuthState; import org.apache.http.client.RedirectException; import org.apache.http.client.RedirectStrategy; +import org.apache.http.client.URICollection; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpExecutionAware; @@ -87,6 +88,11 @@ public class RedirectExec implements ClientExecChain { Args.notNull(request, "HTTP request"); Args.notNull(context, "HTTP context"); + final URICollection redirectLocations = context.getRedirectLocations(); + if (redirectLocations != null) { + redirectLocations.clear(); + } + final RequestConfig config = context.getRequestConfig(); final int maxRedirects = config.getMaxRedirects() > 0 ? config.getMaxRedirects() : 50; HttpRoute currentRoute = route; diff --git a/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java b/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java index b695293ee..5540f8df5 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java +++ b/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultRedirectStrategy.java @@ -272,26 +272,38 @@ public class TestDefaultRedirectStrategy { final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); final HttpClientContext context = HttpClientContext.create(); context.setAttribute(HttpCoreContext.HTTP_TARGET_HOST, new HttpHost("localhost")); - final HttpGet httpget = new HttpGet("http://localhost/"); final RequestConfig config = RequestConfig.custom().setCircularRedirectsAllowed(true).build(); context.setRequestConfig(config); - final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, + final URI uri1 = URI.create("http://localhost/stuff1"); + final URI uri2 = URI.create("http://localhost/stuff2"); + final URI uri3 = URI.create("http://localhost/stuff3"); + final HttpGet httpget1 = new HttpGet("http://localhost/"); + final HttpResponse response1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_MOVED_TEMPORARILY, "Redirect"); - response.addHeader("Location", "http://localhost/stuff"); - final 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)); + response1.addHeader("Location", uri1.toASCIIString()); + final HttpGet httpget2 = new HttpGet(uri1.toASCIIString()); + final HttpResponse response2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_MOVED_TEMPORARILY, "Redirect"); + response2.addHeader("Location", uri2.toASCIIString()); + final HttpGet httpget3 = new HttpGet(uri2.toASCIIString()); + final HttpResponse response3 = new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_MOVED_TEMPORARILY, "Redirect"); + response3.addHeader("Location", uri3.toASCIIString()); + Assert.assertEquals(uri1, redirectStrategy.getLocationURI(httpget1, response1, context)); + Assert.assertEquals(uri2, redirectStrategy.getLocationURI(httpget2, response2, context)); + Assert.assertEquals(uri3, redirectStrategy.getLocationURI(httpget3, response3, context)); final URICollection redirectLocations = context.getRedirectLocations(); Assert.assertNotNull(redirectLocations); - Assert.assertTrue(redirectLocations.contains(uri)); + Assert.assertTrue(redirectLocations.contains(uri1)); + Assert.assertTrue(redirectLocations.contains(uri2)); + Assert.assertTrue(redirectLocations.contains(uri3)); final List uris = redirectLocations.getAll(); Assert.assertNotNull(uris); Assert.assertEquals(3, uris.size()); - Assert.assertEquals(uri, uris.get(0)); - Assert.assertEquals(uri, uris.get(1)); - Assert.assertEquals(uri, uris.get(2)); + Assert.assertEquals(uri1, uris.get(0)); + Assert.assertEquals(uri2, uris.get(1)); + Assert.assertEquals(uri3, uris.get(2)); } @Test(expected=ProtocolException.class) @@ -299,7 +311,7 @@ public class TestDefaultRedirectStrategy { final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); final HttpClientContext context = HttpClientContext.create(); context.setAttribute(HttpCoreContext.HTTP_TARGET_HOST, new HttpHost("localhost")); - final HttpGet httpget = new HttpGet("http://localhost/"); + final HttpGet httpget = new HttpGet("http://localhost/stuff"); final RequestConfig config = RequestConfig.custom().setCircularRedirectsAllowed(false).build(); context.setRequestConfig(config); final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, diff --git a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java index cd75902fa..44180e672 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java +++ b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestRedirects.java @@ -182,6 +182,28 @@ public class TestRedirects extends IntegrationTestBase { } } + private static class RomeRedirectService implements HttpRequestHandler { + + public RomeRedirectService() { + super(); + } + + public void handle( + final HttpRequest request, + final HttpResponse response, + final HttpContext context) throws HttpException, IOException { + final String uri = request.getRequestLine().getUri(); + if (uri.equals("/rome")) { + response.setStatusCode(HttpStatus.SC_OK); + final StringEntity entity = new StringEntity("Successful redirect"); + response.setEntity(entity); + } else { + response.setStatusCode(HttpStatus.SC_MOVED_TEMPORARILY); + response.addHeader(new BasicHeader("Location", "/rome")); + } + } + } + private static class BogusRedirectService implements HttpRequestHandler { private final String url; @@ -426,6 +448,89 @@ public class TestRedirects extends IntegrationTestBase { } } + @Test + public void testRepeatRequest() throws Exception { + final HttpHost target = getServerHttp(); + this.localServer.register("*", new RomeRedirectService()); + + final HttpClientContext context = HttpClientContext.create(); + + final RequestConfig config = RequestConfig.custom().setRelativeRedirectsAllowed(true).build(); + final HttpGet first = new HttpGet("/rome"); + first.setConfig(config); + + EntityUtils.consume(this.httpclient.execute(target, first, context).getEntity()); + + final HttpGet second = new HttpGet("/rome"); + second.setConfig(config); + + final HttpResponse response = this.httpclient.execute(target, second, context); + EntityUtils.consume(response.getEntity()); + + final HttpRequest reqWrapper = context.getRequest(); + final HttpHost host = context.getTargetHost(); + + Assert.assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + Assert.assertEquals("/rome", reqWrapper.getRequestLine().getUri()); + Assert.assertEquals(host, target); + } + + @Test + public void testRepeatRequestRedirect() throws Exception { + final HttpHost target = getServerHttp(); + this.localServer.register("*", new RomeRedirectService()); + + final HttpClientContext context = HttpClientContext.create(); + + final RequestConfig config = RequestConfig.custom().setRelativeRedirectsAllowed(true).build(); + final HttpGet first = new HttpGet("/lille"); + first.setConfig(config); + + final HttpResponse response1 = this.httpclient.execute(target, first, context); + EntityUtils.consume(response1.getEntity()); + + final HttpGet second = new HttpGet("/lille"); + second.setConfig(config); + + final HttpResponse response2 = this.httpclient.execute(target, second, context); + EntityUtils.consume(response2.getEntity()); + + final HttpRequest reqWrapper = context.getRequest(); + final HttpHost host = context.getTargetHost(); + + Assert.assertEquals(HttpStatus.SC_OK, response2.getStatusLine().getStatusCode()); + Assert.assertEquals("/rome", reqWrapper.getRequestLine().getUri()); + Assert.assertEquals(host, target); + } + + @Test + public void testDifferentRequestSameRedirect() throws Exception { + final HttpHost target = getServerHttp(); + this.localServer.register("*", new RomeRedirectService()); + + final HttpClientContext context = HttpClientContext.create(); + + final RequestConfig config = RequestConfig.custom().setRelativeRedirectsAllowed(true).build(); + final HttpGet first = new HttpGet("/alian"); + first.setConfig(config); + + final HttpResponse response1 = this.httpclient.execute(target, first, context); + EntityUtils.consume(response1.getEntity()); + + final HttpGet second = new HttpGet("/lille"); + second.setConfig(config); + + final HttpResponse response2 = this.httpclient.execute(target, second, context); + EntityUtils.consume(response2.getEntity()); + + final HttpRequest reqWrapper = context.getRequest(); + final HttpHost host = context.getTargetHost(); + + Assert.assertEquals(HttpStatus.SC_OK, response2.getStatusLine().getStatusCode()); + Assert.assertEquals("/rome", reqWrapper.getRequestLine().getUri()); + Assert.assertEquals(host, target); + } + @Test public void testPostNoRedirect() throws Exception { final HttpHost target = getServerHttp();