Skip to content

Commit 91b3f83

Browse files
committed
core: add javadoc and tests to enshrine some existing dns behavior
Update javadoc/tests to match reality w.r.t target URI path: path isn't actually the name to resolve, only the first path segment is actually used. This is a side effect of authority parsing later, in DnsNameResolver.java. Any additional path segments are silently ignored. Also fix an javadoc error regarding port: it isn't actually an input to the DNS layer, it's carried forward as a property of our output addresses at the io.grpc.NameResolver layer.
1 parent 5f48c44 commit 91b3f83

File tree

2 files changed

+40
-13
lines changed

2 files changed

+40
-13
lines changed

core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,18 @@
3131
* A provider for {@link DnsNameResolver}.
3232
*
3333
* <p>It resolves a target URI whose scheme is {@code "dns"}. The (optional) authority of the target
34-
* URI is reserved for the address of alternative DNS server (not implemented yet). The path of the
35-
* target URI, excluding the leading slash {@code '/'}, is treated as the host name and the optional
36-
* port to be resolved by DNS. Example target URIs:
34+
* URI is reserved for the address of alternative DNS server (not implemented yet). The first path
35+
* segment of the hierarchical target URI is interpreted as the RFC 2396 server-based authority of
36+
* the resulting {@link NameResolver} (see {@link NameResolver#getServiceAuthority()}). The "host"
37+
* part of this authority becomes the name to be resolved by DNS. The optional "port" part of this
38+
* authority is used as the port number of all {@link InetSocketAddress} produced by this resolver
39+
* (see also {@link NameResolver.Args#getDefaultPort()}). Example target URIs:
3740
*
3841
* <ul>
39-
* <li>{@code "dns:///foo.googleapis.com:8080"} (using default DNS)</li>
42+
* <li>{@code "dns:///foo.googleapis.com:8080"} (using default DNS)
4043
* <li>{@code "dns://8.8.8.8/foo.googleapis.com:8080"} (using alternative DNS (not implemented
41-
* yet))</li>
42-
* <li>{@code "dns:///foo.googleapis.com"} (without port)</li>
44+
* yet))
45+
* <li>{@code "dns:///foo.googleapis.com"} (without port)
4346
* </ul>
4447
*/
4548
public final class DnsNameResolverProvider extends NameResolverProvider {

core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616

1717
package io.grpc.internal;
1818

19-
import static org.junit.Assert.assertNull;
20-
import static org.junit.Assert.assertSame;
19+
import static com.google.common.truth.Truth.assertThat;
2120
import static org.junit.Assert.assertTrue;
2221
import static org.mockito.Mockito.mock;
2322

@@ -59,10 +58,35 @@ public void isAvailable() {
5958
}
6059

6160
@Test
62-
public void newNameResolver() {
63-
assertSame(DnsNameResolver.class,
64-
provider.newNameResolver(URI.create("dns:///localhost:443"), args).getClass());
65-
assertNull(
66-
provider.newNameResolver(URI.create("notdns:///localhost:443"), args));
61+
public void newNameResolver_acceptsHostAndPort() {
62+
NameResolver nameResolver = provider.newNameResolver(URI.create("dns:///localhost:443"), args);
63+
assertThat(nameResolver).isNotNull();
64+
assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
65+
assertThat(nameResolver.getServiceAuthority()).isEqualTo("localhost:443");
66+
}
67+
68+
@Test
69+
public void newNameResolver_rejectsNonDnsScheme() {
70+
NameResolver nameResolver =
71+
provider.newNameResolver(URI.create("notdns:///localhost:443"), args);
72+
assertThat(nameResolver).isNull();
73+
}
74+
75+
@Test
76+
public void newNameResolver_toleratesTrailingPathSegments() {
77+
NameResolver nameResolver =
78+
provider.newNameResolver(URI.create("dns:///foo.googleapis.com/ig/nor/ed"), args);
79+
assertThat(nameResolver).isNotNull();
80+
assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
81+
assertThat(nameResolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
82+
}
83+
84+
@Test
85+
public void newNameResolver_toleratesAuthority() {
86+
NameResolver nameResolver =
87+
provider.newNameResolver(URI.create("dns://8.8.8.8/foo.googleapis.com"), args);
88+
assertThat(nameResolver).isNotNull();
89+
assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
90+
assertThat(nameResolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
6791
}
6892
}

0 commit comments

Comments
 (0)