DnsNameResolver.resolve*(...) never notifies the Future when empty hostname is used.

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.
This commit is contained in:
Norman Maurer 2017-01-20 15:46:46 +01:00
parent 640ef615be
commit a416b79d86
7 changed files with 239 additions and 7 deletions

View File

@ -182,6 +182,22 @@ public final class SocketUtils {
});
}
public static InetAddress loopbackAddress() {
return AccessController.doPrivileged(new PrivilegedAction<InetAddress>() {
@Override
public InetAddress run() {
if (PlatformDependent.javaVersion() >= 7) {
return InetAddress.getLoopbackAddress();
}
try {
return InetAddress.getByName(null);
} catch (UnknownHostException e) {
throw new IllegalStateException(e);
}
}
});
}
public static byte[] hardwareAddressFromNetworkInterface(final NetworkInterface intf) throws SocketException {
try {
return AccessController.doPrivileged(new PrivilegedExceptionAction<byte[]>() {

View File

@ -757,6 +757,9 @@
<!-- JDK 9 -->
<ignore>java.nio.ByteBuffer</ignore>
<ignore>java.nio.CharBuffer</ignore>
<!-- Resolver -->
<ignore>java.net.InetAddress</ignore>
</ignores>
</configuration>
<executions>

View File

@ -411,8 +411,11 @@ public class DnsNameResolver extends InetNameResolver {
*/
public final Future<InetAddress> resolve(String inetHost, Iterable<DnsRecord> additionals,
Promise<InetAddress> promise) {
checkNotNull(inetHost, "inetHost");
checkNotNull(promise, "promise");
if (inetHost == null || inetHost.isEmpty()) {
// If an empty hostname is used we should use "localhost", just like InetAddress.getByName(...) does.
return promise.setSuccess(loopbackAddress());
}
DnsRecord[] additionalsArray = toArray(additionals, true);
try {
doResolve(inetHost, additionalsArray, promise, resolveCache);
@ -445,8 +448,13 @@ public class DnsNameResolver extends InetNameResolver {
*/
public final Future<List<InetAddress>> resolveAll(String inetHost, Iterable<DnsRecord> additionals,
Promise<List<InetAddress>> promise) {
checkNotNull(inetHost, "inetHost");
checkNotNull(promise, "promise");
if (inetHost == null || inetHost.isEmpty()) {
// If an empty hostname is used we should use "localhost", just like InetAddress.getAllByName(...) does.
return promise.setSuccess(Collections.singletonList(loopbackAddress()));
}
DnsRecord[] additionalsArray = toArray(additionals, true);
try {
doResolveAll(inetHost, additionalsArray, promise, resolveCache);
@ -492,6 +500,12 @@ public class DnsNameResolver extends InetNameResolver {
}
}
@Override
protected final InetAddress loopbackAddress() {
return preferredAddressType() == InternetProtocolFamily.IPv4 ?
NetUtil.LOCALHOST4 : NetUtil.LOCALHOST6;
}
/**
* Hook designed for extensibility so one can pass a different cache on each resolution attempt
* instead of using the global one.

View File

@ -146,7 +146,9 @@ abstract class DnsNameResolverContext<T> {
private void internalResolve(Promise<T> promise) {
InetSocketAddress nameServerAddrToTry = nameServerAddrs.next();
for (DnsRecordType type: parent.resolveRecordTypes()) {
query(nameServerAddrToTry, new DefaultDnsQuestion(hostname, type), promise);
if (!query(hostname, type, nameServerAddrToTry, promise)) {
return;
}
}
}
@ -382,7 +384,7 @@ abstract class DnsNameResolverContext<T> {
if (!triedCNAME) {
// As the last resort, try to query CNAME, just in case the name server has it.
triedCNAME = true;
query(nameServerAddrs.next(), new DefaultDnsQuestion(hostname, DnsRecordType.CNAME), promise);
query(hostname, DnsRecordType.CNAME, nameServerAddrs.next(), promise);
return;
}
}
@ -509,14 +511,27 @@ abstract class DnsNameResolverContext<T> {
}
final InetSocketAddress nextAddr = nameServerAddrs.next();
if (parent.isCnameFollowARecords()) {
query(nextAddr, new DefaultDnsQuestion(cname, DnsRecordType.A), promise);
if (parent.isCnameFollowARecords() && !query(hostname, DnsRecordType.A, nextAddr, promise)) {
return;
}
if (parent.isCnameFollowAAAARecords()) {
query(nextAddr, new DefaultDnsQuestion(cname, DnsRecordType.AAAA), promise);
query(hostname, DnsRecordType.AAAA, nextAddr, promise);
}
}
private boolean query(String hostname, DnsRecordType type, final InetSocketAddress nextAddr, Promise<T> promise) {
final DnsQuestion question;
try {
question = new DefaultDnsQuestion(hostname, type);
} catch (IllegalArgumentException e) {
// java.net.IDN.toASCII(...) may throw an IllegalArgumentException if it fails to parse the hostname
promise.tryFailure(e);
return false;
}
query(nextAddr, question, promise);
return true;
}
private void addTrace(InetSocketAddress nameServerAddr, String msg) {
assert traceEnabled;

View File

@ -28,6 +28,7 @@ import io.netty.handler.codec.dns.DnsRecordType;
import io.netty.handler.codec.dns.DnsResponse;
import io.netty.handler.codec.dns.DnsResponseCode;
import io.netty.handler.codec.dns.DnsSection;
import io.netty.util.NetUtil;
import io.netty.util.concurrent.Future;
import io.netty.util.internal.StringUtil;
import io.netty.util.internal.logging.InternalLogger;
@ -496,6 +497,67 @@ public class DnsNameResolverTest {
}
}
@Test
public void testResolveEmptyIpv4() {
testResolve0(InternetProtocolFamily.IPv4, NetUtil.LOCALHOST4, StringUtil.EMPTY_STRING);
}
@Test
public void testResolveEmptyIpv6() {
testResolve0(InternetProtocolFamily.IPv6, NetUtil.LOCALHOST6, StringUtil.EMPTY_STRING);
}
@Test
public void testResolveNullIpv4() {
testResolve0(InternetProtocolFamily.IPv4, NetUtil.LOCALHOST4, null);
}
@Test
public void testResolveNullIpv6() {
testResolve0(InternetProtocolFamily.IPv6, NetUtil.LOCALHOST6, null);
}
private static void testResolve0(InternetProtocolFamily family, InetAddress expectedAddr, String name) {
DnsNameResolver resolver = newResolver(family).build();
try {
InetAddress address = resolver.resolve(name).syncUninterruptibly().getNow();
assertEquals(expectedAddr, address);
} finally {
resolver.close();
}
}
@Test
public void testResolveAllEmptyIpv4() {
testResolveAll0(InternetProtocolFamily.IPv4, NetUtil.LOCALHOST4, StringUtil.EMPTY_STRING);
}
@Test
public void testResolveAllEmptyIpv6() {
testResolveAll0(InternetProtocolFamily.IPv6, NetUtil.LOCALHOST6, StringUtil.EMPTY_STRING);
}
@Test
public void testResolveAllNullIpv4() {
testResolveAll0(InternetProtocolFamily.IPv4, NetUtil.LOCALHOST4, null);
}
@Test
public void testResolveAllNullIpv6() {
testResolveAll0(InternetProtocolFamily.IPv6, NetUtil.LOCALHOST6, null);
}
private static void testResolveAll0(InternetProtocolFamily family, InetAddress expectedAddr, String name) {
DnsNameResolver resolver = newResolver(family).build();
try {
List<InetAddress> addresses = resolver.resolveAll(name).syncUninterruptibly().getNow();
assertEquals(1, addresses.size());
assertEquals(expectedAddr, addresses.get(0));
} finally {
resolver.close();
}
}
private static void resolve(DnsNameResolver resolver, Map<String, Future<InetAddress>> futures, String hostname) {
futures.put(hostname, resolver.resolve(hostname));
}

View File

@ -17,15 +17,21 @@ package io.netty.resolver;
import io.netty.util.concurrent.EventExecutor;
import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.Promise;
import io.netty.util.internal.SocketUtils;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.Collections;
import java.util.List;
/**
* A skeletal {@link NameResolver} implementation that resolves {@link InetAddress}.
*/
public abstract class InetNameResolver extends SimpleNameResolver<InetAddress> {
private final InetAddress loopbackAddress;
private volatile AddressResolver<InetSocketAddress> addressResolver;
/**
@ -34,6 +40,32 @@ public abstract class InetNameResolver extends SimpleNameResolver<InetAddress> {
*/
protected InetNameResolver(EventExecutor executor) {
super(executor);
loopbackAddress = SocketUtils.loopbackAddress();
}
/**
* Returns the {@link InetAddress} for loopback.
*/
protected InetAddress loopbackAddress() {
return loopbackAddress;
}
@Override
public Future<InetAddress> resolve(String inetHost, Promise<InetAddress> promise) {
if (inetHost == null || inetHost.isEmpty()) {
// If an empty hostname is used we should use "localhost", just like InetAddress.getByName(...) does.
return promise.setSuccess(loopbackAddress());
}
return super.resolve(inetHost, promise);
}
@Override
public Future<List<InetAddress>> resolveAll(String inetHost, Promise<List<InetAddress>> promise) {
if (inetHost == null || inetHost.isEmpty()) {
// If an empty hostname is used we should use "localhost", just like InetAddress.getByName(...) does.
return promise.setSuccess(Collections.singletonList(loopbackAddress()));
}
return super.resolveAll(inetHost, promise);
}
/**

View File

@ -0,0 +1,90 @@
/*@
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you 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 io.netty.resolver;
import io.netty.util.concurrent.ImmediateEventExecutor;
import io.netty.util.concurrent.Promise;
import io.netty.util.internal.SocketUtils;
import io.netty.util.internal.StringUtil;
import org.junit.Test;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.List;
import static org.junit.Assert.assertEquals;
public class InetNameResolverTest {
@Test
public void testResolveEmpty() throws UnknownHostException {
testResolve0(SocketUtils.addressByName(StringUtil.EMPTY_STRING), StringUtil.EMPTY_STRING);
}
@Test
public void testResolveNull() throws UnknownHostException {
testResolve0(SocketUtils.addressByName(null), null);
}
@Test
public void testResolveAllEmpty() throws UnknownHostException {
testResolveAll0(Arrays.asList(
SocketUtils.allAddressesByName(StringUtil.EMPTY_STRING)), StringUtil.EMPTY_STRING);
}
@Test
public void testResolveAllNull() throws UnknownHostException {
testResolveAll0(Arrays.asList(
SocketUtils.allAddressesByName(null)), null);
}
private static void testResolve0(InetAddress expectedAddr, String name) {
InetNameResolver resolver = new TestInetNameResolver();
try {
InetAddress address = resolver.resolve(name).syncUninterruptibly().getNow();
assertEquals(expectedAddr, address);
} finally {
resolver.close();
}
}
private static void testResolveAll0(List<InetAddress> expectedAddrs, String name) {
InetNameResolver resolver = new TestInetNameResolver();
try {
List<InetAddress> addresses = resolver.resolveAll(name).syncUninterruptibly().getNow();
assertEquals(expectedAddrs, addresses);
} finally {
resolver.close();
}
}
private static final class TestInetNameResolver extends InetNameResolver {
public TestInetNameResolver() {
super(ImmediateEventExecutor.INSTANCE);
}
@Override
protected void doResolve(String inetHost, Promise<InetAddress> promise) throws Exception {
promise.setFailure(new UnsupportedOperationException());
}
@Override
protected void doResolveAll(String inetHost, Promise<List<InetAddress>> promise) throws Exception {
promise.setFailure(new UnsupportedOperationException());
}
}
}