diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java index 15be8f914b..44bae7ab15 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java @@ -42,6 +42,7 @@ import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.client5.http.protocol.RedirectStrategy; import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.testing.SSLTestContexts; import org.apache.hc.core5.http.Header; @@ -167,6 +168,12 @@ public TestClientBuilder setUnixDomainSocket(final Path unixDomainSocket) { return this; } + @Override + public TestClientBuilder setRedirectStrategy(final RedirectStrategy redirectStrategy) { + this.clientBuilder.setRedirectStrategy(redirectStrategy); + return this; + } + @Override public TestClient build() throws Exception { final HttpClientConnectionManager connectionManagerCopy = connectionManager != null ? connectionManager : diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java index 775ff47c4b..f84d15de35 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java @@ -37,6 +37,7 @@ import org.apache.hc.client5.http.auth.CredentialsProvider; import org.apache.hc.client5.http.classic.ExecChainHandler; import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.client5.http.protocol.RedirectStrategy; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpRequestInterceptor; import org.apache.hc.core5.http.HttpResponseInterceptor; @@ -108,6 +109,10 @@ default TestClientBuilder setUnixDomainSocket(Path unixDomainSocket) { throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } + default TestClientBuilder setRedirectStrategy(RedirectStrategy redirectStrategy) { + throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); + } + TestClient build() throws Exception; } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java index 25c9e7f413..1371dd8d0b 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java @@ -50,6 +50,7 @@ import org.apache.hc.client5.http.cookie.BasicCookieStore; import org.apache.hc.client5.http.cookie.CookieStore; import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; +import org.apache.hc.client5.http.impl.LaxRedirectStrategy; import org.apache.hc.client5.http.impl.cookie.BasicClientCookie; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.protocol.RedirectLocations; @@ -761,4 +762,57 @@ void testCrossSiteRedirectWithSensitiveHeaders(final String headerName) throws E } } + @ParameterizedTest(name = "{displayName}; manually added header: {0}") + @ValueSource(strings = {HttpHeaders.AUTHORIZATION, HttpHeaders.COOKIE}) + void testCrossSiteRedirectWithSensitiveHeadersAndLaxRedirectStrategy(final String headerName) throws Exception { + configureClient(builder -> builder + .setRedirectStrategy(new LaxRedirectStrategy()) + ); + final URIScheme scheme = scheme(); + final TestServer secondServer = new TestServerBootstrap(scheme()) + .register("/random/*", new RandomHandler()) + .build(); + try { + final InetSocketAddress address2 = secondServer.start(); + + final HttpHost redirectTarget = new HttpHost(scheme.name(), "localhost", address2.getPort()); + + configureServer(bootstrap -> bootstrap + .setExchangeHandlerDecorator(requestHandler -> new RedirectingDecorator( + requestHandler, + requestUri -> { + final URI location = new URIBuilder(requestUri) + .setHttpHost(redirectTarget) + .setPath("/random/100") + .build(); + return new Redirect(HttpStatus.SC_MOVED_TEMPORARILY, location.toString()); + })) + .register("/random/*", new RandomHandler()) + ); + + final HttpHost target = startServer(); + + final TestClient client = client(); + final HttpClientContext context = HttpClientContext.create(); + + client.execute(target, + ClassicRequestBuilder.get() + .setHttpHost(target) + .setPath("/oldlocation") + .setHeader(headerName, "custom header") + .build(), + context, + response -> { + Assertions.assertEquals(HttpStatus.SC_OK, response.getCode()); + EntityUtils.consume(response.getEntity()); + return null; + }); + final RedirectLocations redirects = context.getRedirectLocations(); + Assertions.assertNotNull(redirects); + Assertions.assertEquals(1, redirects.size()); + } finally { + secondServer.shutdown(CloseMode.GRACEFUL); + } + } + } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/LaxRedirectStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/LaxRedirectStrategy.java index 49fbf7abee..76f1089932 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/LaxRedirectStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/LaxRedirectStrategy.java @@ -36,6 +36,7 @@ import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; @@ -84,6 +85,16 @@ public boolean isRedirected(final HttpRequest request, final HttpResponse respon } } + @Override + public boolean isRedirectAllowed( + final HttpHost currentTarget, + final HttpHost newTarget, + final HttpRequest redirect, + final HttpContext context) { + // Always allow, regardless of sensitive headers + return true; + } + protected boolean isRedirectable(final String method) { for (final String m : REDIRECT_METHODS) { if (m.equalsIgnoreCase(method)) { diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java index ccbfc916f7..a874b42682 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java @@ -261,13 +261,6 @@ void testRedirectAllowed() throws Exception { BasicRequestBuilder.get("/").build(), null)); - Assertions.assertTrue(redirectStrategy.isRedirectAllowed( - new HttpHost("somehost", 1234), - new HttpHost("somehost", 1234), - BasicRequestBuilder.get("/") - .build(), - null)); - Assertions.assertTrue(redirectStrategy.isRedirectAllowed( new HttpHost("somehost", 1234), new HttpHost("somehost", 1234), diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestLaxRedirectStrategy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestLaxRedirectStrategy.java index 1df2d0e4c0..b86847b975 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestLaxRedirectStrategy.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestLaxRedirectStrategy.java @@ -32,13 +32,18 @@ import org.apache.hc.client5.http.classic.methods.HttpHead; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.classic.methods.HttpPut; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.http.support.BasicRequestBuilder; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -68,6 +73,24 @@ void testIsRedirectedWithNonRedirectMethod() { testIsRedirected(new HttpPut("/put"), false); } + @Test + void testIsRedirectAllowedAlwaysTrue() { + final LaxRedirectStrategy strategy = new LaxRedirectStrategy(); + final HttpContext context = mock(HttpContext.class); + final HttpHost current = new HttpHost("http", "localhost", 80); + final HttpHost next = new HttpHost("http", "example.com", 80); + // Create a request with an Authorization header + final HttpRequest request = new HttpGet("/test"); + request.addHeader(HttpHeaders.AUTHORIZATION, "Bearer token"); + + // Even with sensitive headers and target change, LaxRedirectStrategy should allow it + assertTrue( + strategy.isRedirectAllowed(current, next, request, context), + "LaxRedirectStrategy should always allow redirects regardless of sensitive headers" + ); + } + + private void testIsRedirected(final HttpRequest request, final boolean expected) { final LaxRedirectStrategy strategy = new LaxRedirectStrategy(); final HttpResponse response = mock(HttpResponse.class); @@ -81,4 +104,83 @@ private void testIsRedirected(final HttpRequest request, final boolean expected) assertEquals(expected, strategy.isRedirected(request, response, context)); } + + /** + * Test {@link LaxRedirectStrategy#isRedirectAllowed(HttpHost, HttpHost, HttpRequest, HttpContext)}. + * The method should return true for all requests, enabling backward compatibility. + * + **/ + @Test + void testRedirectAllowed() throws Exception { + final LaxRedirectStrategy redirectStrategy = new LaxRedirectStrategy(); + + // Same host and port + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("somehost", 1234), + BasicRequestBuilder.get("/").build(), + null)); + + // Same host and port with Authorization header + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("somehost", 1234), + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.AUTHORIZATION, "let me pass") + .build(), + null)); + + // Same host and port with Cookie header + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("somehost", 1234), + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.COOKIE, "stuff=blah") + .build(), + null)); + + // Different host and same port + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("someotherhost", 1234), + BasicRequestBuilder.get("/") + .build(), + null)); + + // Different host and same port with Authorization header + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("someotherhost", 1234), + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.AUTHORIZATION, "let me pass") + .build(), + null)); + + // Different host and same port with Cookie header + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("someotherhost", 1234), + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.COOKIE, "stuff=blah") + .build(), + null)); + + // Same host and different ports with Authorization header + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("somehost", 80), + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.AUTHORIZATION, "let me pass") + .build(), + null)); + + // Same host and different ports with Cookie header + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + new HttpHost("somehost", 1234), + new HttpHost("somehost", 80), + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.COOKIE, "stuff=blah") + .build(), + null)); + } } \ No newline at end of file