diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java index 7d2a9705f..39360acda 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java @@ -20,6 +20,7 @@ import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpStatusClass; import io.netty.handler.codec.http.HttpUtil; +import io.netty.handler.codec.http.cookie.ClientCookieDecoder; import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http2.Http2StreamChannel; import io.netty.util.AsciiString; @@ -47,7 +48,9 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import static io.netty.handler.codec.http.HttpHeaderNames.AUTHORIZATION; @@ -57,6 +60,7 @@ import static io.netty.handler.codec.http.HttpHeaderNames.HOST; import static io.netty.handler.codec.http.HttpHeaderNames.LOCATION; import static io.netty.handler.codec.http.HttpHeaderNames.PROXY_AUTHORIZATION; +import static io.netty.handler.codec.http.HttpHeaderNames.SET_COOKIE; import static org.asynchttpclient.uri.Uri.HTTP; import static org.asynchttpclient.uri.Uri.HTTPS; import static org.asynchttpclient.uri.Uri.WS; @@ -104,6 +108,7 @@ static boolean isRedirect(int statusCode) { private final boolean stripAuthorizationOnRedirect; private final boolean refuseSchemeDowngradeOnRedirect; private final boolean refuseCrossOriginBodyOnRedirect; + private final ClientCookieDecoder cookieDecoder; Redirect30xInterceptor(ChannelManager channelManager, AsyncHttpClientConfig config, NettyRequestSender requestSender) { this.channelManager = channelManager; @@ -112,6 +117,7 @@ static boolean isRedirect(int statusCode) { stripAuthorizationOnRedirect = config.isStripAuthorizationOnRedirect(); // New flag refuseSchemeDowngradeOnRedirect = config.isRefuseSchemeDowngradeOnRedirect(); refuseCrossOriginBodyOnRedirect = config.isRefuseCrossOriginBodyOnRedirect(); + cookieDecoder = config.isUseLaxCookieEncoder() ? ClientCookieDecoder.LAX : ClientCookieDecoder.STRICT; maxRedirectException = unknownStackTrace(new MaxRedirectException("Maximum redirect reached: " + config.getMaxRedirects()), Redirect30xInterceptor.class, "exitAfterHandlingRedirect"); } @@ -199,6 +205,15 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture .setProxyServer(request.getProxyServer()) .setRangeOffset(request.getRangeOffset()); } + // A same-origin hop keeps the caller's cookies but not the store's: those are its values from + // before this response and would outrank what it just set. The store adds its current ones below. + CookieStore cookieStore = config.getCookieStore(); + if (stripAuth) { + requestBuilder.resetCookies(); + } else { + requestBuilder.setCookies(cookieStore == null + ? request.getCookies() : callersOwnCookies(request, response, cookieStore)); + } requestBuilder.setMethod(switchToGet ? GET : originalMethod) .setFollowRedirect(true) @@ -235,15 +250,12 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture if (stripAuth) { future.setRealm(null); future.setProxyRealm(null); - // Request.toBuilder copies Cookie objects separately from the Cookie header. - requestBuilder.resetCookies(); } // in case of a redirect from HTTP to HTTPS, future // attributes might change final boolean initialConnectionKeepAlive = future.isKeepAlive(); - CookieStore cookieStore = config.getCookieStore(); if (cookieStore != null) { // Update request's cookies assuming that cookie store is already updated by Interceptors for (Cookie cookie : cookieStore.get(newUri)) { @@ -448,6 +460,42 @@ private enum BodyRepresentation { NONE } + /** + * The request's cookies minus the ones the store put there: those this response set, rotated or deleted, + * and those the store still holds with the same value. A caller's cookie sharing only a name with a stored + * one stays the caller's. + */ + private List callersOwnCookies(Request request, HttpResponse response, CookieStore cookieStore) { + List cookies = request.getCookies(); + if (cookies.isEmpty()) { + return cookies; + } + Set setByResponse = new HashSet<>(); + for (String header : response.headers().getAll(SET_COOKIE)) { + Cookie cookie = cookieDecoder.decode(header); + if (cookie != null) { + setByResponse.add(cookie.name()); + } + } + List stored = cookieStore.get(request.getUri()); + List callers = new ArrayList<>(cookies.size()); + for (Cookie cookie : cookies) { + if (!setByResponse.contains(cookie.name()) && !holdsSameValue(stored, cookie)) { + callers.add(cookie); + } + } + return callers; + } + + private static boolean holdsSameValue(List stored, Cookie cookie) { + for (Cookie candidate : stored) { + if (candidate.name().equals(cookie.name()) && candidate.value().equals(cookie.value())) { + return true; + } + } + return false; + } + private static HttpHeaders propagatedHeaders(Request request, Realm realm, boolean keepBody, boolean stripAuthorization) { HttpHeaders headers = request.getHeaders().copy().remove(HOST); diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java new file mode 100644 index 000000000..097611568 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java @@ -0,0 +1,214 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.asynchttpclient.netty.handler.intercept; + +import io.netty.handler.codec.http.cookie.DefaultCookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.asynchttpclient.AbstractBasicTest; +import org.asynchttpclient.AsyncHttpClient; +import org.asynchttpclient.AsyncHttpClientConfig; +import org.asynchttpclient.BoundRequestBuilder; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; + +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The request a redirect leads to must carry what the redirect response just stored, not the cookies the + * previous request went out with, whether the redirect keeps the method or switches to GET. + */ +public class RedirectCookieRotationTest extends AbstractBasicTest { + + private static final String RECEIVED_COOKIE = "received-cookie"; + + @Override + public AbstractHandler configureHandler() { + return new AbstractHandler() { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, + HttpServletResponse response) throws IOException { + switch (target) { + case "/seed": + response.addHeader("Set-Cookie", "SID=old; Path=/"); + response.addHeader("Set-Cookie", "P=scoped; Path=/p"); + break; + case "/login": + redirect(response, HttpServletResponse.SC_FOUND, "SID=new; Path=/", "/home"); + break; + case "/login-307": + redirect(response, 307, "SID=new; Path=/", "/home"); + break; + case "/logout": + redirect(response, HttpServletResponse.SC_FOUND, "SID=; Path=/; Max-Age=0", "/home"); + break; + case "/logout-303": + redirect(response, HttpServletResponse.SC_SEE_OTHER, "SID=; Path=/; Max-Age=0", "/home"); + break; + case "/logout-307": + redirect(response, 307, "SID=; Path=/; Max-Age=0", "/home"); + break; + case "/p/a": + redirect(response, HttpServletResponse.SC_FOUND, null, "/q/b"); + break; + case "/bounce": + redirect(response, HttpServletResponse.SC_FOUND, null, "/home"); + break; + case "/bounce-307": + redirect(response, 307, null, "/home"); + break; + case "/see-other": + redirect(response, HttpServletResponse.SC_SEE_OTHER, null, "/home"); + break; + case "/elsewhere": + redirect(response, HttpServletResponse.SC_FOUND, null, "http://127.0.0.1:" + port1 + "/home"); + break; + default: + String cookie = request.getHeader("Cookie"); + if (cookie != null) { + response.setHeader(RECEIVED_COOKIE, cookie); + } + } + baseRequest.setHandled(true); + } + }; + } + + @Test + void aGetRedirectSendsTheSessionItRotated() throws Exception { + assertEquals("SID=new", afterSeeding(client -> client.prepareGet(url("/login")))); + } + + @Test + void a307SendsTheSessionItRotated() throws Exception { + assertEquals("SID=new", afterSeeding(client -> client.preparePost(url("/login-307")))); + } + + @Test + void aGetRedirectDoesNotResendACookieItDeleted() throws Exception { + assertNull(afterSeeding(client -> client.prepareGet(url("/logout")))); + } + + @Test + void a307DoesNotResendACookieItDeleted() throws Exception { + assertNull(afterSeeding(client -> client.preparePost(url("/logout-307")))); + } + + @Test + void aPathScopedCookieDoesNotFollowARedirectOutOfItsPath() throws Exception { + String received = afterSeeding(client -> client.prepareGet(url("/p/a"))); + assertFalse(received != null && received.contains("P="), "sent to /q/b: " + received); + } + + // The next two already hold on main, where a redirect to GET is built from scratch; they keep it that way. + + @Test + void aPostRedirectedToGetSendsTheSessionItRotated() throws Exception { + assertEquals("SID=new", afterSeeding(client -> client.preparePost(url("/login")))); + } + + @Test + void a303DoesNotResendACookieItDeleted() throws Exception { + assertNull(afterSeeding(client -> client.preparePost(url("/logout-303")))); + } + + // The caller's own cookies follow a same-origin redirect; only the store's are replaced. + + @Test + void theCallersCookieFollowsASameOriginRedirect() throws Exception { + assertTrue(cookiesSent(withCallerCookie("X", "1", client -> client.prepareGet(url("/bounce")))).contains("X=1"), + "GET, 302"); + assertTrue(cookiesSent(withCallerCookie("X", "1", client -> client.preparePost(url("/bounce-307")))) + .contains("X=1"), "POST, 307"); + } + + @Test + void theCallersCookieIsSentBesideTheSessionTheRedirectRotated() throws Exception { + Set sent = cookiesSent(withCallerCookie("X", "1", client -> client.prepareGet(url("/login")))); + assertEquals(new HashSet<>(Arrays.asList("X=1", "SID=new")), sent); + } + + @Test + void theCallersCookieStillBeatsAStoredOneOfTheSameName() throws Exception { + assertEquals("SID=mine", withCallerCookie("SID", "mine", client -> client.prepareGet(url("/bounce")))); + } + + // Without a cookie store every cookie on the request is the caller's own. + + @Test + void withoutAStoreTheCallersCookieFollowsASameOriginRedirect() throws Exception { + assertEquals("X=1", withoutAStore(client -> client.prepareGet(url("/login"))), "GET, 302"); + assertEquals("X=1", withoutAStore(client -> client.preparePost(url("/see-other"))), "POST, 303"); + } + + @Test + void withoutAStoreTheCallersCookieStaysBehindOnACrossOriginRedirect() throws Exception { + assertNull(withoutAStore(client -> client.prepareGet(url("/elsewhere")))); + } + + private String afterSeeding(Function request) throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + client.prepareGet(url("/seed")).execute().get(TIMEOUT, TimeUnit.SECONDS); + return request.apply(client).execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + } + } + + private String withCallerCookie(String name, String value, Function request) + throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + client.prepareGet(url("/seed")).execute().get(TIMEOUT, TimeUnit.SECONDS); + return request.apply(client).addCookie(new DefaultCookie(name, value)) + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + } + } + + private static Set cookiesSent(String header) { + return header == null ? new HashSet<>() : new HashSet<>(Arrays.asList(header.split("; "))); + } + + private String withoutAStore(Function request) throws Exception { + AsyncHttpClientConfig noStore = config().setFollowRedirect(true).setCookieStore(null).build(); + try (AsyncHttpClient client = asyncHttpClient(noStore)) { + return request.apply(client).addCookie(new DefaultCookie("X", "1")) + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + } + } + + private static void redirect(HttpServletResponse response, int status, String setCookie, String location) { + if (setCookie != null) { + response.addHeader("Set-Cookie", setCookie); + } + response.setStatus(status); + response.setHeader("Location", location); + } + + private String url(String path) { + return "http://localhost:" + port1 + path; + } +}