From a177bd3c209526474bd90217836b37742be80f9a Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Wed, 26 Nov 2025 23:34:26 +0530 Subject: [PATCH 1/2] Fix NPE in NettyConnectListener (#2127) Fixes a NullPointerException triggered when timeout logging tries to append a null `remoteAddress` Fixes #2124 --- .../netty/channel/NettyConnectListener.java | 7 +- .../netty/timeout/TimeoutTimerTask.java | 22 +++++ .../netty/timeout/TimeoutsHolder.java | 8 +- .../netty/timeout/TimeoutTimerTaskTest.java | 87 +++++++++++++++++++ 4 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java diff --git a/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java b/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java index 2b6a840f5..a1d61177e 100755 --- a/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java +++ b/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java @@ -98,7 +98,12 @@ public void onSuccess(Channel channel, InetSocketAddress remoteAddress) { Request request = future.getTargetRequest(); Uri uri = request.getUri(); - timeoutsHolder.setResolvedRemoteAddress(remoteAddress); + // don't set a null resolved address - if the remoteAddress is null we keep + // the previously scheduled (possibly unresolved) address to avoid NPEs in + // timeout logging and keep useful diagnostic information + if (remoteAddress != null) { + timeoutsHolder.setResolvedRemoteAddress(remoteAddress); + } ProxyServer proxyServer = future.getProxyServer(); // For HTTPS proxies, establish SSL connection to the proxy server first diff --git a/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutTimerTask.java b/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutTimerTask.java index 3c9a3675e..b7e678fa8 100755 --- a/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutTimerTask.java +++ b/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutTimerTask.java @@ -57,6 +57,28 @@ public void clean() { void appendRemoteAddress(StringBuilder sb) { InetSocketAddress remoteAddress = timeoutsHolder.remoteAddress(); + + // Guard against null remoteAddress which can happen when the TimeoutsHolder + // was created without an original remote address (for example when using a + // pooled channel whose remoteAddress() returned null). In that case fall + // back to the URI host/port from the request to avoid a NPE and provide + // a useful diagnostic. + if (remoteAddress == null) { + if (nettyResponseFuture != null && nettyResponseFuture.getTargetRequest() != null) { + try { + String host = nettyResponseFuture.getTargetRequest().getUri().getHost(); + int port = nettyResponseFuture.getTargetRequest().getUri().getExplicitPort(); + sb.append(host == null ? "unknown" : host); + sb.append(':').append(port); + } catch (Exception ignored) { + sb.append("unknown:0"); + } + } else { + sb.append("unknown:0"); + } + return; + } + sb.append(remoteAddress.getHostString()); if (!remoteAddress.isUnresolved()) { sb.append('/').append(remoteAddress.getAddress().getHostAddress()); diff --git a/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java b/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java index acce84b6d..93f6b26a2 100755 --- a/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java +++ b/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java @@ -110,6 +110,12 @@ public void cancel() { } private Timeout newTimeout(TimerTask task, long delay) { - return requestSender.isClosed() ? null : nettyTimer.newTimeout(task, delay, TimeUnit.MILLISECONDS); + // requestSender or nettyTimer might be null in unit tests or in some edge + // cases where a channel's remote address wasn't available. In such cases + // avoid scheduling any timeouts rather than throwing a NPE. + if (requestSender == null || nettyTimer == null || requestSender.isClosed()) { + return null; + } + return nettyTimer.newTimeout(task, delay, TimeUnit.MILLISECONDS); } } diff --git a/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java b/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java new file mode 100644 index 000000000..2a5f5e205 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2014-2025 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.timeout; + +import org.asynchttpclient.AsyncCompletionHandler; +import org.asynchttpclient.DefaultAsyncHttpClientConfig; +import org.asynchttpclient.Request; +import org.asynchttpclient.RequestBuilder; +import org.asynchttpclient.channel.ChannelPoolPartitioning; +import org.asynchttpclient.netty.NettyResponseFuture; +import org.junit.jupiter.api.Test; + +import java.net.InetSocketAddress; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class TimeoutTimerTaskTest { + + @Test + public void appendRemoteAddressShouldNotThrowWhenRemoteAddressIsNull() { + Request request = new RequestBuilder().setUrl("http://example.com:12345").build(); + NettyResponseFuture future = new NettyResponseFuture<>(request, new AsyncCompletionHandler() { + @Override + public Object onCompleted(org.asynchttpclient.Response response) throws Exception { + return null; + } + }, null, + 0, ChannelPoolPartitioning.PerHostChannelPoolPartitioning.INSTANCE, null, null); + + // create TimeoutsHolder without an original remote address + TimeoutsHolder timeoutsHolder = new TimeoutsHolder(null, future, null, new DefaultAsyncHttpClientConfig.Builder().build(), null); + + TimeoutTimerTask task = new TimeoutTimerTask(future, null, timeoutsHolder) { + @Override + public void run(io.netty.util.Timeout timeout) { + // no-op + } + }; + + StringBuilder sb = new StringBuilder(); + task.appendRemoteAddress(sb); + + // fallback should include URI host/port + assertTrue(sb.toString().contains("example.com:12345"), sb.toString()); + } + + @Test + public void appendRemoteAddressShouldPrintResolvedAddressIfAvailable() { + Request request = new RequestBuilder().setUrl("http://example.com:12345").build(); + NettyResponseFuture future = new NettyResponseFuture<>(request, new AsyncCompletionHandler() { + @Override + public Object onCompleted(org.asynchttpclient.Response response) throws Exception { + return null; + } + }, null, + 0, ChannelPoolPartitioning.PerHostChannelPoolPartitioning.INSTANCE, null, null); + + TimeoutsHolder timeoutsHolder = new TimeoutsHolder(null, future, null, new DefaultAsyncHttpClientConfig.Builder().build(), null); + + // set a resolved remote address + timeoutsHolder.setResolvedRemoteAddress(new InetSocketAddress("127.0.0.1", 8080)); + + TimeoutTimerTask task = new TimeoutTimerTask(future, null, timeoutsHolder) { + @Override + public void run(io.netty.util.Timeout timeout) { + // no-op + } + }; + + StringBuilder sb = new StringBuilder(); + task.appendRemoteAddress(sb); + assertTrue(sb.toString().contains(":8080"), sb.toString()); + } +} From 4932e863fbce3464cc5b69b21b66ad15ae46e65e Mon Sep 17 00:00:00 2001 From: Aayush Atharva <24762260+hyperxpro@users.noreply.github.com> Date: Wed, 3 Dec 2025 03:02:06 +0530 Subject: [PATCH 2/2] Maintain Content-Type set explicitly by client (#2130) Closes #2129 --- .../asynchttpclient/RequestBuilderBase.java | 61 +++++- .../asynchttpclient/RequestBuilderTest.java | 192 ++++++++++++++++++ 2 files changed, 250 insertions(+), 3 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java b/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java index dbc5e4144..29bbaa670 100644 --- a/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java +++ b/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java @@ -93,6 +93,30 @@ public abstract class RequestBuilderBase> { protected @Nullable Charset charset; protected ChannelPoolPartitioning channelPoolPartitioning = ChannelPoolPartitioning.PerHostChannelPoolPartitioning.INSTANCE; protected NameResolver nameResolver = DEFAULT_NAME_RESOLVER; + protected boolean contentTypeLocked; + + /** + * Mark the Content-Type header as explicitly set by the user. When locked, the + * Content-Type header will not be modified by the client (e.g., charset addition). + */ + protected final void doContentTypeLock() { + this.contentTypeLocked = true; + } + + /** + * Clear the Content-Type lock, allowing the client to modify the Content-Type header + * if needed (for example, to add charset when it was auto-generated). + */ + protected final void resetContentTypeLock() { + this.contentTypeLocked = false; + } + + /** + * Return whether the Content-Type header has been locked as explicitly set by the user. + */ + protected final boolean isContentTypeLocked() { + return this.contentTypeLocked; + } protected RequestBuilderBase(String method, boolean disableUrlEncoding) { this(method, disableUrlEncoding, true); @@ -116,6 +140,10 @@ protected RequestBuilderBase(Request prototype, boolean disableUrlEncoding, bool localAddress = prototype.getLocalAddress(); headers = new DefaultHttpHeaders(validateHeaders); headers.add(prototype.getHeaders()); + // If prototype has Content-Type, consider it as explicitly set + if (headers.contains(CONTENT_TYPE)) { + doContentTypeLock(); + } if (isNonEmpty(prototype.getCookies())) { cookies = new ArrayList<>(prototype.getCookies()); } @@ -181,6 +209,7 @@ public T setVirtualHost(String virtualHost) { */ public T clearHeaders() { headers.clear(); + resetContentTypeLock(); return asDerivedType(); } @@ -203,6 +232,9 @@ public T setHeader(CharSequence name, String value) { */ public T setHeader(CharSequence name, Object value) { headers.set(name, value); + if (CONTENT_TYPE.contentEqualsIgnoreCase(name)) { + doContentTypeLock(); + } return asDerivedType(); } @@ -215,6 +247,9 @@ public T setHeader(CharSequence name, Object value) { */ public T setHeader(CharSequence name, Iterable values) { headers.set(name, values); + if (CONTENT_TYPE.contentEqualsIgnoreCase(name)) { + doContentTypeLock(); + } return asDerivedType(); } @@ -243,6 +278,9 @@ public T addHeader(CharSequence name, Object value) { } headers.add(name, value); + if (CONTENT_TYPE.contentEqualsIgnoreCase(name)) { + doContentTypeLock(); + } return asDerivedType(); } @@ -256,6 +294,9 @@ public T addHeader(CharSequence name, Object value) { */ public T addHeader(CharSequence name, Iterable values) { headers.add(name, values); + if (CONTENT_TYPE.contentEqualsIgnoreCase(name)) { + doContentTypeLock(); + } return asDerivedType(); } @@ -264,6 +305,9 @@ public T setHeaders(HttpHeaders headers) { this.headers.clear(); } else { this.headers = headers; + if (headers.contains(CONTENT_TYPE)) { + doContentTypeLock(); + } } return asDerivedType(); } @@ -278,7 +322,12 @@ public T setHeaders(HttpHeaders headers) { public T setHeaders(Map> headers) { clearHeaders(); if (headers != null) { - headers.forEach((name, values) -> this.headers.add(name, values)); + headers.forEach((name, values) -> { + this.headers.add(name, values); + if (CONTENT_TYPE.contentEqualsIgnoreCase(name)) { + doContentTypeLock(); + } + }); } return asDerivedType(); } @@ -293,7 +342,12 @@ public T setHeaders(Map> headers) public T setSingleHeaders(Map headers) { clearHeaders(); if (headers != null) { - headers.forEach((name, value) -> this.headers.add(name, value)); + headers.forEach((name, value) -> { + this.headers.add(name, value); + if (CONTENT_TYPE.contentEqualsIgnoreCase(name)) { + doContentTypeLock(); + } + }); } return asDerivedType(); } @@ -634,7 +688,8 @@ private void updateCharset() { String contentTypeHeader = headers.get(CONTENT_TYPE); Charset contentTypeCharset = extractContentTypeCharsetAttribute(contentTypeHeader); charset = withDefault(contentTypeCharset, withDefault(charset, UTF_8)); - if (contentTypeHeader != null && contentTypeHeader.regionMatches(true, 0, "text/", 0, 5) && contentTypeCharset == null) { + // Only add charset if Content-Type was not explicitly set by user + if (!isContentTypeLocked() && contentTypeHeader != null && contentTypeHeader.regionMatches(true, 0, "text/", 0, 5) && contentTypeCharset == null) { // add explicit charset to content-type header headers.set(CONTENT_TYPE, contentTypeHeader + "; charset=" + charset.name()); } diff --git a/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java b/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java index 34e79121d..2da2246d6 100644 --- a/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java +++ b/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java @@ -16,6 +16,8 @@ package org.asynchttpclient; import io.github.artsok.RepeatedIfExceptionsTest; +import io.netty.handler.codec.http.DefaultHttpHeaders; +import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http.cookie.DefaultCookie; @@ -29,7 +31,9 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; import static org.asynchttpclient.Dsl.get; +import static org.asynchttpclient.Dsl.post; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; public class RequestBuilderTest { @@ -220,4 +224,192 @@ public void testSettingHeadersUsingMapWithStringKeys() { Request request = requestBuilder.build(); assertEquals(request.getHeaders().get("X-Forwarded-For"), "10.0.0.1"); } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testUserSetTextPlainContentTypeShouldNotBeModified() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", "text/plain") + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type should not be modified when user explicitly sets it"); + assertFalse(contentType.contains("charset"), "Charset should not be added to user-specified Content-Type"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testUserSetTextXmlContentTypeShouldNotBeModified() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", "text/xml") + .setBody("Hello") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/xml", contentType, "Content-Type should not be modified when user explicitly sets it"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testUserSetTextHtmlContentTypeShouldNotBeModified() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", "text/html") + .setBody("") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/html", contentType, "Content-Type should not be modified when user explicitly sets it"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testUserSetContentTypeWithCharsetShouldBePreserved() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", "text/xml; charset=ISO-8859-1") + .setBody("Hello") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/xml; charset=ISO-8859-1", contentType, "User-specified charset should be preserved"); + assertTrue(contentType.contains("ISO-8859-1"), "ISO-8859-1 charset should be preserved"); + assertFalse(contentType.contains("UTF-8"), "UTF-8 should not be added"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testApplicationJsonContentTypeShouldNotBeModified() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", "application/json") + .setBody("{\"key\": \"value\"}") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("application/json", contentType, "application/json should not be modified"); + assertFalse(contentType.contains("charset"), "Charset should not be added to application/json"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testAddHeaderContentTypeShouldNotBeModified() { + Request request = post("http://localhost/test") + .addHeader("Content-Type", "text/plain") + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type set via addHeader should not be modified"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testSetHeadersWithHttpHeadersShouldLockContentType() { + HttpHeaders httpHeaders = new DefaultHttpHeaders(); + httpHeaders.set("Content-Type", "text/plain"); + + Request request = post("http://localhost/test") + .setHeaders(httpHeaders) + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type set via setHeaders(HttpHeaders) should not be modified"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testSetHeadersWithMapShouldLockContentType() { + Map> headerMap = new HashMap<>(); + headerMap.put("Content-Type", singletonList("text/plain")); + + Request request = post("http://localhost/test") + .setHeaders(headerMap) + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type set via setHeaders(Map) should not be modified"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testSetSingleHeadersShouldLockContentType() { + Map headerMap = new HashMap<>(); + headerMap.put("Content-Type", "text/plain"); + + Request request = post("http://localhost/test") + .setSingleHeaders(headerMap) + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type set via setSingleHeaders should not be modified"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testClearHeadersShouldResetContentTypeLock() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", "text/plain") + .clearHeaders() + .setHeader("Content-Type", "text/xml") + .setBody("") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/xml", contentType, "Content-Type should still be preserved after clear and re-set"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testPrototypeRequestShouldPreserveContentType() { + Request original = post("http://localhost/test") + .setHeader("Content-Type", "text/plain") + .setBody("Hello") + .build(); + + Request copy = post("http://localhost/test") + .setUrl(original.getUri().toUrl()) + .setHeaders(original.getHeaders()) + .setBody("Hello") + .build(); + + String contentType = copy.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type should be preserved from prototype"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testRequestBuilderFromPrototypeShouldPreserveContentType() { + Request original = post("http://localhost/test") + .setHeader("Content-Type", "text/plain") + .setBody("Hello") + .build(); + + Request copy = new RequestBuilder(original).build(); + + String contentType = copy.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type should be preserved from prototype via RequestBuilder"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testCaseInsensitiveContentTypeHeader() { + Request request = post("http://localhost/test") + .setHeader("content-type", "text/plain") + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type should be matched case-insensitively"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testSetHeaderWithIterableShouldLockContentType() { + Request request = post("http://localhost/test") + .setHeader("Content-Type", singletonList("text/plain")) + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type set via setHeader(Iterable) should not be modified"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testAddHeaderWithIterableShouldLockContentType() { + Request request = post("http://localhost/test") + .addHeader("Content-Type", singletonList("text/plain")) + .setBody("Hello World") + .build(); + + String contentType = request.getHeaders().get("Content-Type"); + assertEquals("text/plain", contentType, "Content-Type set via addHeader(Iterable) should not be modified"); + } }