Skip to content

Commit 8014ce7

Browse files
committed
Fix NPE in NettyConnectListener
1 parent b7e0319 commit 8014ce7

File tree

4 files changed

+110
-2
lines changed

4 files changed

+110
-2
lines changed

client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,12 @@ public void onSuccess(Channel channel, InetSocketAddress remoteAddress) {
9898

9999
Request request = future.getTargetRequest();
100100
Uri uri = request.getUri();
101-
timeoutsHolder.setResolvedRemoteAddress(remoteAddress);
101+
// don't set a null resolved address - if the remoteAddress is null we keep
102+
// the previously scheduled (possibly unresolved) address to avoid NPEs in
103+
// timeout logging and keep useful diagnostic information
104+
if (remoteAddress != null) {
105+
timeoutsHolder.setResolvedRemoteAddress(remoteAddress);
106+
}
102107
ProxyServer proxyServer = future.getProxyServer();
103108

104109
// For HTTPS proxies, establish SSL connection to the proxy server first

client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutTimerTask.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,28 @@ public void clean() {
5757

5858
void appendRemoteAddress(StringBuilder sb) {
5959
InetSocketAddress remoteAddress = timeoutsHolder.remoteAddress();
60+
61+
// Guard against null remoteAddress which can happen when the TimeoutsHolder
62+
// was created without an original remote address (for example when using a
63+
// pooled channel whose remoteAddress() returned null). In that case fall
64+
// back to the URI host/port from the request to avoid a NPE and provide
65+
// a useful diagnostic.
66+
if (remoteAddress == null) {
67+
if (nettyResponseFuture != null && nettyResponseFuture.getTargetRequest() != null) {
68+
try {
69+
String host = nettyResponseFuture.getTargetRequest().getUri().getHost();
70+
int port = nettyResponseFuture.getTargetRequest().getUri().getExplicitPort();
71+
sb.append(host == null ? "unknown" : host);
72+
sb.append(':').append(port);
73+
} catch (Exception ignored) {
74+
sb.append("unknown:0");
75+
}
76+
} else {
77+
sb.append("unknown:0");
78+
}
79+
return;
80+
}
81+
6082
sb.append(remoteAddress.getHostString());
6183
if (!remoteAddress.isUnresolved()) {
6284
sb.append('/').append(remoteAddress.getAddress().getHostAddress());

client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ public void cancel() {
110110
}
111111

112112
private Timeout newTimeout(TimerTask task, long delay) {
113-
return requestSender.isClosed() ? null : nettyTimer.newTimeout(task, delay, TimeUnit.MILLISECONDS);
113+
// requestSender or nettyTimer might be null in unit tests or in some edge
114+
// cases where a channel's remote address wasn't available. In such cases
115+
// avoid scheduling any timeouts rather than throwing a NPE.
116+
if (requestSender == null || nettyTimer == null || requestSender.isClosed()) {
117+
return null;
118+
}
119+
return nettyTimer.newTimeout(task, delay, TimeUnit.MILLISECONDS);
114120
}
115121
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Copyright (c) 2014-2025 AsyncHttpClient Project.
3+
*/
4+
package org.asynchttpclient.netty.timeout;
5+
6+
import org.asynchttpclient.AsyncCompletionHandler;
7+
import org.asynchttpclient.DefaultAsyncHttpClientConfig;
8+
import org.asynchttpclient.Request;
9+
import org.asynchttpclient.RequestBuilder;
10+
import org.asynchttpclient.channel.ChannelPoolPartitioning;
11+
import org.asynchttpclient.netty.NettyResponseFuture;
12+
import org.junit.jupiter.api.Test;
13+
14+
import java.net.InetSocketAddress;
15+
16+
import static org.junit.jupiter.api.Assertions.assertTrue;
17+
18+
public class TimeoutTimerTaskTest {
19+
20+
@Test
21+
public void appendRemoteAddressShouldNotThrowWhenRemoteAddressIsNull() {
22+
Request request = new RequestBuilder().setUrl("http://example.com:12345").build();
23+
NettyResponseFuture<?> future = new NettyResponseFuture<>(request, new AsyncCompletionHandler<Object>() {
24+
@Override
25+
public Object onCompleted(org.asynchttpclient.Response response) throws Exception {
26+
return null;
27+
}
28+
}, null,
29+
0, ChannelPoolPartitioning.PerHostChannelPoolPartitioning.INSTANCE, null, null);
30+
31+
// create TimeoutsHolder without an original remote address
32+
TimeoutsHolder timeoutsHolder = new TimeoutsHolder(null, future, null, new DefaultAsyncHttpClientConfig.Builder().build(), null);
33+
34+
TimeoutTimerTask task = new TimeoutTimerTask(future, null, timeoutsHolder) {
35+
@Override
36+
public void run(io.netty.util.Timeout timeout) {
37+
// no-op
38+
}
39+
};
40+
41+
StringBuilder sb = new StringBuilder();
42+
task.appendRemoteAddress(sb);
43+
44+
// fallback should include URI host/port
45+
assertTrue(sb.toString().contains("example.com:12345"), sb.toString());
46+
}
47+
48+
@Test
49+
public void appendRemoteAddressShouldPrintResolvedAddressIfAvailable() {
50+
Request request = new RequestBuilder().setUrl("http://example.com:12345").build();
51+
NettyResponseFuture<?> future = new NettyResponseFuture<>(request, new AsyncCompletionHandler<Object>() {
52+
@Override
53+
public Object onCompleted(org.asynchttpclient.Response response) throws Exception {
54+
return null;
55+
}
56+
}, null,
57+
0, ChannelPoolPartitioning.PerHostChannelPoolPartitioning.INSTANCE, null, null);
58+
59+
TimeoutsHolder timeoutsHolder = new TimeoutsHolder(null, future, null, new DefaultAsyncHttpClientConfig.Builder().build(), null);
60+
61+
// set a resolved remote address
62+
timeoutsHolder.setResolvedRemoteAddress(new InetSocketAddress("127.0.0.1", 8080));
63+
64+
TimeoutTimerTask task = new TimeoutTimerTask(future, null, timeoutsHolder) {
65+
@Override
66+
public void run(io.netty.util.Timeout timeout) {
67+
// no-op
68+
}
69+
};
70+
71+
StringBuilder sb = new StringBuilder();
72+
task.appendRemoteAddress(sb);
73+
assertTrue(sb.toString().contains(":8080"), sb.toString());
74+
}
75+
}

0 commit comments

Comments
 (0)