Apache
/
httpcomponents-client
mirror of https://github.com/apache/httpcomponents-client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SEE OTHER redirect handling fix

This commit is contained in:
Oleg Kalnichevski 2023-11-19 11:14:34 +01:00
parent 3486b47452
commit 83d603c9d8
3 changed files with 63 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java
View File

@ -90,6 +90,7 @@ public final class AsyncRedirectExec implements AsyncExecChainHandler {
volatile URI redirectURI; volatile URI redirectURI;
volatile int maxRedirects; volatile int maxRedirects;
volatile int redirectCount; volatile int redirectCount;
volatile HttpRequest originalRequest;
volatile HttpRequest currentRequest; volatile HttpRequest currentRequest;
volatile AsyncEntityProducer currentEntityProducer; volatile AsyncEntityProducer currentEntityProducer;
volatile RedirectLocations redirectLocations; volatile RedirectLocations redirectLocations;
@ -150,7 +151,7 @@ public final class AsyncRedirectExec implements AsyncExecChainHandler {
redirectBuilder = BasicRequestBuilder.get(); redirectBuilder = BasicRequestBuilder.get();
state.currentEntityProducer = null; state.currentEntityProducer = null;
} else { } else {
redirectBuilder = BasicRequestBuilder.copy(scope.originalRequest); redirectBuilder = BasicRequestBuilder.copy(state.originalRequest);
} }
break; break;
case HttpStatus.SC_SEE_OTHER: case HttpStatus.SC_SEE_OTHER:
@ -158,15 +159,16 @@ public final class AsyncRedirectExec implements AsyncExecChainHandler {
redirectBuilder = BasicRequestBuilder.get(); redirectBuilder = BasicRequestBuilder.get();
state.currentEntityProducer = null; state.currentEntityProducer = null;
} else { } else {
redirectBuilder = BasicRequestBuilder.copy(scope.originalRequest); redirectBuilder = BasicRequestBuilder.copy(state.originalRequest);
} }
break; break;
default: default:
redirectBuilder = BasicRequestBuilder.copy(scope.originalRequest); redirectBuilder = BasicRequestBuilder.copy(state.originalRequest);
} }
redirectBuilder.setUri(redirectUri); redirectBuilder.setUri(redirectUri);
state.reroute = false; state.reroute = false;
state.redirectURI = redirectUri; state.redirectURI = redirectUri;
state.originalRequest = redirectBuilder.build();
state.currentRequest = redirectBuilder.build(); state.currentRequest = redirectBuilder.build();
if (!Objects.equals(currentRoute.getTargetHost(), newTarget)) { if (!Objects.equals(currentRoute.getTargetHost(), newTarget)) {
@ -270,6 +272,7 @@ public final class AsyncRedirectExec implements AsyncExecChainHandler {
final State state = new State(); final State state = new State();
state.maxRedirects = config.getMaxRedirects() > 0 ? config.getMaxRedirects() : 50; state.maxRedirects = config.getMaxRedirects() > 0 ? config.getMaxRedirects() : 50;
state.redirectCount = 0; state.redirectCount = 0;
state.originalRequest = scope.originalRequest;
state.currentRequest = request; state.currentRequest = request;
state.currentEntityProducer = entityProducer; state.currentEntityProducer = entityProducer;
state.redirectLocations = redirectLocations; state.redirectLocations = redirectLocations;

8
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java
View File

@ -108,6 +108,7 @@ public final class RedirectExec implements ExecChainHandler {
final RequestConfig config = context.getRequestConfig(); final RequestConfig config = context.getRequestConfig();
final int maxRedirects = config.getMaxRedirects() > 0 ? config.getMaxRedirects() : 50; final int maxRedirects = config.getMaxRedirects() > 0 ? config.getMaxRedirects() : 50;
ClassicHttpRequest originalRequest = scope.originalRequest;
ClassicHttpRequest currentRequest = request; ClassicHttpRequest currentRequest = request;
ExecChain.Scope currentScope = scope; ExecChain.Scope currentScope = scope;
for (int redirectCount = 0;;) { for (int redirectCount = 0;;) {
@ -153,18 +154,18 @@ public final class RedirectExec implements ExecChainHandler {
if (Method.POST.isSame(request.getMethod())) { if (Method.POST.isSame(request.getMethod())) {
redirectBuilder = ClassicRequestBuilder.get(); redirectBuilder = ClassicRequestBuilder.get();
} else { } else {
redirectBuilder = ClassicRequestBuilder.copy(scope.originalRequest); redirectBuilder = ClassicRequestBuilder.copy(originalRequest);
} }
break; break;
case HttpStatus.SC_SEE_OTHER: case HttpStatus.SC_SEE_OTHER:
if (!Method.GET.isSame(request.getMethod()) && !Method.HEAD.isSame(request.getMethod())) { if (!Method.GET.isSame(request.getMethod()) && !Method.HEAD.isSame(request.getMethod())) {
redirectBuilder = ClassicRequestBuilder.get(); redirectBuilder = ClassicRequestBuilder.get();
} else { } else {
redirectBuilder = ClassicRequestBuilder.copy(scope.originalRequest); redirectBuilder = ClassicRequestBuilder.copy(originalRequest);
} }
break; break;
default: default:
redirectBuilder = ClassicRequestBuilder.copy(scope.originalRequest); redirectBuilder = ClassicRequestBuilder.copy(originalRequest);
} }
redirectBuilder.setUri(redirectUri); redirectBuilder.setUri(redirectUri);
@ -201,6 +202,7 @@ public final class RedirectExec implements ExecChainHandler {
if (LOG.isDebugEnabled()) { if (LOG.isDebugEnabled()) {
LOG.debug("{} redirecting to '{}' via {}", exchangeId, redirectUri, currentRoute); LOG.debug("{} redirecting to '{}' via {}", exchangeId, redirectUri, currentRoute);
} }
originalRequest = redirectBuilder.build();
currentRequest = redirectBuilder.build(); currentRequest = redirectBuilder.build();
RequestEntityProxy.enhance(currentRequest); RequestEntityProxy.enhance(currentRequest);

52
httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRedirectExec.java
View File

@ -56,6 +56,8 @@ import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.io.support.ClassicResponseBuilder;
import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
@ -357,6 +359,56 @@ public class TestRedirectExec {
Mockito.verify(response1).close(); Mockito.verify(response1).close();
} }
@Test
public void testPutSeeOtherRedirect() throws Exception {
final HttpRoute route = new HttpRoute(target);
final URI targetUri = new URI("http://localhost:80/stuff");
final ClassicHttpRequest request = ClassicRequestBuilder.put()
.setUri(targetUri)
.setEntity("stuff")
.build();
final HttpClientContext context = HttpClientContext.create();
final URI redirect1 = new URI("http://localhost:80/see-something-else");
final ClassicHttpResponse response1 = ClassicResponseBuilder.create(HttpStatus.SC_SEE_OTHER)
.addHeader(HttpHeaders.LOCATION, redirect1.toASCIIString())
.build();
final URI redirect2 = new URI("http://localhost:80/other-stuff");
final ClassicHttpResponse response2 = ClassicResponseBuilder.create(HttpStatus.SC_MOVED_PERMANENTLY)
.addHeader(HttpHeaders.LOCATION, redirect2.toASCIIString())
.build();
final ClassicHttpResponse response3 = ClassicResponseBuilder.create(HttpStatus.SC_OK)
.build();
Mockito.when(chain.proceed(
HttpRequestMatcher.matchesRequestUri(targetUri),
ArgumentMatchers.any())).thenReturn(response1);
Mockito.when(chain.proceed(
HttpRequestMatcher.matchesRequestUri(redirect1),
ArgumentMatchers.any())).thenReturn(response2);
Mockito.when(chain.proceed(
HttpRequestMatcher.matchesRequestUri(redirect2),
ArgumentMatchers.any())).thenReturn(response3);
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
final ClassicHttpResponse finalResponse = redirectExec.execute(request, scope, chain);
Assertions.assertEquals(200, finalResponse.getCode());
final ArgumentCaptor<ClassicHttpRequest> reqCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class);
Mockito.verify(chain, Mockito.times(3)).proceed(reqCaptor.capture(), ArgumentMatchers.same(scope));
final List<ClassicHttpRequest> allValues = reqCaptor.getAllValues();
Assertions.assertNotNull(allValues);
Assertions.assertEquals(3, allValues.size());
final ClassicHttpRequest request1 = allValues.get(0);
final ClassicHttpRequest request2 = allValues.get(1);
final ClassicHttpRequest request3 = allValues.get(2);
Assertions.assertSame(request, request1);
Assertions.assertEquals(request1.getMethod(), "PUT");
Assertions.assertEquals(request2.getMethod(), "GET");
Assertions.assertEquals(request3.getMethod(), "GET");
}
private static class HttpRequestMatcher implements ArgumentMatcher<ClassicHttpRequest> { private static class HttpRequestMatcher implements ArgumentMatcher<ClassicHttpRequest> {
private final URI expectedRequestUri; private final URI expectedRequestUri;