Skip to content

Commit

Permalink
DNS Resolve ambiguity in which DNS servers are used during resolution
Browse files Browse the repository at this point in the history
Motivation:
Recently DnsServerAddressStreamProvider was introduced to allow control for each query as to which DNS server should be used for resolution to respect the local host's default DNS server configuration. However resolver-dns also accepts a stream of DNS servers to use by default, but this stream is not host name aware. This creates an ambiguity as to which method is used to determine the DNS server to user during resolution, and in which order. We can remove this ambiguity and provide a more general API by just supporting DnsServerAddressStreamProvider.

Modifications:
- Remove the fixed DnsServerAddresses and instead only accept a DnsServerAddressStreamProvider.
- Add utility methods to help use DnsServerAddressStreamProvider for a single entry, a list of entries, and get the default for the current machine.

Result:
Fixes netty#6573.
  • Loading branch information
Scottmitch authored and kiril-me committed Feb 28, 2018
1 parent 4f465db commit e5d3152
Show file tree
Hide file tree
Showing 14 changed files with 205 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@

import io.netty.util.internal.UnstableApi;

import static io.netty.resolver.dns.DnsServerAddresses.defaultAddresses;

/**
* A {@link DnsServerAddressStreamProvider} which will use predefined default DNS servers to use for DNS resolution.
* These defaults do not respect your host's machines defaults.
*/
@UnstableApi
public final class NoopDnsServerAddressStreamProvider implements DnsServerAddressStreamProvider {
public static final NoopDnsServerAddressStreamProvider INSTANCE = new NoopDnsServerAddressStreamProvider();
public final class DefaultDnsServerAddressStreamProvider implements DnsServerAddressStreamProvider {
public static final DefaultDnsServerAddressStreamProvider INSTANCE = new DefaultDnsServerAddressStreamProvider();

private NoopDnsServerAddressStreamProvider() {
private DefaultDnsServerAddressStreamProvider() {
}

@Override
public DnsServerAddressStream nameServerAddressStream(String hostname) {
return null;
return defaultAddresses().stream();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,22 @@
public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddress> {

private final ChannelFactory<? extends DatagramChannel> channelFactory;
private final DnsServerAddresses nameServerAddresses;
private final DnsServerAddressStreamProvider nameServerProvider;

private final ConcurrentMap<String, Promise<InetAddress>> resolvesInProgress = newConcurrentHashMap();
private final ConcurrentMap<String, Promise<List<InetAddress>>> resolveAllsInProgress = newConcurrentHashMap();

public DnsAddressResolverGroup(
Class<? extends DatagramChannel> channelType,
DnsServerAddresses nameServerAddresses) {
this(new ReflectiveChannelFactory<DatagramChannel>(channelType), nameServerAddresses);
DnsServerAddressStreamProvider nameServerProvider) {
this(new ReflectiveChannelFactory<DatagramChannel>(channelType), nameServerProvider);
}

public DnsAddressResolverGroup(
ChannelFactory<? extends DatagramChannel> channelFactory,
DnsServerAddresses nameServerAddresses) {
DnsServerAddressStreamProvider nameServerProvider) {
this.channelFactory = channelFactory;
this.nameServerAddresses = nameServerAddresses;
this.nameServerProvider = nameServerProvider;
}

@SuppressWarnings("deprecation")
Expand All @@ -70,20 +70,20 @@ protected final AddressResolver<InetSocketAddress> newResolver(EventExecutor exe
" (expected: " + StringUtil.simpleClassName(EventLoop.class));
}

return newResolver((EventLoop) executor, channelFactory, nameServerAddresses);
return newResolver((EventLoop) executor, channelFactory, nameServerProvider);
}

/**
* @deprecated Override {@link #newNameResolver(EventLoop, ChannelFactory, DnsServerAddresses)}.
* @deprecated Override {@link #newNameResolver(EventLoop, ChannelFactory, DnsServerAddressStreamProvider)}.
*/
@Deprecated
protected AddressResolver<InetSocketAddress> newResolver(
EventLoop eventLoop, ChannelFactory<? extends DatagramChannel> channelFactory,
DnsServerAddresses nameServerAddresses) throws Exception {
DnsServerAddressStreamProvider nameServerProvider) throws Exception {

final NameResolver<InetAddress> resolver = new InflightNameResolver<InetAddress>(
eventLoop,
newNameResolver(eventLoop, channelFactory, nameServerAddresses),
newNameResolver(eventLoop, channelFactory, nameServerProvider),
resolvesInProgress,
resolveAllsInProgress);

Expand All @@ -96,10 +96,11 @@ protected AddressResolver<InetSocketAddress> newResolver(
*/
protected NameResolver<InetAddress> newNameResolver(EventLoop eventLoop,
ChannelFactory<? extends DatagramChannel> channelFactory,
DnsServerAddresses nameServerAddresses) throws Exception {
DnsServerAddressStreamProvider nameServerProvider)
throws Exception {
return new DnsNameResolverBuilder(eventLoop)
.channelFactory(channelFactory)
.nameServerAddresses(nameServerAddresses)
.nameServerProvider(nameServerProvider)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ public class DnsNameResolver extends InetNameResolver {
private static final DatagramDnsResponseDecoder DECODER = new DatagramDnsResponseDecoder();
private static final DatagramDnsQueryEncoder ENCODER = new DatagramDnsQueryEncoder();

final DnsServerAddresses nameServerAddresses;
final Future<Channel> channelFuture;
final DatagramChannel ch;

Expand All @@ -151,7 +150,7 @@ public class DnsNameResolver extends InetNameResolver {
new FastThreadLocal<DnsServerAddressStream>() {
@Override
protected DnsServerAddressStream initialValue() throws Exception {
return nameServerAddresses.stream();
return dnsServerAddressStreamProvider.nameServerAddressStream("");
}
};

Expand All @@ -178,9 +177,6 @@ protected DnsServerAddressStream initialValue() throws Exception {
*
* @param eventLoop the {@link EventLoop} which will perform the communication with the DNS servers
* @param channelFactory the {@link ChannelFactory} that will create a {@link DatagramChannel}
* @param nameServerAddresses the addresses of the DNS server. For each DNS query, a new stream is created from
* this to determine which DNS server should be contacted for the next retry in case
* of failure.
* @param resolveCache the DNS resolved entries cache
* @param authoritativeDnsServerCache the cache used to find the authoritative DNS server for a domain
* @param queryTimeoutMillis timeout of each DNS query in millis
Expand All @@ -191,7 +187,7 @@ protected DnsServerAddressStream initialValue() throws Exception {
* @param maxPayloadSize the capacity of the datagram packet buffer
* @param optResourceEnabled if automatic inclusion of a optional records is enabled
* @param hostsFileEntriesResolver the {@link HostsFileEntriesResolver} used to check for local aliases
* @param dnsServerAddressStreamProvider The {@link DnsServerAddressStreamProvider} used to override the name
* @param dnsServerAddressStreamProvider The {@link DnsServerAddressStreamProvider} used to determine the name
* servers for each hostname lookup.
* @param searchDomains the list of search domain
* @param ndots the ndots value
Expand All @@ -201,7 +197,6 @@ protected DnsServerAddressStream initialValue() throws Exception {
public DnsNameResolver(
EventLoop eventLoop,
ChannelFactory<? extends DatagramChannel> channelFactory,
DnsServerAddresses nameServerAddresses,
final DnsCache resolveCache,
DnsCache authoritativeDnsServerCache,
long queryTimeoutMillis,
Expand All @@ -218,7 +213,6 @@ public DnsNameResolver(
boolean decodeIdn) {
super(eventLoop);
checkNotNull(channelFactory, "channelFactory");
this.nameServerAddresses = checkNotNull(nameServerAddresses, "nameServerAddresses");
this.queryTimeoutMillis = checkPositive(queryTimeoutMillis, "queryTimeoutMillis");
this.resolvedAddressTypes = resolvedAddressTypes != null ? resolvedAddressTypes : DEFAULT_RESOLVE_ADDRESS_TYPES;
this.recursionDesired = recursionDesired;
Expand Down Expand Up @@ -636,12 +630,8 @@ private void doResolveUncached(String hostname,
DnsRecord[] additionals,
Promise<InetAddress> promise,
DnsCache resolveCache) {
DnsServerAddressStream dnsServerAddressStream =
dnsServerAddressStreamProvider.nameServerAddressStream(hostname);
SingleResolverContext ctx = dnsServerAddressStream == null ?
new SingleResolverContext(this, hostname, additionals, resolveCache, nameServerAddresses.stream()) :
new SingleResolverContext(this, hostname, additionals, resolveCache, dnsServerAddressStream);
ctx.resolve(promise);
new SingleResolverContext(this, hostname, additionals, resolveCache,
dnsServerAddressStreamProvider.nameServerAddressStream(hostname)).resolve(promise);
}

static final class SingleResolverContext extends DnsNameResolverContext<InetAddress> {
Expand Down Expand Up @@ -797,12 +787,8 @@ private void doResolveAllUncached(String hostname,
DnsRecord[] additionals,
Promise<List<InetAddress>> promise,
DnsCache resolveCache) {
DnsServerAddressStream dnsServerAddressStream =
dnsServerAddressStreamProvider.nameServerAddressStream(hostname);
ListResolverContext ctx = dnsServerAddressStream == null ?
new ListResolverContext(this, hostname, additionals, resolveCache, nameServerAddresses.stream()) :
new ListResolverContext(this, hostname, additionals, resolveCache, dnsServerAddressStream);
ctx.resolve(promise);
new ListResolverContext(this, hostname, additionals, resolveCache,
dnsServerAddressStreamProvider.nameServerAddressStream(hostname)).resolve(promise);
}

private static String hostname(String inetHost) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.ArrayList;
import java.util.List;

import static io.netty.resolver.dns.DnsServerAddressStreamProviders.platformDefault;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static io.netty.util.internal.ObjectUtil.intValue;

Expand All @@ -35,14 +36,8 @@
*/
@UnstableApi
public final class DnsNameResolverBuilder {
// TODO(scott): how is this done on Windows? This may require a JNI call to GetNetworkParams
// https://msdn.microsoft.com/en-us/library/aa365968(VS.85).aspx.
private static final DnsServerAddressStreamProvider DEFAULT_DNS_SERVER_ADDRESS_STREAM_PROVIDER =
UnixResolverDnsServerAddressStreamProvider.parseSilently();

private final EventLoop eventLoop;
private ChannelFactory<? extends DatagramChannel> channelFactory;
private DnsServerAddresses nameServerAddresses = DnsServerAddresses.defaultAddresses();
private DnsCache resolveCache;
private DnsCache authoritativeDnsServerCache;
private Integer minTtl;
Expand All @@ -56,7 +51,7 @@ public final class DnsNameResolverBuilder {
private int maxPayloadSize = 4096;
private boolean optResourceEnabled = true;
private HostsFileEntriesResolver hostsFileEntriesResolver = HostsFileEntriesResolver.DEFAULT;
private DnsServerAddressStreamProvider dnsServerAddressStreamProvider = DEFAULT_DNS_SERVER_ADDRESS_STREAM_PROVIDER;
private DnsServerAddressStreamProvider dnsServerAddressStreamProvider = platformDefault();
private String[] searchDomains = DnsNameResolver.DEFAULT_SEARCH_DOMAINS;
private int ndots = 1;
private boolean decodeIdn = true;
Expand Down Expand Up @@ -93,17 +88,6 @@ public DnsNameResolverBuilder channelType(Class<? extends DatagramChannel> chann
return channelFactory(new ReflectiveChannelFactory<DatagramChannel>(channelType));
}

/**
* Sets the addresses of the DNS server.
*
* @param nameServerAddresses the DNS server addresses
* @return {@code this}
*/
public DnsNameResolverBuilder nameServerAddresses(DnsServerAddresses nameServerAddresses) {
this.nameServerAddresses = nameServerAddresses;
return this;
}

/**
* Sets the cache for resolution results.
*
Expand Down Expand Up @@ -282,7 +266,7 @@ public DnsNameResolverBuilder hostsFileEntriesResolver(HostsFileEntriesResolver
* each hostname.
* @return {@code this}.
*/
public DnsNameResolverBuilder nameServerCache(DnsServerAddressStreamProvider dnsServerAddressStreamProvider) {
public DnsNameResolverBuilder nameServerProvider(DnsServerAddressStreamProvider dnsServerAddressStreamProvider) {
this.dnsServerAddressStreamProvider =
checkNotNull(dnsServerAddressStreamProvider, "dnsServerAddressStreamProvider");
return this;
Expand Down Expand Up @@ -364,7 +348,6 @@ public DnsNameResolver build() {
return new DnsNameResolver(
eventLoop,
channelFactory,
nameServerAddresses,
resolveCache,
authoritativeDnsServerCache,
queryTimeoutMillis,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.FutureListener;
import io.netty.util.concurrent.Promise;
import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.StringUtil;

import java.net.IDN;
Expand Down Expand Up @@ -96,7 +97,7 @@ protected DnsNameResolverContext(DnsNameResolver parent,
this.additionals = additionals;
this.resolveCache = resolveCache;

this.nameServerAddrs = nameServerAddrs;
this.nameServerAddrs = ObjectUtil.checkNotNull(nameServerAddrs, "nameServerAddrs");
maxAllowedQueries = parent.maxQueriesPerResolve();
resolvedInternetProtocolFamilies = parent.resolvedInternetProtocolFamiliesUnsafe();
traceEnabled = parent.isTraceEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ public interface DnsServerAddressStreamProvider {
/**
* Ask this provider for the name servers to query for {@code hostname}.
* @param hostname The hostname for which to lookup the DNS server addressed to use.
* @return The {@link DnsServerAddressStream} which should be used to resolve {@code hostname} or {@code null} to
* use the default resolvers.
* If this is the final {@link DnsServerAddressStreamProvider} to be queried then generally empty
* string or {@code '.'} correspond to the default {@link DnsServerAddressStream}.
* @return The {@link DnsServerAddressStream} which should be used to resolve {@code hostname}.
*/
DnsServerAddressStream nameServerAddressStream(String hostname);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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.dns;

import io.netty.util.internal.UnstableApi;

/**
* Utility methods related to {@link DnsServerAddressStreamProvider}.
*/
@UnstableApi
public final class DnsServerAddressStreamProviders {
// TODO(scott): how is this done on Windows? This may require a JNI call to GetNetworkParams
// https://msdn.microsoft.com/en-us/library/aa365968(VS.85).aspx.
private static final DnsServerAddressStreamProvider DEFAULT_DNS_SERVER_ADDRESS_STREAM_PROVIDER =
UnixResolverDnsServerAddressStreamProvider.parseSilently();

private DnsServerAddressStreamProviders() {
}

/**
* A {@link DnsServerAddressStreamProvider} which inherits the DNS servers from your local host's configuration.
* <p>
* Note that only macOS and Linux are currently supported.
* @return A {@link DnsServerAddressStreamProvider} which inherits the DNS servers from your local host's
* configuration.
*/
public static DnsServerAddressStreamProvider platformDefault() {
return DEFAULT_DNS_SERVER_ADDRESS_STREAM_PROVIDER;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* 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.dns;

import io.netty.util.internal.UnstableApi;

import java.util.List;

/**
* A {@link DnsServerAddressStreamProvider} which iterates through a collection of
* {@link DnsServerAddressStreamProvider} until the first non-{@code null} result is found.
*/
@UnstableApi
public final class MultiDnsServerAddressStreamProvider implements DnsServerAddressStreamProvider {
private final DnsServerAddressStreamProvider[] providers;

/**
* Create a new instance.
* @param providers The providers to use for DNS resolution. They will be queried in order.
*/
public MultiDnsServerAddressStreamProvider(List<DnsServerAddressStreamProvider> providers) {
this.providers = providers.toArray(new DnsServerAddressStreamProvider[0]);
}

/**
* Create a new instance.
* @param providers The providers to use for DNS resolution. They will be queried in order.
*/
public MultiDnsServerAddressStreamProvider(DnsServerAddressStreamProvider... providers) {
this.providers = providers.clone();
}

@Override
public DnsServerAddressStream nameServerAddressStream(String hostname) {
for (DnsServerAddressStreamProvider provider : providers) {
DnsServerAddressStream stream = provider.nameServerAddressStream(hostname);
if (stream != null) {
return stream;
}
}
return null;
}
}
Loading

0 comments on commit e5d3152

Please sign in to comment.