Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect UnixResolverDnsServerAddressStreamProvider #6736

Closed
trustin opened this issue May 12, 2017 · 4 comments
Closed

Incorrect UnixResolverDnsServerAddressStreamProvider #6736

trustin opened this issue May 12, 2017 · 4 comments
Assignees
Labels
Milestone

Comments

@trustin
Copy link
Member

trustin commented May 12, 2017

Minimal yet complete reproducer code (or URL to code)

import static org.junit.Assert.assertNotNull;

import java.io.File;
import java.io.FileOutputStream;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import io.netty.resolver.dns.UnixResolverDnsServerAddressStreamProvider;

public class ResolverTest {

    @Rule
    public final TemporaryFolder folder = new TemporaryFolder();

    @Test
    public void test() throws Exception {
        File f = folder.newFile();
        try (OutputStream out = new FileOutputStream(f)) {
            out.write(("domain linecorp.local\n" +
                       "nameserver 1.2.3.4\n" +
                       "nameserver 5.6.7.8\n").getBytes(StandardCharsets.UTF_8));
        }
        UnixResolverDnsServerAddressStreamProvider p =
                new UnixResolverDnsServerAddressStreamProvider(f, null);

        assertNotNull(p.nameServerAddressStream("somehost"));
    }
}

Expected behavior

UnixResolverDnsServerAddressStreamProvider.nameServerAddressStream() should not return null.

The expected behavior of DnsNameResolver is to try somehost. and then somehost.linecorp.local.

Actual behavior

Attempting to resolve a host will trigger a NPE:

java.lang.NullPointerException: nameServerAddrs
 at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:31)
 at io.netty.resolver.dns.DnsNameResolverContext.<init>(DnsNameResolverContext.java:120)
 at io.netty.resolver.dns.DnsNameResolver$SingleResolverContext.<init>(DnsNameResolver.java:659)
 at io.netty.resolver.dns.DnsNameResolver.doResolveUncached(DnsNameResolver.java:653)
 at io.netty.resolver.dns.DnsNameResolver.doResolve(DnsNameResolver.java:590)
 at io.netty.resolver.dns.DnsNameResolver.doResolve(DnsNameResolver.java:523)
 at io.netty.resolver.SimpleNameResolver.resolve(SimpleNameResolver.java:63)
 at io.netty.resolver.dns.InflightNameResolver.resolve(InflightNameResolver.java:100)
 at io.netty.resolver.dns.InflightNameResolver.resolve(InflightNameResolver.java:66)
 at io.netty.resolver.dns.InflightNameResolver.resolve(InflightNameResolver.java:51)
 at io.netty.resolver.InetSocketAddressResolver.doResolve(InetSocketAddressResolver.java:57)
 at io.netty.resolver.InetSocketAddressResolver.doResolve(InetSocketAddressResolver.java:32)
 at io.netty.resolver.AbstractAddressResolver.resolve(AbstractAddressResolver.java:108)
 at io.netty.bootstrap.Bootstrap.doResolveAndConnect0(Bootstrap.java:208)
 at io.netty.bootstrap.Bootstrap.doResolveAndConnect(Bootstrap.java:170)
 at io.netty.bootstrap.Bootstrap.connect(Bootstrap.java:145)

Netty version

4.1.11

Workaround

Use DefaultDnsServerAddressStreamProvider.INSTANCE instead of DnsServerAddressStreamProviders.platformDefault().

@trustin trustin added the defect label May 12, 2017
@normanmaurer
Copy link
Member

@trustin can you submit a pr with a fix ?

@trustin
Copy link
Member Author

trustin commented May 12, 2017

Can't promise it at the moment because of busyness... 😢

@Scottmitch
Copy link
Member

The expected behavior of DnsNameResolver is to try somehost and then somehost.linecorp.local.

The behavior of modifying the hostname should be provided at a higher layer (e.g. DnsNameResolverContext.java). However UnixResolverDnsServerAddressStreamProvider default value when no match was found needs some work.

@Scottmitch
Copy link
Member

See PR #6748

Scottmitch added a commit to Scottmitch/netty that referenced this issue May 18, 2017
…ion and ordering bug

Motivation:
UnixResolverDnsServerAddressStreamProvider allows the default name server address stream to be null, but there should always be a default stream to fall back to ([1] Search Strategy).
UnixResolverDnsServerAddressStreamProvider currently shuffles the names servers are multiple are present, but the defined behavior is to try them sequentially [2].

[1] Search Strategy Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
[2] DESCRIPTION/nameserver Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html

Modifications:
- UnixResolverDnsServerAddressStreamProvider should always use the first file provided to derive the default domain server address stream. Currently if there are multiple domain names in the file identified by the first argument of the constructor then one will be selected at random.
- UnixResolverDnsServerAddressStreamProvider should return name servers sequentially.
- Reduce access level on some methods which don't have known use-cases externally.

Result:
Fixes netty#6736
liuzhengyang pushed a commit to liuzhengyang/netty that referenced this issue Sep 10, 2017
…ion and ordering bug

Motivation:
UnixResolverDnsServerAddressStreamProvider allows the default name server address stream to be null, but there should always be a default stream to fall back to ([1] Search Strategy).
UnixResolverDnsServerAddressStreamProvider currently shuffles the names servers are multiple are present, but the defined behavior is to try them sequentially [2].

[1] Search Strategy Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
[2] DESCRIPTION/nameserver Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html

Modifications:
- UnixResolverDnsServerAddressStreamProvider should always use the first file provided to derive the default domain server address stream. Currently if there are multiple domain names in the file identified by the first argument of the constructor then one will be selected at random.
- UnixResolverDnsServerAddressStreamProvider should return name servers sequentially.
- Reduce access level on some methods which don't have known use-cases externally.

Result:
Fixes netty#6736
kiril-me pushed a commit to kiril-me/netty that referenced this issue Feb 28, 2018
…ion and ordering bug

Motivation:
UnixResolverDnsServerAddressStreamProvider allows the default name server address stream to be null, but there should always be a default stream to fall back to ([1] Search Strategy).
UnixResolverDnsServerAddressStreamProvider currently shuffles the names servers are multiple are present, but the defined behavior is to try them sequentially [2].

[1] Search Strategy Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
[2] DESCRIPTION/nameserver Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html

Modifications:
- UnixResolverDnsServerAddressStreamProvider should always use the first file provided to derive the default domain server address stream. Currently if there are multiple domain names in the file identified by the first argument of the constructor then one will be selected at random.
- UnixResolverDnsServerAddressStreamProvider should return name servers sequentially.
- Reduce access level on some methods which don't have known use-cases externally.

Result:
Fixes netty#6736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants