From dfe2be0ee21eb1d5cf8b73ed20719043f3caa084 Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:00:02 +0200 Subject: [PATCH 1/3] Skip redirect set lookup for non-3xx responses Redirect30xInterceptor.REDIRECT_STATUSES is a Set, so the membership test in Interceptors.exitAfterIntercept autoboxed the int status code. HTTP status codes are all above 127 and therefore outside the range Integer.valueOf caches, so every response allocated a fresh Integer and hashed it, only for the answer to be false on the 2xx, 4xx and 5xx responses that make up almost all traffic. Guard the lookup with a 300..399 range check. The set is public and mutable, so it is deliberately still consulted rather than inlined as a switch over the five known codes: a caller that registered an extra 3xx status keeps having it honoured, and only a genuine redirect now pays for the boxing. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5 --- .../netty/handler/intercept/Interceptors.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java index 5e12e2dfa..8aa98e5d5 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java @@ -125,7 +125,11 @@ public boolean exitAfterIntercept(Channel channel, NettyResponseFuture future return continue100Interceptor.exitAfterHandling100(channel, future); } - if (Redirect30xInterceptor.REDIRECT_STATUSES.contains(statusCode)) { + // Range check first: REDIRECT_STATUSES is a Set, so contains(statusCode) boxed a fresh + // Integer on every response (status codes are outside Integer's valueOf cache). The set is public + // and mutable, so the lookup is kept rather than inlined as a switch, and a caller that registered + // an extra 3xx status still has it honoured. + if (statusCode >= 300 && statusCode < 400 && Redirect30xInterceptor.REDIRECT_STATUSES.contains(statusCode)) { return redirect30xInterceptor.exitAfterHandlingRedirect(channel, future, response, request, statusCode, realm); } From e9bceb1ea89cf402ff3ad6ad6d052a6de1fe72b4 Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Tue, 4 Aug 2026 08:15:54 +0200 Subject: [PATCH 2/3] Move the redirect status test next to the set Review feedback on #2301. The range check lived in Interceptors while the statuses it guards live in Redirect30xInterceptor, splitting one decision across two classes. Move it into a package-private isRedirect(int) beside the set, so Interceptors reads as a question about redirects and the knowledge stays in one place. Use Netty's HttpStatusClass.REDIRECTION for the class check rather than an open-coded 300..400: it takes an int, so it still keeps the boxing lookup off non-3xx responses, and it names what the bounds mean. Drop the comment at the call site. It described the previous version of the code rather than the code, and the predicate now says what it does. The new test covers the part that is not obvious from either half on its own: a 3xx that is not a followed redirect, 304 above all, has to be rejected exactly like a non-3xx status. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5 --- .../netty/handler/intercept/Interceptors.java | 6 +-- .../intercept/Redirect30xInterceptor.java | 11 ++++ .../intercept/Redirect30xInterceptorTest.java | 52 +++++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java index 8aa98e5d5..c64cdf5a0 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java @@ -125,11 +125,7 @@ public boolean exitAfterIntercept(Channel channel, NettyResponseFuture future return continue100Interceptor.exitAfterHandling100(channel, future); } - // Range check first: REDIRECT_STATUSES is a Set, so contains(statusCode) boxed a fresh - // Integer on every response (status codes are outside Integer's valueOf cache). The set is public - // and mutable, so the lookup is kept rather than inlined as a switch, and a caller that registered - // an extra 3xx status still has it honoured. - if (statusCode >= 300 && statusCode < 400 && Redirect30xInterceptor.REDIRECT_STATUSES.contains(statusCode)) { + if (Redirect30xInterceptor.isRedirect(statusCode)) { return redirect30xInterceptor.exitAfterHandlingRedirect(channel, future, response, request, statusCode, realm); } 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 189e78309..910d4f209 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 @@ -18,6 +18,7 @@ import io.netty.channel.Channel; import io.netty.handler.codec.http.HttpHeaders; 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.Cookie; import org.asynchttpclient.AsyncHttpClientConfig; @@ -70,6 +71,16 @@ public class Redirect30xInterceptor { REDIRECT_STATUSES.add(PERMANENT_REDIRECT_308); } + /** + * Whether {@code statusCode} is a redirect this interceptor follows. Only a 3xx can be, and the class + * check takes an {@code int}, so it keeps the {@link #REDIRECT_STATUSES} lookup, which boxes, off the + * responses that make up almost all traffic. The set is still consulted rather than inlined here because + * it is public and mutable, so an extra 3xx status a caller registered stays honoured. + */ + static boolean isRedirect(int statusCode) { + return HttpStatusClass.REDIRECTION.contains(statusCode) && REDIRECT_STATUSES.contains(statusCode); + } + private final ChannelManager channelManager; private final AsyncHttpClientConfig config; private final NettyRequestSender requestSender; diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java new file mode 100644 index 000000000..797cceea3 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java @@ -0,0 +1,52 @@ +/* + * 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 org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests {@link Redirect30xInterceptor#isRedirect(int)}: the 3xx class check and the + * {@link Redirect30xInterceptor#REDIRECT_STATUSES} membership must agree, so a 3xx that is not a followed + * redirect is rejected just like a non-3xx status. + */ +public class Redirect30xInterceptorTest { + + @Test + public void acceptsTheFollowedRedirectStatuses() { + for (int statusCode : new int[]{301, 302, 303, 307, 308}) { + assertTrue(Redirect30xInterceptor.isRedirect(statusCode), statusCode + " should be a redirect"); + } + } + + @Test + public void rejects3xxStatusesThatAreNotFollowed() { + // in the 3xx class, but not redirects this interceptor acts on: 304 in particular must fall through + // to the normal response path rather than be treated as a redirect + for (int statusCode : new int[]{300, 304, 305, 306, 399}) { + assertFalse(Redirect30xInterceptor.isRedirect(statusCode), statusCode + " should not be a redirect"); + } + } + + @Test + public void rejectsStatusesOutsideThe3xxClass() { + for (int statusCode : new int[]{100, 200, 204, 299, 400, 404, 500}) { + assertFalse(Redirect30xInterceptor.isRedirect(statusCode), statusCode + " should not be a redirect"); + } + } +} From 40c58c05627966f9a1cb1fa7b2975379508ae4af Mon Sep 17 00:00:00 2001 From: Aayush Atharva <24762260+hyperxpro@users.noreply.github.com> Date: Wed, 5 Aug 2026 23:46:59 +0530 Subject: [PATCH 3/3] Update client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java --- .../netty/handler/intercept/Redirect30xInterceptor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 910d4f209..bc5154c24 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 @@ -73,9 +73,9 @@ public class Redirect30xInterceptor { /** * Whether {@code statusCode} is a redirect this interceptor follows. Only a 3xx can be, and the class - * check takes an {@code int}, so it keeps the {@link #REDIRECT_STATUSES} lookup, which boxes, off the - * responses that make up almost all traffic. The set is still consulted rather than inlined here because - * it is public and mutable, so an extra 3xx status a caller registered stays honoured. + * check takes an {@code int}, so the boxing {@link #REDIRECT_STATUSES} lookup stays off the 2xx, 4xx + * and 5xx responses that carry almost all traffic. The set is consulted rather than inlined because it + * is public and mutable, so an extra 3xx a caller registered is honoured. */ static boolean isRedirect(int statusCode) { return HttpStatusClass.REDIRECTION.contains(statusCode) && REDIRECT_STATUSES.contains(statusCode);