Skip to content

Commit 4596aa0

Browse files
mkurzcodex
mkurz
and
codex
committed
Preserve QUERY redirects per RFC 10008
RFC 10008 requires QUERY requests to retain their method and content across 301, 302, 307, and 308 redirects. AHC treated QUERY like POST on 301 and non-strict 302, changing it to GET and dropping its content. Preserve QUERY while keeping the established POST and 303 behavior. Expose the standardized method constant and cover every redirect status, strict 302, repeatable bodies, and cross-origin credential stripping. OpenAI Codex on behalf of Matthias Kurz. Co-Authored-By: OpenAI Codex <codex@openai.com>
1 parent 49230f5 commit 4596aa0

4 files changed

Lines changed: 186 additions & 5 deletions

File tree

‎client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import static org.asynchttpclient.util.HttpConstants.Methods.GET;
5757
import static org.asynchttpclient.util.HttpConstants.Methods.HEAD;
5858
import static org.asynchttpclient.util.HttpConstants.Methods.OPTIONS;
59+
import static org.asynchttpclient.util.HttpConstants.Methods.QUERY;
5960
import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.FOUND_302;
6061
import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.MOVED_PERMANENTLY_301;
6162
import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.PERMANENT_REDIRECT_308;
@@ -116,11 +117,19 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture<?>
116117
future.setScramContext(null);
117118

118119
String originalMethod = request.getMethod();
119-
boolean switchToGet = !originalMethod.equals(GET) &&
120-
!originalMethod.equals(OPTIONS) &&
121-
!originalMethod.equals(HEAD) &&
122-
(statusCode == MOVED_PERMANENTLY_301 || statusCode == SEE_OTHER_303 || statusCode == FOUND_302 && !config.isStrict302Handling());
123-
boolean keepBody = statusCode == TEMPORARY_REDIRECT_307 || statusCode == PERMANENT_REDIRECT_308 || statusCode == FOUND_302 && config.isStrict302Handling();
120+
boolean isQuery = QUERY.equals(originalMethod);
121+
boolean methodAlreadyPreserved = GET.equals(originalMethod) ||
122+
OPTIONS.equals(originalMethod) || HEAD.equals(originalMethod);
123+
boolean strict302 = statusCode == FOUND_302 && config.isStrict302Handling();
124+
boolean queryRedirect = isQuery &&
125+
(statusCode == MOVED_PERMANENTLY_301 || statusCode == FOUND_302);
126+
boolean legacyRedirectToGet = statusCode == MOVED_PERMANENTLY_301 ||
127+
(statusCode == FOUND_302 && !strict302);
128+
boolean switchToGet = !methodAlreadyPreserved &&
129+
(statusCode == SEE_OTHER_303 || (!isQuery && legacyRedirectToGet));
130+
boolean keepBody = queryRedirect ||
131+
statusCode == TEMPORARY_REDIRECT_307 || statusCode == PERMANENT_REDIRECT_308 ||
132+
strict302;
124133

125134
HttpHeaders responseHeaders = response.headers();
126135
String location = responseHeaders.get(LOCATION);

‎client/src/main/java/org/asynchttpclient/util/HttpConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public static final class Methods {
3333
public static final String PATCH = HttpMethod.PATCH.name();
3434
public static final String POST = HttpMethod.POST.name();
3535
public static final String PUT = HttpMethod.PUT.name();
36+
public static final String QUERY = "QUERY";
3637
public static final String TRACE = HttpMethod.TRACE.name();
3738

3839
private Methods() {

‎client/src/test/java/org/asynchttpclient/RedirectBodyTest.java‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.commons.io.IOUtils;
2424
import org.asynchttpclient.filter.FilterContext;
2525
import org.asynchttpclient.filter.ResponseFilter;
26+
import org.asynchttpclient.request.body.generator.ByteArrayBodyGenerator;
2627
import org.asynchttpclient.request.body.generator.InputStreamBodyGenerator;
2728
import org.asynchttpclient.request.body.multipart.InputStreamPart;
2829
import org.asynchttpclient.request.body.multipart.StringPart;
@@ -49,6 +50,9 @@
4950
import static io.netty.handler.codec.http.HttpHeaderNames.LOCATION;
5051
import static org.asynchttpclient.Dsl.asyncHttpClient;
5152
import static org.asynchttpclient.Dsl.config;
53+
import static org.asynchttpclient.util.HttpConstants.Methods.GET;
54+
import static org.asynchttpclient.util.HttpConstants.Methods.POST;
55+
import static org.asynchttpclient.util.HttpConstants.Methods.QUERY;
5256
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
5357
import static org.junit.jupiter.api.Assertions.assertEquals;
5458
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
@@ -64,13 +68,15 @@ public class RedirectBodyTest extends AbstractBasicTest {
6468
private static final List<String> receivedContentLengths = new CopyOnWriteArrayList<>();
6569
private static volatile boolean redirectAlreadyPerformed;
6670
private static volatile String receivedContentType;
71+
private static volatile String receivedMethod;
6772
private static volatile Path fileToDeleteBeforeRedirect;
6873

6974
@BeforeEach
7075
public void setUp() {
7176
receivedContentLengths.clear();
7277
redirectAlreadyPerformed = false;
7378
receivedContentType = null;
79+
receivedMethod = null;
7480
fileToDeleteBeforeRedirect = null;
7581
}
7682

@@ -94,6 +100,7 @@ public void handle(String pathInContext, Request request, HttpServletRequest htt
94100

95101
} else {
96102
receivedContentType = request.getContentType();
103+
receivedMethod = request.getMethod();
97104
httpResponse.setStatus(200);
98105
httpResponse.setContentLength(body.length);
99106
if (body.length > 0) {
@@ -114,6 +121,7 @@ public void regular301LosesBody() throws Exception {
114121

115122
Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "301").execute().get(TIMEOUT, TimeUnit.SECONDS);
116123
assertEquals(response.getResponseBody(), "");
124+
assertEquals(GET, receivedMethod);
117125
assertNull(receivedContentType);
118126
}
119127
}
@@ -126,6 +134,7 @@ public void regular302LosesBody() throws Exception {
126134

127135
Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "302").execute().get(TIMEOUT, TimeUnit.SECONDS);
128136
assertEquals(response.getResponseBody(), "");
137+
assertEquals(GET, receivedMethod);
129138
assertNull(receivedContentType);
130139
}
131140
}
@@ -138,10 +147,24 @@ public void regular302StrictKeepsBody() throws Exception {
138147

139148
Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "302").execute().get(TIMEOUT, TimeUnit.SECONDS);
140149
assertEquals(response.getResponseBody(), body);
150+
assertEquals(POST, receivedMethod);
141151
assertEquals(receivedContentType, contentType);
142152
}
143153
}
144154

155+
@RepeatedIfExceptionsTest(repeats = 5)
156+
public void regular303SwitchesToGetAndLosesBody() throws Exception {
157+
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
158+
String body = "hello there";
159+
String contentType = "text/plain; charset=UTF-8";
160+
161+
Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "303").execute().get(TIMEOUT, TimeUnit.SECONDS);
162+
assertEquals("", response.getResponseBody());
163+
assertEquals(GET, receivedMethod);
164+
assertNull(receivedContentType);
165+
}
166+
}
167+
145168
@RepeatedIfExceptionsTest(repeats = 5)
146169
public void regular307KeepsBody() throws Exception {
147170
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
@@ -150,10 +173,85 @@ public void regular307KeepsBody() throws Exception {
150173

151174
Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "307").execute().get(TIMEOUT, TimeUnit.SECONDS);
152175
assertEquals(response.getResponseBody(), body);
176+
assertEquals(POST, receivedMethod);
153177
assertEquals(receivedContentType, contentType);
154178
}
155179
}
156180

