core: ignore localhost and IP addresses for JNDI (1.16.x backport)

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.

2 files changed, 106 insertions, 1 deletions

diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java
index ccbce27db..dfa815525 100644
--- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java
+++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java
@@ -86,6 +86,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", "false");
   private static final String JNDI_TXT_PROPERTY =
@@ -107,6 +109,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);
@@ -230,7 +234,7 @@ final class DnsNameResolver extends NameResolver {
           ResolutionResults resolutionResults;
           try {
             ResourceResolver resourceResolver = null;
-            if (enableJndi) {
+            if (shouldUseJndi(enableJndi, enableJndiLocalhost, host)) {
               resourceResolver = getResourceResolver();
             }
             resolutionResults =
@@ -629,4 +633,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 6bcd3ce68..6f99cc3bc 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;
@@ -772,6 +773,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);