Skip to content

Commit 33d9ced

Browse files
slandelleslandelle
committed
DnsNameResolver.resolve*(...) never notifies the Future when empty hostname is used, close AsyncHttpClient#1330
Motivation: When an empty hostname is used in DnsNameResolver.resolve*(...) it will never notify the future / promise. The root cause is that we not correctly guard against errors of IDN.toASCII(...) which will throw an IllegalArgumentException when it can not parse its input. That said we should also handle an empty hostname the same way as the JDK does and just use "localhost" when this happens. Modifications: - If the try to resolve an empty hostname we use localhost - Correctly guard against errors raised by IDN.toASCII(...) so we will always noify the future / promise - Add unit test. Result: DnsNameResolver.resolve*(...) will always notify the future.
1 parent 6b4f75d commit 33d9ced

File tree

5 files changed

+372
-8
lines changed

5 files changed

+372
-8
lines changed

‎netty-bp/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
import java.util.List;
6565
import java.util.Set;
6666

67-
import static io.netty.util.internal.ObjectUtil2.*;
67+
import static io.netty.util.internal.ObjectUtil.*;
6868

6969
/**
7070
* A DNS-based {@link InetNameResolver}.
@@ -411,8 +411,11 @@ public final Future<InetAddress> resolve(String inetHost, Iterable<DnsRecord> ad
411411
*/
412412
public final Future<InetAddress> resolve(String inetHost, Iterable<DnsRecord> additionals,
413413
Promise<InetAddress> promise) {
414-
checkNotNull(inetHost, "inetHost");
415414
checkNotNull(promise, "promise");
415+
if (inetHost == null || inetHost.isEmpty()) {
416+
// If an empty hostname is used we should use "localhost", just like InetAddress.getByName(...) does.
417+
return promise.setSuccess(loopbackAddress());
418+
}
416419
DnsRecord[] additionalsArray = toArray(additionals, true);
417420
try {
418421
doResolve(inetHost, additionalsArray, promise, resolveCache);
@@ -445,8 +448,13 @@ public final Future<List<InetAddress>> resolveAll(String inetHost, Iterable<DnsR
445448
*/
446449
public final Future<List<InetAddress>> resolveAll(String inetHost, Iterable<DnsRecord> additionals,
447450
Promise<List<InetAddress>> promise) {
448-
checkNotNull(inetHost, "inetHost");
449451
checkNotNull(promise, "promise");
452+
453+
if (inetHost == null || inetHost.isEmpty()) {
454+
// If an empty hostname is used we should use "localhost", just like InetAddress.getAllByName(...) does.
455+
return promise.setSuccess(Collections.singletonList(loopbackAddress()));
456+
}
457+
450458
DnsRecord[] additionalsArray = toArray(additionals, true);
451459
try {
452460
doResolveAll(inetHost, additionalsArray, promise, resolveCache);
@@ -492,6 +500,12 @@ private static void validateAdditional(DnsRecord record, boolean validateType) {
492500
}
493501
}
494502

503+
@Override
504+
protected final InetAddress loopbackAddress() {
505+
return preferredAddressType() == InternetProtocolFamily2.IPv4 ?
506+
NetUtil2.LOCALHOST4 : NetUtil2.LOCALHOST6;
507+
}
508+
495509
/**
496510
* Hook designed for extensibility so one can pass a different cache on each resolution attempt
497511
* instead of using the global one.

‎netty-bp/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ public void operationComplete(Future<T> future) throws Exception {
147147
private void internalResolve(Promise<T> promise) {
148148
InetSocketAddress nameServerAddrToTry = nameServerAddrs.next();
149149
for (DnsRecordType type: parent.resolveRecordTypes()) {
150-
query(nameServerAddrToTry, new DefaultDnsQuestion(hostname, type), promise);
150+
if (!query(hostname, type, nameServerAddrToTry, promise)) {
151+
return;
152+
}
151153
}
152154
}
153155

@@ -383,7 +385,7 @@ void tryToFinishResolve(Promise<T> promise) {
383385
if (!triedCNAME) {
384386
// As the last resort, try to query CNAME, just in case the name server has it.
385387
triedCNAME = true;
386-
query(nameServerAddrs.next(), new DefaultDnsQuestion(hostname, DnsRecordType.CNAME), promise);
388+
query(hostname, DnsRecordType.CNAME, nameServerAddrs.next(), promise);
387389
return;
388390
}
389391
}
@@ -510,12 +512,25 @@ private void followCname(InetSocketAddress nameServerAddr, String name, String c
510512
}
511513

512514
final InetSocketAddress nextAddr = nameServerAddrs.next();
513-
if (parent.isCnameFollowARecords()) {
514-
query(nextAddr, new DefaultDnsQuestion(cname, DnsRecordType.A), promise);
515+
if (parent.isCnameFollowARecords() && !query(hostname, DnsRecordType.A, nextAddr, promise)) {
516+
return;
515517
}
516518
if (parent.isCnameFollowAAAARecords()) {
517-
query(nextAddr, new DefaultDnsQuestion(cname, DnsRecordType.AAAA), promise);
519+
query(hostname, DnsRecordType.AAAA, nextAddr, promise);
520+
}
521+
}
522+
523+
private boolean query(String hostname, DnsRecordType type, final InetSocketAddress nextAddr, Promise<T> promise) {
524+
final DnsQuestion question;
525+
try {
526+
question = new DefaultDnsQuestion(hostname, type);
527+
} catch (IllegalArgumentException e) {
528+
// java.net.IDN.toASCII(...) may throw an IllegalArgumentException if it fails to parse the hostname
529+
promise.tryFailure(e);
530+
return false;
518531
}
532+
query(nextAddr, question, promise);
533+
return true;
519534
}
520535

521536
private void addTrace(InetSocketAddress nameServerAddr, String msg) {

‎netty-bp/resolver/src/main/java/io/netty/resolver/InetNameResolver.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,21 @@
1717

1818
import io.netty.util.concurrent.EventExecutor;
1919
import io.netty.util.concurrent.Future;
20+
import io.netty.util.concurrent.Promise;
21+
import io.netty.util.internal.SocketUtils2;
2022

2123
import java.net.InetAddress;
2224
import java.net.InetSocketAddress;
25+
import java.util.Collections;
26+
import java.util.List;
2327

2428
/**
2529
* A skeletal {@link NameResolver} implementation that resolves {@link InetAddress}.
2630
*/
2731
public abstract class InetNameResolver extends SimpleNameResolver<InetAddress> {
2832

33+
private final InetAddress loopbackAddress;
34+
2935
private volatile AddressResolver<InetSocketAddress> addressResolver;
3036

3137
/**
@@ -34,6 +40,32 @@ public abstract class InetNameResolver extends SimpleNameResolver<InetAddress> {
3440
*/
3541
protected InetNameResolver(EventExecutor executor) {
3642
super(executor);
43+
loopbackAddress = SocketUtils2.loopbackAddress();
44+
}
45+
46+
/**
47+
* Returns the {@link InetAddress} for loopback.
48+
*/
49+
protected InetAddress loopbackAddress() {
50+
return loopbackAddress;
51+
}
52+
53+
@Override
54+
public Future<InetAddress> resolve(String inetHost, Promise<InetAddress> promise) {
55+
if (inetHost == null || inetHost.isEmpty()) {
56+
// If an empty hostname is used we should use "localhost", just like InetAddress.getByName(...) does.
57+
return promise.setSuccess(loopbackAddress());
58+
}
59+
return super.resolve(inetHost, promise);
60+
}
61+
62+
@Override
63+
public Future<List<InetAddress>> resolveAll(String inetHost, Promise<List<InetAddress>> promise) {
64+
if (inetHost == null || inetHost.isEmpty()) {
65+
// If an empty hostname is used we should use "localhost", just like InetAddress.getByName(...) does.
66+
return promise.setSuccess(Collections.singletonList(loopbackAddress()));
67+
}
68+
return super.resolveAll(inetHost, promise);
3769
}
3870

3971
/**

‎netty-bp/resolver/src/main/java/io/netty/util/internal/SocketUtils2.java‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
/*
2+
* Copyright 2016 The Netty Project
3+
*
4+
* The Netty Project licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package io.netty.util.internal;
17+
18+
import java.io.IOException;
19+
import java.net.InetAddress;
20+
import java.net.InetSocketAddress;
21+
import java.net.NetworkInterface;
22+
import java.net.ServerSocket;
23+
import java.net.Socket;
24+
import java.net.SocketAddress;
25+
import java.net.SocketException;
26+
import java.net.SocketPermission;
27+
import java.net.UnknownHostException;
28+
import java.nio.channels.DatagramChannel;
29+
import java.nio.channels.ServerSocketChannel;
30+
import java.nio.channels.SocketChannel;
31+
import java.security.AccessController;
32+
import java.security.PrivilegedAction;
33+
import java.security.PrivilegedActionException;
34+
import java.security.PrivilegedExceptionAction;
35+
import java.util.Enumeration;
36+
37+
/**
38+
* Provides socket operations with privileges enabled. This is necessary for applications that use the
39+
* {@link SecurityManager} to restrict {@link SocketPermission} to their application. By asserting that these
40+
* operations are privileged, the operations can proceed even if some code in the calling chain lacks the appropriate
41+
* {@link SocketPermission}.
42+
*/
43+
public final class SocketUtils2 {
44+
45+
private SocketUtils2() {
46+
}
47+
48+
public static void connect(final Socket socket, final SocketAddress remoteAddress, final int timeout)
49+
throws IOException {
50+
try {
51+
AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
52+
@Override
53+
public Void run() throws IOException {
54+
socket.connect(remoteAddress, timeout);
55+
return null;
56+
}
57+
});
58+
} catch (PrivilegedActionException e) {
59+
throw (IOException) e.getCause();
60+
}
61+
}
62+
63+
public static void bind(final Socket socket, final SocketAddress bindpoint) throws IOException {
64+
try {
65+
AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
66+
@Override
67+
public Void run() throws IOException {
68+
socket.bind(bindpoint);
69+
return null;
70+
}
71+
});
72+
} catch (PrivilegedActionException e) {
73+
throw (IOException) e.getCause();
74+
}
75+
}
76+
77+
public static boolean connect(final SocketChannel socketChannel, final SocketAddress remoteAddress)
78+
throws IOException {
79+
try {
80+
return AccessController.doPrivileged(new PrivilegedExceptionAction<Boolean>() {
81+
@Override
82+
public Boolean run() throws IOException {
83+
return socketChannel.connect(remoteAddress);
84+
}
85+
});
86+
} catch (PrivilegedActionException e) {
87+
throw (IOException) e.getCause();
88+
}
89+
}
90+
91+
public static void bind(final SocketChannel socketChannel, final SocketAddress address) throws IOException {
92+
try {
93+
AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
94+
@Override
95+
public Void run() throws IOException {
96+
socketChannel.bind(address);
97+
return null;
98+
}
99+
});
100+
} catch (PrivilegedActionException e) {
101+
throw (IOException) e.getCause();
102+
}
103+
}
104+
105+
public static SocketChannel accept(final ServerSocketChannel serverSocketChannel) throws IOException {
106+
try {
107+
return AccessController.doPrivileged(new PrivilegedExceptionAction<SocketChannel>() {
108+
@Override
109+
public SocketChannel run() throws IOException {
110+
return serverSocketChannel.accept();
111+
}
112+
});
113+
} catch (PrivilegedActionException e) {
114+
throw (IOException) e.getCause();
115+
}
116+
}
117+
118+
public static void bind(final DatagramChannel networkChannel, final SocketAddress address) throws IOException {
119+
try {
120+
AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
121+
@Override
122+
public Void run() throws IOException {
123+
networkChannel.bind(address);
124+
return null;
125+
}
126+
});
127+
} catch (PrivilegedActionException e) {
128+
throw (IOException) e.getCause();
129+
}
130+
}
131+
132+
public static SocketAddress localSocketAddress(final ServerSocket socket) {
133+
return AccessController.doPrivileged(new PrivilegedAction<SocketAddress>() {
134+
@Override
135+
public SocketAddress run() {
136+
return socket.getLocalSocketAddress();
137+
}
138+
});
139+
}
140+
141+
public static InetAddress addressByName(final String hostname) throws UnknownHostException {
142+
try {
143+
return AccessController.doPrivileged(new PrivilegedExceptionAction<InetAddress>() {
144+
@Override
145+
public InetAddress run() throws UnknownHostException {
146+
return InetAddress.getByName(hostname);
147+
}
148+
});
149+
} catch (PrivilegedActionException e) {
150+
throw (UnknownHostException) e.getCause();
151+
}
152+
}
153+
154+
public static InetAddress[] allAddressesByName(final String hostname) throws UnknownHostException {
155+
try {
156+
return AccessController.doPrivileged(new PrivilegedExceptionAction<InetAddress[]>() {
157+
@Override
158+
public InetAddress[] run() throws UnknownHostException {
159+
return InetAddress.getAllByName(hostname);
160+
}
161+
});
162+
} catch (PrivilegedActionException e) {
163+
throw (UnknownHostException) e.getCause();
164+
}
165+
}
166+
167+
public static InetSocketAddress socketAddress(final String hostname, final int port) {
168+
return AccessController.doPrivileged(new PrivilegedAction<InetSocketAddress>() {
169+
@Override
170+
public InetSocketAddress run() {
171+
return new InetSocketAddress(hostname, port);
172+
}
173+
});
174+
}
175+
176+
public static Enumeration<InetAddress> addressesFromNetworkInterface(final NetworkInterface intf) {
177+
return AccessController.doPrivileged(new PrivilegedAction<Enumeration<InetAddress>>() {
178+
@Override
179+
public Enumeration<InetAddress> run() {
180+
return intf.getInetAddresses();
181+
}
182+
});
183+
}
184+
185+
public static InetAddress loopbackAddress() {
186+
return AccessController.doPrivileged(new PrivilegedAction<InetAddress>() {
187+
@Override
188+
public InetAddress run() {
189+
if (PlatformDependent.javaVersion() >= 7) {
190+
return InetAddress.getLoopbackAddress();
191+
}
192+
try {
193+
return InetAddress.getByName(null);
194+
} catch (UnknownHostException e) {
195+
throw new IllegalStateException(e);
196+
}
197+
}
198+
});
199+
}
200+
201+
public static byte[] hardwareAddressFromNetworkInterface(final NetworkInterface intf) throws SocketException {
202+
try {
203+
return AccessController.doPrivileged(new PrivilegedExceptionAction<byte[]>() {
204+
@Override
205+
public byte[] run() throws SocketException {
206+
return intf.getHardwareAddress();
207+
}
208+
});
209+
} catch (PrivilegedActionException e) {
210+
throw (SocketException) e.getCause();
211+
}
212+
}
213+
}

0 commit comments

Comments
 (0)