From 98b9d436f871eacdf33846686bb5af58546ff569 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 21 Feb 2020 20:29:17 +0100 Subject: [PATCH] HTTPCLIENT-2052: Fixed redirection of entity enclosing requests with non-repeatable entities; additional integration tests for redirects of entity enclosing methods --- .../http/impl/execchain/RedirectExec.java | 7 +- .../client/integration/TestRedirects.java | 164 ++++++++++++++++++ 2 files changed, 170 insertions(+), 1 deletion(-) 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 2deb75444..3a5673da8 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 @@ -112,7 +112,12 @@ public class RedirectExec implements ClientExecChain { try { if (config.isRedirectsEnabled() && this.redirectStrategy.isRedirected(currentRequest.getOriginal(), response, context)) { - + if (!RequestEntityProxy.isRepeatable(currentRequest)) { + if (log.isDebugEnabled()) { + log.debug("Cannot redirect non-repeatable request"); + } + return response; + } if (redirectCount >= maxRedirects) { throw new RedirectException("Maximum redirects ("+ maxRedirects + ") exceeded"); } 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 b6ade302d..72f9659c1 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 @@ -26,11 +26,13 @@ */ package org.apache.http.impl.client.integration; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.net.URI; import java.util.Arrays; import java.util.List; +import org.apache.http.Consts; import org.apache.http.Header; import org.apache.http.HttpException; import org.apache.http.HttpHost; @@ -49,14 +51,18 @@ import org.apache.http.client.methods.HttpPost; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.client.utils.URIUtils; import org.apache.http.cookie.SM; +import org.apache.http.entity.InputStreamEntity; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.BasicCookieStore; +import org.apache.http.impl.client.LaxRedirectStrategy; import org.apache.http.impl.cookie.BasicClientCookie; +import org.apache.http.localserver.EchoHandler; import org.apache.http.localserver.LocalServerTestBase; import org.apache.http.message.BasicHeader; import org.apache.http.protocol.HTTP; import org.apache.http.protocol.HttpContext; import org.apache.http.protocol.HttpCoreContext; +import org.apache.http.protocol.HttpExpectationVerifier; import org.apache.http.protocol.HttpRequestHandler; import org.apache.http.protocol.UriHttpRequestHandlerMapper; import org.apache.http.util.EntityUtils; @@ -105,6 +111,37 @@ public class TestRedirects extends LocalServerTestBase { } + private static class RedirectExpectationVerifier implements HttpExpectationVerifier { + + private final int statuscode; + + public RedirectExpectationVerifier(final int statuscode) { + super(); + this.statuscode = statuscode > 0 ? statuscode : HttpStatus.SC_MOVED_TEMPORARILY; + } + + public RedirectExpectationVerifier() { + this(-1); + } + + @Override + public void verify( + final HttpRequest request, final HttpResponse response, final HttpContext context) throws HttpException { + final HttpInetConnection conn = (HttpInetConnection) context.getAttribute(HttpCoreContext.HTTP_CONNECTION); + final int port = conn.getLocalPort(); + final String uri = request.getRequestLine().getUri(); + if (uri.equals("/oldlocation/")) { + response.setStatusCode(this.statuscode); + response.addHeader(new BasicHeader("Location", + "http://localhost:" + port + "/newlocation/")); + response.addHeader(new BasicHeader("Connection", "close")); + } else { + response.setStatusCode(HttpStatus.SC_CONTINUE); + } + } + + } + private static class CircularRedirectService implements HttpRequestHandler { public CircularRedirectService() { @@ -586,6 +623,133 @@ public class TestRedirects extends LocalServerTestBase { Assert.assertEquals("GET", reqWrapper.getRequestLine().getMethod()); } + @Test + public void testPostRedirectMovedPermatently() throws Exception { + this.serverBootstrap.registerHandler("*", new BasicRedirectService(HttpStatus.SC_MOVED_PERMANENTLY)); + + 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()); + Assert.assertEquals(HttpStatus.SC_MOVED_PERMANENTLY, response.getStatusLine().getStatusCode()); + } + + @Test + public void testPostRedirectMovedPermatentlyLaxStrategy() throws Exception { + this.serverBootstrap.registerHandler("*", new BasicRedirectService(HttpStatus.SC_MOVED_PERMANENTLY)); + this.clientBuilder.setRedirectStrategy(LaxRedirectStrategy.INSTANCE); + + 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_OK, response.getStatusLine().getStatusCode()); + Assert.assertEquals("/newlocation/", reqWrapper.getRequestLine().getUri()); + Assert.assertEquals("GET", reqWrapper.getRequestLine().getMethod()); + } + + @Test + public void testPostTemporaryRedirectLaxStrategy() throws Exception { + this.serverBootstrap.registerHandler("*", new BasicRedirectService(HttpStatus.SC_TEMPORARY_REDIRECT)); + this.clientBuilder.setRedirectStrategy(LaxRedirectStrategy.INSTANCE); + + 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_OK, response.getStatusLine().getStatusCode()); + Assert.assertEquals("/newlocation/", reqWrapper.getRequestLine().getUri()); + Assert.assertEquals("POST", reqWrapper.getRequestLine().getMethod()); + } + + @Test + public void testNonRepeatablePostTemporaryRedirectLaxStrategy() throws Exception { + this.serverBootstrap.registerHandler("*", new BasicRedirectService(HttpStatus.SC_TEMPORARY_REDIRECT)); + this.clientBuilder.setRedirectStrategy(LaxRedirectStrategy.INSTANCE); + + final HttpHost target = start(); + + final HttpClientContext context = HttpClientContext.create(); + + final HttpPost httppost = new HttpPost("/oldlocation/"); + final byte[] content = "stuff".getBytes(Consts.ASCII); + httppost.setEntity(new InputStreamEntity(new ByteArrayInputStream(content), content.length)); + + final HttpResponse response = this.httpclient.execute(target, httppost, context); + EntityUtils.consume(response.getEntity()); + Assert.assertEquals(HttpStatus.SC_TEMPORARY_REDIRECT, response.getStatusLine().getStatusCode()); + } + + @Test + public void testNonRepeatablePostTemporaryWithExpectContinueRedirectLaxStrategy() throws Exception { + this.serverBootstrap.registerHandler("*", new BasicRedirectService(HttpStatus.SC_TEMPORARY_REDIRECT)); + this.clientBuilder.setRedirectStrategy(LaxRedirectStrategy.INSTANCE); + + final HttpHost target = start(); + + final HttpClientContext context = HttpClientContext.create(); + context.setRequestConfig(RequestConfig.custom() + .setExpectContinueEnabled(true) + .build()); + + final HttpPost httppost = new HttpPost("/oldlocation/"); + final byte[] content = "stuff".getBytes(Consts.ASCII); + httppost.setEntity(new InputStreamEntity(new ByteArrayInputStream(content), content.length)); + + final HttpResponse response = this.httpclient.execute(target, httppost, context); + EntityUtils.consume(response.getEntity()); + Assert.assertEquals(HttpStatus.SC_TEMPORARY_REDIRECT, response.getStatusLine().getStatusCode()); + } + + @Test + public void testNonRepeatablePostTemporaryWithExpectContinueExpectVerifierRedirectLaxStrategy() throws Exception { + this.serverBootstrap.registerHandler("*", new EchoHandler()); + this.serverBootstrap.setExpectationVerifier(new RedirectExpectationVerifier(HttpStatus.SC_TEMPORARY_REDIRECT)); + + this.clientBuilder.setRedirectStrategy(LaxRedirectStrategy.INSTANCE); + + final HttpHost target = start(); + + final HttpClientContext context = HttpClientContext.create(); + context.setRequestConfig(RequestConfig.custom() + .setExpectContinueEnabled(true) + .build()); + + final HttpPost httppost = new HttpPost("/oldlocation/"); + final byte[] content = "stuff".getBytes(Consts.ASCII); + httppost.setEntity(new InputStreamEntity(new ByteArrayInputStream(content), content.length)); + + final HttpResponse response = this.httpclient.execute(target, httppost, context); + EntityUtils.consume(response.getEntity()); + + final HttpRequest reqWrapper = context.getRequest(); + + Assert.assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + Assert.assertEquals("/newlocation/", reqWrapper.getRequestLine().getUri()); + Assert.assertEquals("POST", reqWrapper.getRequestLine().getMethod()); + } + @Test public void testRelativeRedirect() throws Exception { this.serverBootstrap.registerHandler("*", new RelativeRedirectService());