core: ignore localhost and IP addresses for JNDI

This change is mainly to fix a test, but it also is an implementation of the proposal here: https://github.com/grpc/proposal/pull/79

In short:

* Do not do SRV or TXT lookups when the target name is `localhost`.  This can be overriden by a system property
* Do not do SRV or TXT lookups when the target name is an IPv6 or IPv4 address.  This _cannot_ be overriden.  The constructed domains for these queries would themselves not be valid.  (e.g. _grpclb._tcp.192.168.0.1)
* Speeds up initial connection when communicating over local host, since it is extremely uncommon that such a connection would need gRPCLB or SRV records

I expect to remove the system property after a release if no one asks about it.
This commit is contained in:
Carl Mastrangelo 2018-10-19 12:19:36 -07:00 committed by GitHub
parent a2ef4c0dbf
commit 93d9b32d12
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 106 additions and 1 deletions

View File

@ -87,6 +87,8 @@ final class DnsNameResolver extends NameResolver {
private static final String JNDI_PROPERTY =
System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_jndi", "true");
private static final String JNDI_LOCALHOST_PROPERTY =
System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_jndi_localhost", "false");
private static final String JNDI_SRV_PROPERTY =
System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_grpclb", "true");
private static final String JNDI_TXT_PROPERTY =
@ -110,6 +112,8 @@ final class DnsNameResolver extends NameResolver {
@VisibleForTesting
static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY);
@VisibleForTesting
static boolean enableJndiLocalhost = Boolean.parseBoolean(JNDI_LOCALHOST_PROPERTY);
@VisibleForTesting
static boolean enableSrv = Boolean.parseBoolean(JNDI_SRV_PROPERTY);
@VisibleForTesting
static boolean enableTxt = Boolean.parseBoolean(JNDI_TXT_PROPERTY);
@ -259,7 +263,7 @@ final class DnsNameResolver extends NameResolver {
ResolutionResults resolutionResults;
try {
ResourceResolver resourceResolver = null;
if (enableJndi) {
if (shouldUseJndi(enableJndi, enableJndiLocalhost, resolver.host)) {
resourceResolver = resolver.getResourceResolver();
}
resolutionResults = resolveAll(
@ -683,4 +687,28 @@ final class DnsNameResolver extends NameResolver {
}
return localHostname;
}
@VisibleForTesting
static boolean shouldUseJndi(boolean jndiEnabled, boolean jndiLocalhostEnabled, String target) {
if (!jndiEnabled) {
return false;
}
if ("localhost".equalsIgnoreCase(target)) {
return jndiLocalhostEnabled;
}
// Check if this name looks like IPv6
if (target.contains(":")) {
return false;
}
// Check if this might be IPv4. Such addresses have no alphabetic characters. This also
// checks the target is empty.
boolean alldigits = true;
for (int i = 0; i < target.length(); i++) {
char c = target.charAt(i);
if (c != '.') {
alldigits &= (c >= '0' && c <= '9');
}
}
return !alldigits;
}
}

View File

@ -18,6 +18,7 @@ package io.grpc.internal;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
@ -827,6 +828,82 @@ public class DnsNameResolverTest {
assertNotNull(DnsNameResolver.maybeChooseServiceConfig(choice, new Random(), "localhost"));
}
@Test
public void shouldUseJndi_alwaysFalseIfDisabled() {
boolean enableJndi = false;
boolean enableJndiLocalhost = true;
String host = "seemingly.valid.host";
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, host));
}
@Test
public void shouldUseJndi_falseIfDisabledForLocalhost() {
boolean enableJndi = true;
boolean enableJndiLocalhost = false;
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "localhost"));
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "LOCALHOST"));
}
@Test
public void shouldUseJndi_trueIfLocalhostOverriden() {
boolean enableJndi = true;
boolean enableJndiLocalhost = true;
String host = "localhost";
assertTrue(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, host));
}
@Test
public void shouldUseJndi_falseForIpv6() {
boolean enableJndi = true;
boolean enableJndiLocalhost = false;
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "::"));
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "::1"));
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "2001:db8:1234::"));
assertFalse(DnsNameResolver.shouldUseJndi(
enableJndi, enableJndiLocalhost, "[2001:db8:1234::]"));
assertFalse(DnsNameResolver.shouldUseJndi(
enableJndi, enableJndiLocalhost, "2001:db8:1234::%3"));
}
@Test
public void shouldUseJndi_falseForIpv4() {
boolean enableJndi = true;
boolean enableJndiLocalhost = false;
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "127.0.0.1"));
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "192.168.0.1"));
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "134744072"));
}
@Test
public void shouldUseJndi_falseForEmpty() {
boolean enableJndi = true;
boolean enableJndiLocalhost = false;
assertFalse(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, ""));
}
@Test
public void shouldUseJndi_trueIfItMightPossiblyBeValid() {
boolean enableJndi = true;
boolean enableJndiLocalhost = false;
assertTrue(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "remotehost"));
assertTrue(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "remotehost.gov"));
assertTrue(DnsNameResolver.shouldUseJndi(enableJndi, enableJndiLocalhost, "f.q.d.n."));
assertTrue(DnsNameResolver.shouldUseJndi(
enableJndi, enableJndiLocalhost, "8.8.8.8.in-addr.arpa."));
assertTrue(DnsNameResolver.shouldUseJndi(
enableJndi, enableJndiLocalhost, "2001-db8-1234--as3.ipv6-literal.net"));
}
private void testInvalidUri(URI uri) {
try {
provider.newNameResolver(uri, NAME_RESOLVER_PARAMS);