From 93d9b32d125e0907f412dba7c3637a3121450ad0 Mon Sep 17 00:00:00 2001 From: Carl Mastrangelo Date: Fri, 19 Oct 2018 12:19:36 -0700 Subject: [PATCH] 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. --- .../io/grpc/internal/DnsNameResolver.java | 30 +++++++- .../io/grpc/internal/DnsNameResolverTest.java | 77 +++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index fbd5762afe..f7a7cc33ed 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -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; + } } diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index a3aa7315cd..e75579e107 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -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);