181+
@RepeatedIfExceptionsTest(repeats = 5)
182+
public void regular308KeepsBody() throws Exception {
183+
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
184+
String body = "hello there";
185+
String contentType = "text/plain; charset=UTF-8";
186+
187+
Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "308").execute().get(TIMEOUT, TimeUnit.SECONDS);
188+
assertEquals(body, response.getResponseBody());
189+
assertEquals(POST, receivedMethod);
190+
assertEquals(contentType, receivedContentType);
191+
}
192+
}
193+
194+
@RepeatedIfExceptionsTest(repeats = 5)
195+
public void query301KeepsMethodAndBody() throws Exception {
196+
queryRedirectKeepsMethodAndBody(301, false);
197+
}
198+
199+
@RepeatedIfExceptionsTest(repeats = 5)
200+
public void query302KeepsMethodAndBody() throws Exception {
201+
queryRedirectKeepsMethodAndBody(302, false);
202+
}
203+
204+
@RepeatedIfExceptionsTest(repeats = 5)
205+
public void query302StrictKeepsMethodAndBody() throws Exception {
206+
queryRedirectKeepsMethodAndBody(302, true);
207+
}
208+
209+
@RepeatedIfExceptionsTest(repeats = 5)
210+
public void query303SwitchesToGetAndDropsBody() throws Exception {
211+
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
212+
String body = "hello there";
213+
String contentType = "text/plain; charset=UTF-8";
214+
215+
Response response = c.prepare(QUERY, getTargetUrl())
216+
.setHeader(CONTENT_TYPE, contentType)
217+
.setBody(body)
218+
.setHeader("X-REDIRECT", "303")
219+
.execute()
220+
.get(TIMEOUT, TimeUnit.SECONDS);
221+
assertEquals("", response.getResponseBody());
222+
assertEquals(GET, receivedMethod);
223+
assertNull(receivedContentType);
224+
}
225+
}
226+
227+
@RepeatedIfExceptionsTest(repeats = 5)
228+
public void query307KeepsMethodAndBody() throws Exception {
229+
queryRedirectKeepsMethodAndBody(307, false);
230+
}
231+
232+
@RepeatedIfExceptionsTest(repeats = 5)
233+
public void query308KeepsMethodAndBody() throws Exception {
234+
queryRedirectKeepsMethodAndBody(308, false);
235+
}
236+
237+
@RepeatedIfExceptionsTest(repeats = 5)
238+
public void query301KeepsRepeatableBodyGenerator() throws Exception {
239+
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
240+
byte[] body = "hello there".getBytes(UTF_8);
241+
String contentType = "text/plain; charset=UTF-8";
242+
243+
Response response = c.prepare(QUERY, getTargetUrl())
244+
.setHeader(CONTENT_TYPE, contentType)
245+
.setBody(new ByteArrayBodyGenerator(body))
246+
.setHeader("X-REDIRECT", "301")
247+
.execute()
248+
.get(TIMEOUT, TimeUnit.SECONDS);
249+
assertEquals("hello there", response.getResponseBody());
250+
assertEquals(QUERY, receivedMethod);
251+
assertEquals(contentType, receivedContentType);
252+
}
253+
}
254+
157255
@RepeatedIfExceptionsTest(repeats = 5)
158256
public void redirectPreservesPerRequestSettings() throws Exception {
159257
Duration readTimeout = Duration.ofSeconds(7);
@@ -424,6 +522,25 @@ public void inputStreamMultipart307FailsPromptly() throws Exception {
424522
}
425523
}
426524

