Skip to content

Commit

Permalink
DnsNameResolver.resolve*(...) never notifies the Future when empty ho…
Browse files Browse the repository at this point in the history
…stname 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.
  • Loading branch information
normanmaurer committed Jan 24, 2017
1 parent 640ef61 commit a416b79
Show file tree
Hide file tree
Showing 7 changed files with 239 additions and 7 deletions.
16 changes: 16 additions & 0 deletions common/src/main/java/io/netty/util/internal/SocketUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,22 @@ public Enumeration<InetAddress> run() {
});
}

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[]>() {
Expand Down
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,11 @@ public final Future<InetAddress> resolve(String inetHost, Iterable<DnsRecord> ad
*/
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);
Expand Down Expand Up @@ -445,8 +448,13 @@ public final Future<List<InetAddress>> resolveAll(String inetHost, Iterable<DnsR
*/
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);
Expand Down Expand Up @@ -492,6 +500,12 @@ private static void validateAdditional(DnsRecord record, boolean validateType) {
}
}

@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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ public void operationComplete(Future<T> future) throws Exception {
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;
}
}
}

Expand Down Expand Up @@ -382,7 +384,7 @@ void tryToFinishResolve(Promise<T> promise) {
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;
}
}
Expand Down Expand Up @@ -509,12 +511,25 @@ private void followCname(InetSocketAddress nameServerAddr, String name, String c
}

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -496,6 +497,67 @@ public void testResolveIp() {
}
}

@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));
}
Expand Down
32 changes: 32 additions & 0 deletions resolver/src/main/java/io/netty/resolver/InetNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@

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;

/**
Expand All @@ -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);
}

/**
Expand Down
90 changes: 90 additions & 0 deletions resolver/src/test/java/io/netty/resolver/InetNameResolverTest.java
Original file line number Diff line number Diff line change
@@ -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());
}
}
}

0 comments on commit a416b79

Please sign in to comment.