525+
private void queryRedirectKeepsMethodAndBody(int statusCode, boolean strict302Handling) throws Exception {
526+
try (AsyncHttpClient c = asyncHttpClient(config()
527+
.setFollowRedirect(true)
528+
.setStrict302Handling(strict302Handling))) {
529+
String body = "hello there";
530+
String contentType = "text/plain; charset=UTF-8";
531+
532+
Response response = c.prepare(QUERY, getTargetUrl())
533+
.setHeader(CONTENT_TYPE, contentType)
534+
.setBody(body)
535+
.setHeader("X-REDIRECT", Integer.toString(statusCode))
536+
.execute()
537+
.get(TIMEOUT, TimeUnit.SECONDS);
538+
assertEquals(body, response.getResponseBody());
539+
assertEquals(QUERY, receivedMethod);
540+
assertEquals(contentType, receivedContentType);
541+
}
542+
}
543+
427544
private static Response execute307(BoundRequestBuilder requestBuilder) throws Exception {
428545
return requestBuilder
429546
.setHeader(CONTENT_TYPE, CONTENT_TYPE_VALUE)

‎client/src/test/java/org/asynchttpclient/RedirectCredentialSecurityTest.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.concurrent.atomic.AtomicReference;
3737

3838
import static org.asynchttpclient.Dsl.basicAuthRealm;
39+
import static org.asynchttpclient.util.HttpConstants.Methods.QUERY;
3940
import static org.junit.jupiter.api.Assertions.assertEquals;
4041
import static org.junit.jupiter.api.Assertions.assertNotNull;
4142
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -65,6 +66,11 @@ public class RedirectCredentialSecurityTest {
6566
private static final AtomicReference<String> cookieOn307Target = new AtomicReference<>();
6667
private static final AtomicReference<String> authOn308Target = new AtomicReference<>();
6768
private static final AtomicReference<String> bodyOn308Target = new AtomicReference<>();
69+
private static final AtomicReference<String> query301AuthOnTarget = new AtomicReference<>();
70+
private static final AtomicReference<String> query301CookieOnTarget = new AtomicReference<>();
71+
private static final AtomicReference<String> query301ContentTypeOnTarget = new AtomicReference<>();
72+
private static final AtomicReference<String> query301MethodOnTarget = new AtomicReference<>();
73+
private static final AtomicReference<String> query301BodyOnTarget = new AtomicReference<>();
6874
private static final AtomicReference<String> lastCookieHeaderOnA = new AtomicReference<>();
6975
private static final AtomicReference<String> lastCookieHeaderOnB = new AtomicReference<>();
7076
private static final AtomicReference<String> cookieAtChainStep2 = new AtomicReference<>();
@@ -189,6 +195,24 @@ public static void startServers() throws Exception {
189195
exchange.close();
190196
});
191197

198+
serverA.createContext("/redirect-query-301-to-b", exchange -> {
199+
exchange.getRequestBody().readAllBytes();
200+
exchange.getResponseHeaders().add("Location", "http://127.0.0.1:" + portB + "/target-query-301");
201+
exchange.sendResponseHeaders(301, -1);
202+
exchange.close();
203+
});
204+
205+
serverB.createContext("/target-query-301", exchange -> {
206+
query301AuthOnTarget.set(exchange.getRequestHeaders().getFirst("Authorization"));
207+
query301CookieOnTarget.set(exchange.getRequestHeaders().getFirst("Cookie"));
208+
query301ContentTypeOnTarget.set(exchange.getRequestHeaders().getFirst("Content-Type"));
209+
query301MethodOnTarget.set(exchange.getRequestMethod());
210+
query301BodyOnTarget.set(new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8));
211+
exchange.sendResponseHeaders(200, 0);
212+
exchange.getResponseBody().close();
213+
exchange.close();
214+
});
215+
192216
// Endpoint reused by the HTTPS-to-HTTP downgrade test (target on server B over plain HTTP)
193217
serverB.createContext("/target-after-downgrade", exchange -> {
194218
authAfterHttpsDowngrade.set(exchange.getRequestHeaders().getFirst("Authorization"));
@@ -511,6 +535,36 @@ void redirect308CrossDomainStripsAuthButPreservesBody() throws Exception {
511535
}
512536
}
513537

538+
@Test
539+
void query301CrossOriginStripsCredentialsAndPreservesRequest() throws Exception {
540+
DefaultAsyncHttpClientConfig config = new DefaultAsyncHttpClientConfig.Builder()
541+
.setFollowRedirect(true)
542+
.build();
543+
try (DefaultAsyncHttpClient client = new DefaultAsyncHttpClient(config)) {
544+
query301AuthOnTarget.set(null);
545+
query301CookieOnTarget.set(null);
546+
query301ContentTypeOnTarget.set(null);
547+
query301MethodOnTarget.set(null);
548+
query301BodyOnTarget.set(null);
549+
550+
client.prepare(QUERY, "http://127.0.0.1:" + portA + "/redirect-query-301-to-b")
551+
.setHeader("Authorization", "Bearer secret-token")
552+
.setHeader("Cookie", "session=secret-session")
553+
.setHeader("Content-Type", "application/query")
554+
.setBody("sensitive-query")
555+
.execute()
556+
.get(5, TimeUnit.SECONDS);
557+
558+
assertNull(query301AuthOnTarget.get(),
559+
"Authorization must be stripped on a cross-origin QUERY redirect");
560+
assertNull(query301CookieOnTarget.get(),
561+
"Cookie must be stripped on a cross-origin QUERY redirect");
562+
assertEquals(QUERY, query301MethodOnTarget.get());
563+
assertEquals("application/query", query301ContentTypeOnTarget.get());
564+
assertEquals("sensitive-query", query301BodyOnTarget.get());
565+
}
566+
}
567+
514568
/**
515569
* Cross-domain redirect (different port) must strip a user-supplied Cookie header.
516570
* Regression test for GHSA-fmxf-pm6p-7xgm.

0 commit comments

Comments
 (0)