From a74bb350b750ddeabdf9d16c6ef86fc3cc4653c5 Mon Sep 17 00:00:00 2001 From: Carl Mastrangelo Date: Tue, 26 Jun 2018 16:15:52 -0700 Subject: [PATCH] core: always do SRV record lookup in DnsNameResolver Instead of failing after a a missing A/AAAA record, this change makes the resolver keep going and try out SRV records too. This is needed for use with ALTS, and is part of the gRPCLB spec. This change also moved the JNDI code to a separate, reflectively loaded file. This makes it easy to exclude the file and not worry about the missing class references on Android. Additionally, if javax.naming might be available on Android, this allows it to be loaded. A key side effect of this is that DnsNameResolver is smaller, and more cleanly tested. --- .../io/grpc/internal/DnsNameResolver.java | 383 +++++++----------- .../internal/JndiResourceResolverFactory.java | 259 ++++++++++++ .../io/grpc/internal/DnsNameResolverTest.java | 235 +++++------ .../internal/JndiResourceResolverTest.java | 77 ++++ 4 files changed, 592 insertions(+), 362 deletions(-) create mode 100644 core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java create mode 100644 core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index 0c8ce689ee..b0e301c0a0 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -27,9 +27,9 @@ import io.grpc.NameResolver; import io.grpc.Status; import io.grpc.internal.SharedResourceHolder.Resource; import java.io.IOException; +import java.lang.reflect.Constructor; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.SocketAddress; import java.net.URI; import java.net.UnknownHostException; import java.util.ArrayList; @@ -42,15 +42,11 @@ import java.util.Map.Entry; import java.util.Random; import java.util.Set; import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Pattern; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; -import javax.naming.NamingEnumeration; -import javax.naming.NamingException; -import javax.naming.directory.Attribute; -import javax.naming.directory.InitialDirContext; /** * A DNS-based {@link NameResolver}. @@ -64,8 +60,6 @@ final class DnsNameResolver extends NameResolver { private static final Logger logger = Logger.getLogger(DnsNameResolver.class.getName()); - private static final boolean JNDI_AVAILABLE = jndiAvailable(); - private static final String SERVICE_CONFIG_CHOICE_CLIENT_LANGUAGE_KEY = "clientLanguage"; private static final String SERVICE_CONFIG_CHOICE_PERCENTAGE_KEY = "percentage"; private static final String SERVICE_CONFIG_CHOICE_CLIENT_HOSTNAME_KEY = "clientHostname"; @@ -93,6 +87,8 @@ final class DnsNameResolver extends NameResolver { @VisibleForTesting static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY); + private static final ResourceResolverFactory resourceResolverFactory = + getResourceResolverFactory(DnsNameResolver.class.getClassLoader()); @VisibleForTesting final ProxyDetector proxyDetector; @@ -102,7 +98,9 @@ final class DnsNameResolver extends NameResolver { private final Random random = new Random(); - private DelegateResolver delegateResolver = pickDelegateResolver(); + private volatile AddressResolver addressResolver = JdkAddressResolver.INSTANCE; + private final AtomicReference resourceResolver = + new AtomicReference(); private final String authority; private final String host; @@ -195,9 +193,14 @@ final class DnsNameResolver extends NameResolver { savedListener.onAddresses(Collections.singletonList(server), Attributes.EMPTY); return; } - ResolutionResults resolvedInetAddrs; + + ResolutionResults resolutionResults; try { - resolvedInetAddrs = delegateResolver.resolve(host); + ResourceResolver resourceResolver = null; + if (enableJndi) { + resourceResolver = getResourceResolver(); + } + resolutionResults = resolveAll(addressResolver, resourceResolver, host); } catch (Exception e) { savedListener.onError( Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e)); @@ -205,17 +208,17 @@ final class DnsNameResolver extends NameResolver { } // Each address forms an EAG List servers = new ArrayList(); - for (InetAddress inetAddr : resolvedInetAddrs.addresses) { + for (InetAddress inetAddr : resolutionResults.addresses) { servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port))); } - servers.addAll(resolvedInetAddrs.balancerAddresses); + servers.addAll(resolutionResults.balancerAddresses); Attributes.Builder attrs = Attributes.newBuilder(); - if (!resolvedInetAddrs.txtRecords.isEmpty()) { + if (!resolutionResults.txtRecords.isEmpty()) { Map serviceConfig = null; try { for (Map possibleConfig : - parseTxtResults(resolvedInetAddrs.txtRecords)) { + parseTxtResults(resolutionResults.txtRecords)) { try { serviceConfig = maybeChooseServiceConfig(possibleConfig, random, getLocalHostname()); @@ -267,20 +270,52 @@ final class DnsNameResolver extends NameResolver { return port; } - private DelegateResolver pickDelegateResolver() { - JdkResolver jdkResolver = new JdkResolver(); - if (JNDI_AVAILABLE && enableJndi) { - return new CompositeResolver(jdkResolver, new JndiResolver()); - } - return jdkResolver; - } - - /** - * Forces the resolver. This should only be used by testing code. - */ @VisibleForTesting - void setDelegateResolver(DelegateResolver delegateResolver) { - this.delegateResolver = delegateResolver; + static ResolutionResults resolveAll( + AddressResolver addressResolver, @Nullable ResourceResolver resourceResolver, String name) { + List addresses = Collections.emptyList(); + Exception addressesException = null; + List balancerAddresses = Collections.emptyList(); + Exception balancerAddressesException = null; + List txtRecords = Collections.emptyList(); + Exception txtRecordsException = null; + + try { + addresses = addressResolver.resolveAddress(name); + } catch (Exception e) { + addressesException = e; + } + if (resourceResolver != null) { + try { + balancerAddresses = resourceResolver.resolveSrv(addressResolver, GRPCLB_NAME_PREFIX + name); + } catch (Exception e) { + balancerAddressesException = e; + } + // Only do the TXT record lookup if one of the above address resolutions succeeded. + if (!(balancerAddressesException != null && addressesException != null)) { + try { + txtRecords = resourceResolver.resolveTxt(SERVICE_CONFIG_NAME_PREFIX + name); + } catch (Exception e) { + txtRecordsException = e; + } + } + } + try { + if (addressesException != null && balancerAddressesException != null) { + throw new RuntimeException(addressesException); + } + } finally { + if (addressesException != null) { + logger.log(Level.FINE, "Address resolution failure", addressesException); + } + if (balancerAddressesException != null) { + logger.log(Level.FINE, "Balancer resolution failure", balancerAddressesException); + } + if (txtRecordsException != null) { + logger.log(Level.FINE, "ServiceConfig resolution failure", txtRecordsException); + } + } + return new ResolutionResults(addresses, txtRecords, balancerAddresses); } @SuppressWarnings("unchecked") @@ -397,48 +432,17 @@ final class DnsNameResolver extends NameResolver { return ServiceConfigUtil.getObject(choice, SERVICE_CONFIG_CHOICE_SERVICE_CONFIG_KEY); } - /** - * Returns whether the JNDI DNS resolver is available. This is accomplished by looking up a - * particular class. It is believed to be the default (only?) DNS resolver that will actually be - * used. It is provided by the OpenJDK, but unlikely Android. Actual resolution will be done by - * using a service provider when a hostname query is present, so the {@code DnsContextFactory} - * may not actually be used to perform the query. This is believed to be "okay." - */ - @VisibleForTesting - @SuppressWarnings("LiteralClassName") - static boolean jndiAvailable() { - if (GrpcUtil.IS_RESTRICTED_APPENGINE) { - return false; - } - try { - Class.forName("javax.naming.directory.InitialDirContext"); - Class.forName("com.sun.jndi.dns.DnsContextFactory"); - } catch (ClassNotFoundException e) { - logger.log(Level.FINE, "Unable to find JNDI DNS resolver, skipping", e); - return false; - } - return true; - } - - /** - * Common interface between the delegate resolvers used by DnsNameResolver. - */ - @VisibleForTesting - abstract static class DelegateResolver { - abstract ResolutionResults resolve(String host) throws Exception; - } - /** * Describes the results from a DNS query. */ @VisibleForTesting static final class ResolutionResults { - final List addresses; + final List addresses; final List txtRecords; final List balancerAddresses; ResolutionResults( - List addresses, + List addresses, List txtRecords, List balancerAddresses) { this.addresses = Collections.unmodifiableList(checkNotNull(addresses, "addresses")); @@ -448,196 +452,103 @@ final class DnsNameResolver extends NameResolver { } } - /** - * A composite DNS resolver that uses both the JDK and JNDI resolvers as delegate. It is - * expected that two DNS queries will be executed, with the second one being from JNDI. - */ @VisibleForTesting - static final class CompositeResolver extends DelegateResolver { + void setAddressResolver(AddressResolver addressResolver) { + this.addressResolver = addressResolver; + } - private final DelegateResolver jdkResovler; - private final DelegateResolver jndiResovler; + /** + * {@link ResourceResolverFactory} is a factory for making resource resolvers. It supports + * optionally checking if the factory is available. + */ + interface ResourceResolverFactory { - CompositeResolver(DelegateResolver jdkResovler, DelegateResolver jndiResovler) { - this.jdkResovler = jdkResovler; - this.jndiResovler = jndiResovler; - } + /** + * Creates a new resource resolver. The return value is {@code null} iff + * {@link #unavailabilityCause()} is not null; + */ + @Nullable ResourceResolver newResourceResolver(); + + /** + * Returns the reason why the resource resolver cannot be created. The return value is + * {@code null} if {@link #newResourceResolver()} is suitable for use. + */ + @Nullable Throwable unavailabilityCause(); + } + + /** + * AddressResolver resolves a hostname into a list of addresses. + */ + interface AddressResolver { + List resolveAddress(String host) throws Exception; + } + + private enum JdkAddressResolver implements AddressResolver { + INSTANCE; @Override - ResolutionResults resolve(String host) throws Exception { - ResolutionResults jdkResults = jdkResovler.resolve(host); - List addresses = jdkResults.addresses; - List txtRecords = Collections.emptyList(); - List balancerAddresses = Collections.emptyList(); - try { - ResolutionResults jdniResults = jndiResovler.resolve(host); - txtRecords = jdniResults.txtRecords; - balancerAddresses = jdniResults.balancerAddresses; - } catch (Throwable e) { - // JndiResolver.resolve may throw Error that could cause rpc to hang. - // Catch and log Throwable and keep using jdkResolver's result to prevent it. - logger.log(Level.SEVERE, "Failed to resolve TXT results", e); - } - - return new ResolutionResults(addresses, txtRecords, balancerAddresses); + public List resolveAddress(String host) throws UnknownHostException { + return Collections.unmodifiableList(Arrays.asList(InetAddress.getAllByName(host))); } } /** - * The default name resolver provided with the JDK. This is unable to lookup TXT records, but - * provides address ordering sorted according to RFC 3484. This is true on OpenJDK, because it - * in turn calls into libc which sorts addresses in order of reachability. + * {@link ResourceResolver} is a Dns ResourceRecord resolver. */ + interface ResourceResolver { + List resolveTxt(String host) throws Exception; + + List resolveSrv( + AddressResolver addressResolver, String host) throws Exception; + } + + @Nullable + private ResourceResolver getResourceResolver() { + ResourceResolver rr; + if ((rr = resourceResolver.get()) == null) { + if (resourceResolverFactory != null) { + assert resourceResolverFactory.unavailabilityCause() == null; + rr = resourceResolverFactory.newResourceResolver(); + } + } + return rr; + } + + @Nullable + @SuppressWarnings("unchecked") @VisibleForTesting - static final class JdkResolver extends DelegateResolver { - - @Override - ResolutionResults resolve(String host) throws Exception { - return new ResolutionResults( - Arrays.asList(InetAddress.getAllByName(host)), - Collections.emptyList(), - Collections.emptyList()); + static ResourceResolverFactory getResourceResolverFactory(ClassLoader loader) { + Class jndiClazz; + try { + jndiClazz = + (Class) + Class.forName("io.grpc.internal.JndiResourceResolverFactory", true, loader); + assert ResourceResolverFactory.class.isAssignableFrom(jndiClazz); + } catch (ClassNotFoundException e) { + logger.log(Level.FINE, "Unable to find JndiResourceResolverFactory, skipping.", e); + return null; } - } - - /** - * A resolver that uses JNDI. This class is capable of looking up both addresses - * and text records, but does not provide ordering guarantees. It is currently not used for - * address resolution. - */ - @VisibleForTesting - static final class JndiResolver extends DelegateResolver { - - private static final Pattern whitespace = Pattern.compile("\\s+"); - - @SuppressWarnings("BetaApi") // Verify is stable in Guava 23.5 - @Override - ResolutionResults resolve(String host) throws NamingException { - List serviceConfigTxtRecords = Collections.emptyList(); - String serviceConfigHostname = SERVICE_CONFIG_NAME_PREFIX + host; - if (logger.isLoggable(Level.FINER)) { - logger.log( - Level.FINER, "About to query TXT records for {0}", new Object[]{serviceConfigHostname}); - } - try { - serviceConfigTxtRecords = getAllRecords("TXT", "dns:///" + serviceConfigHostname); - } catch (NamingException e) { - - if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, "Unable to look up " + serviceConfigHostname, e); - } - } - - String grpclbHostname = GRPCLB_NAME_PREFIX + host; - if (logger.isLoggable(Level.FINER)) { - logger.log( - Level.FINER, "About to query SRV records for {0}", new Object[]{grpclbHostname}); - } - List balancerAddresses = Collections.emptyList(); - try { - List grpclbSrvRecords = getAllRecords("SRV", "dns:///" + grpclbHostname); - balancerAddresses = new ArrayList(grpclbSrvRecords.size()); - for (String srvRecord : grpclbSrvRecords) { - try { - String[] parts = whitespace.split(srvRecord); - Verify.verify(parts.length == 4, "Bad SRV Record: %s, ", srvRecord); - String srvHostname = parts[3]; - int port = Integer.parseInt(parts[2]); - - InetAddress[] addrs = InetAddress.getAllByName(srvHostname); - List sockaddrs = new ArrayList(addrs.length); - for (InetAddress addr : addrs) { - sockaddrs.add(new InetSocketAddress(addr, port)); - } - Attributes attrs = Attributes.newBuilder() - .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, srvHostname) - .build(); - balancerAddresses.add( - new EquivalentAddressGroup(Collections.unmodifiableList(sockaddrs), attrs)); - } catch (UnknownHostException e) { - logger.log(Level.WARNING, "Can't find address for SRV record" + srvRecord, e); - } catch (RuntimeException e) { - logger.log(Level.WARNING, "Failed to construct SRV record" + srvRecord, e); - } - } - } catch (NamingException e) { - if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, "Unable to look up " + serviceConfigHostname, e); - } - } - - return new ResolutionResults( - /*addresses=*/ Collections.emptyList(), - serviceConfigTxtRecords, - Collections.unmodifiableList(balancerAddresses)); + Constructor jndiCtor; + try { + jndiCtor = jndiClazz.getConstructor(); + } catch (Exception e) { + logger.log(Level.FINE, "Can't find JndiResourceResolverFactory ctor, skipping.", e); + return null; } - - private List getAllRecords(String recordType, String name) throws NamingException { - InitialDirContext dirContext = new InitialDirContext(); - String[] rrType = new String[]{recordType}; - javax.naming.directory.Attributes attrs = dirContext.getAttributes(name, rrType); - List records = new ArrayList(); - - NamingEnumeration rrGroups = attrs.getAll(); - try { - while (rrGroups.hasMore()) { - Attribute rrEntry = rrGroups.next(); - assert Arrays.asList(rrType).contains(rrEntry.getID()); - NamingEnumeration rrValues = rrEntry.getAll(); - try { - while (rrValues.hasMore()) { - records.add(normalizeData(recordType, String.valueOf(rrValues.next()))); - } - } finally { - rrValues.close(); - } - } - } finally { - rrGroups.close(); - } - return records; + ResourceResolverFactory rrf; + try { + rrf = jndiCtor.newInstance(); + } catch (Exception e) { + logger.log(Level.FINE, "Can't construct JndiResourceResolverFactory, skipping.", e); + return null; } - } - - /** - * Convert returned RR data to a form that's consumable by the grpc library. - */ - @VisibleForTesting - static String normalizeData(String recordType, String rrData) { - String normalized = rrData; - if (recordType.equals("TXT")) { - normalized = unquote(normalized); + if (rrf.unavailabilityCause() != null) { + logger.log( + Level.FINE, + "JndiResourceResolverFactory not available, skipping.", + rrf.unavailabilityCause()); } - return normalized; - } - - /** - * Undo the quoting done in {@link com.sun.jndi.dns.ResourceRecord#decodeTxt}. - */ - static String unquote(String txtRecord) { - StringBuilder sb = new StringBuilder(txtRecord.length()); - boolean inquote = false; - for (int i = 0; i < txtRecord.length(); i++) { - char c = txtRecord.charAt(i); - if (!inquote) { - if (c == ' ') { - continue; - } else if (c == '"') { - inquote = true; - continue; - } - } else { - if (c == '"') { - inquote = false; - continue; - } else if (c == '\\') { - c = txtRecord.charAt(++i); - assert c == '"' || c == '\\'; - } - } - sb.append(c); - } - return sb.toString(); + return rrf; } private static String getLocalHostname() { diff --git a/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java b/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java new file mode 100644 index 0000000000..997ace361a --- /dev/null +++ b/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java @@ -0,0 +1,259 @@ +/* + * Copyright 2018 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Verify; +import io.grpc.Attributes; +import io.grpc.EquivalentAddressGroup; +import io.grpc.internal.DnsNameResolver.AddressResolver; +import io.grpc.internal.DnsNameResolver.ResourceResolver; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Pattern; +import javax.annotation.Nullable; +import javax.naming.NamingEnumeration; +import javax.naming.NamingException; +import javax.naming.directory.Attribute; +import javax.naming.directory.DirContext; +import javax.naming.directory.InitialDirContext; + +/** + * {@link JndiResourceResolverFactory} resolves additional records for the DnsNameResolver. + */ +final class JndiResourceResolverFactory implements DnsNameResolver.ResourceResolverFactory { + + @Nullable + private static final Throwable JNDI_UNAVAILABILITY_CAUSE = initJndi(); + + // @UsedReflectively + public JndiResourceResolverFactory() {} + + /** + * Returns whether the JNDI DNS resolver is available. This is accomplished by looking up a + * particular class. It is believed to be the default (only?) DNS resolver that will actually be + * used. It is provided by the OpenJDK, but unlikely Android. Actual resolution will be done by + * using a service provider when a hostname query is present, so the {@code DnsContextFactory} + * may not actually be used to perform the query. This is believed to be "okay." + */ + @Nullable + @SuppressWarnings("LiteralClassName") + private static Throwable initJndi() { + if (GrpcUtil.IS_RESTRICTED_APPENGINE) { + return new UnsupportedOperationException( + "Currently running in an AppEngine restricted environment"); + } + try { + Class.forName("javax.naming.directory.InitialDirContext"); + Class.forName("com.sun.jndi.dns.DnsContextFactory"); + } catch (ClassNotFoundException e) { + return e; + } catch (RuntimeException e) { + return e; + } catch (Error e) { + return e; + } + return null; + } + + @Nullable + @Override + public ResourceResolver newResourceResolver() { + if (unavailabilityCause() != null) { + return null; + } + return new JndiResourceResolver(); + } + + @Nullable + @Override + public Throwable unavailabilityCause() { + return JNDI_UNAVAILABILITY_CAUSE; + } + + @VisibleForTesting + static final class JndiResourceResolver implements DnsNameResolver.ResourceResolver { + private static final Logger logger = + Logger.getLogger(JndiResourceResolver.class.getName()); + + private static final Pattern whitespace = Pattern.compile("\\s+"); + + @Override + public List resolveTxt(String serviceConfigHostname) throws NamingException { + checkAvailable(); + if (logger.isLoggable(Level.FINER)) { + logger.log( + Level.FINER, "About to query TXT records for {0}", new Object[]{serviceConfigHostname}); + } + List serviceConfigRawTxtRecords = + getAllRecords(new InitialDirContext(), "TXT", "dns:///" + serviceConfigHostname); + if (logger.isLoggable(Level.FINER)) { + logger.log( + Level.FINER, "Found {0} TXT records", new Object[]{serviceConfigRawTxtRecords.size()}); + } + List serviceConfigTxtRecords = + new ArrayList(serviceConfigRawTxtRecords.size()); + for (String serviceConfigRawTxtRecord : serviceConfigRawTxtRecords) { + serviceConfigTxtRecords.add(unquote(serviceConfigRawTxtRecord)); + } + return Collections.unmodifiableList(serviceConfigTxtRecords); + } + + @Override + public List resolveSrv( + AddressResolver addressResolver, String grpclbHostname) throws Exception { + checkAvailable(); + if (logger.isLoggable(Level.FINER)) { + logger.log( + Level.FINER, "About to query SRV records for {0}", new Object[]{grpclbHostname}); + } + List grpclbSrvRecords = + getAllRecords(new InitialDirContext(), "SRV", "dns:///" + grpclbHostname); + if (logger.isLoggable(Level.FINER)) { + logger.log( + Level.FINER, "Found {0} SRV records", new Object[]{grpclbSrvRecords.size()}); + } + List balancerAddresses = + new ArrayList(grpclbSrvRecords.size()); + Exception first = null; + Level level = Level.WARNING; + for (String srvRecord : grpclbSrvRecords) { + try { + SrvRecord record = parseSrvRecord(srvRecord); + + List addrs = addressResolver.resolveAddress(record.host); + List sockaddrs = new ArrayList(addrs.size()); + for (InetAddress addr : addrs) { + sockaddrs.add(new InetSocketAddress(addr, record.port)); + } + Attributes attrs = Attributes.newBuilder() + .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, record.host) + .build(); + balancerAddresses.add( + new EquivalentAddressGroup(Collections.unmodifiableList(sockaddrs), attrs)); + } catch (UnknownHostException e) { + logger.log(level, "Can't find address for SRV record " + srvRecord, e); + // TODO(carl-mastrangelo): these should be added by addSuppressed when we have Java 7. + if (first == null) { + first = e; + level = Level.FINE; + } + } catch (RuntimeException e) { + logger.log(level, "Failed to construct SRV record " + srvRecord, e); + if (first == null) { + first = e; + level = Level.FINE; + } + } + } + if (balancerAddresses.isEmpty() && first != null) { + throw first; + } + return Collections.unmodifiableList(balancerAddresses); + } + + @VisibleForTesting + static final class SrvRecord { + SrvRecord(String host, int port) { + this.host = host; + this.port = port; + } + + final String host; + final int port; + } + + @VisibleForTesting + @SuppressWarnings("BetaApi") // Verify is only kinda beta + static SrvRecord parseSrvRecord(String rawRecord) { + String[] parts = whitespace.split(rawRecord); + Verify.verify(parts.length == 4, "Bad SRV Record: %s", rawRecord); + return new SrvRecord(parts[3], Integer.parseInt(parts[2])); + } + + private static List getAllRecords( + DirContext dirContext, String recordType, String name) throws NamingException { + String[] rrType = new String[]{recordType}; + javax.naming.directory.Attributes attrs = dirContext.getAttributes(name, rrType); + List records = new ArrayList(); + + NamingEnumeration rrGroups = attrs.getAll(); + try { + while (rrGroups.hasMore()) { + Attribute rrEntry = rrGroups.next(); + assert Arrays.asList(rrType).contains(rrEntry.getID()); + NamingEnumeration rrValues = rrEntry.getAll(); + try { + while (rrValues.hasMore()) { + records.add(String.valueOf(rrValues.next())); + } + } finally { + rrValues.close(); + } + } + } finally { + rrGroups.close(); + } + return records; + } + + /** + * Undo the quoting done in {@link com.sun.jndi.dns.ResourceRecord#decodeTxt}. + */ + @VisibleForTesting + static String unquote(String txtRecord) { + StringBuilder sb = new StringBuilder(txtRecord.length()); + boolean inquote = false; + for (int i = 0; i < txtRecord.length(); i++) { + char c = txtRecord.charAt(i); + if (!inquote) { + if (c == ' ') { + continue; + } else if (c == '"') { + inquote = true; + continue; + } + } else { + if (c == '"') { + inquote = false; + continue; + } else if (c == '\\') { + c = txtRecord.charAt(++i); + assert c == '"' || c == '\\'; + } + } + sb.append(c); + } + return sb.toString(); + } + + private static void checkAvailable() { + if (JNDI_UNAVAILABILITY_CAUSE != null) { + throw new UnsupportedOperationException( + "JNDI is not currently available", JNDI_UNAVAILABILITY_CAUSE); + } + } + } +} diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 97de00801e..129b951bfa 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -25,45 +25,46 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import com.google.common.base.MoreObjects; import com.google.common.collect.Iterables; +import com.google.common.net.InetAddresses; import io.grpc.Attributes; import io.grpc.EquivalentAddressGroup; import io.grpc.NameResolver; -import io.grpc.internal.DnsNameResolver.DelegateResolver; +import io.grpc.internal.DnsNameResolver.AddressResolver; import io.grpc.internal.DnsNameResolver.ResolutionResults; +import io.grpc.internal.DnsNameResolver.ResourceResolver; +import io.grpc.internal.DnsNameResolver.ResourceResolverFactory; import io.grpc.internal.SharedResourceHolder.Resource; +import java.net.Inet4Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.net.URI; import java.net.UnknownHostException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Queue; import java.util.Random; import java.util.concurrent.ExecutorService; import org.junit.After; -import org.junit.Assume; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.DisableOnDebug; import org.junit.rules.ExpectedException; +import org.junit.rules.TestRule; import org.junit.rules.Timeout; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; import org.mockito.Captor; +import org.mockito.Matchers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -71,7 +72,7 @@ import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class DnsNameResolverTest { - @Rule public final Timeout globalTimeout = Timeout.seconds(10); + @Rule public final TestRule globalTimeout = new DisableOnDebug(Timeout.seconds(10)); @Rule public final ExpectedException thrown = ExpectedException.none(); @@ -85,7 +86,6 @@ public class DnsNameResolverTest { private final DnsNameResolverProvider provider = new DnsNameResolverProvider(); private final FakeClock fakeClock = new FakeClock(); private final FakeClock fakeExecutor = new FakeClock(); - private MockResolver mockResolver = new MockResolver(); private final Resource fakeExecutorResource = new Resource() { @@ -105,13 +105,12 @@ public class DnsNameResolverTest { private ArgumentCaptor> resultCaptor; private DnsNameResolver newResolver(String name, int port) { - return newResolver(name, port, mockResolver, GrpcUtil.NOOP_PROXY_DETECTOR); + return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR); } private DnsNameResolver newResolver( String name, int port, - DelegateResolver delegateResolver, ProxyDetector proxyDetector) { DnsNameResolver dnsResolver = new DnsNameResolver( null, @@ -119,7 +118,6 @@ public class DnsNameResolverTest { Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(), fakeExecutorResource, proxyDetector); - dnsResolver.setDelegateResolver(delegateResolver); return dnsResolver; } @@ -159,97 +157,136 @@ public class DnsNameResolverTest { @Test public void resolve() throws Exception { - List answer1 = createAddressList(2); - List answer2 = createAddressList(1); + final List answer1 = createAddressList(2); + final List answer2 = createAddressList(1); String name = "foo.googleapis.com"; DnsNameResolver resolver = newResolver(name, 81); - mockResolver.addAnswer(answer1).addAnswer(answer2); + AddressResolver mockResolver = mock(AddressResolver.class); + when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2); + resolver.setAddressResolver(mockResolver); + resolver.start(mockListener); assertEquals(1, fakeExecutor.runDueTasks()); verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class)); - assertEquals(name, mockResolver.invocations.poll()); assertAnswerMatches(answer1, 81, resultCaptor.getValue()); assertEquals(0, fakeClock.numPendingTasks()); resolver.refresh(); assertEquals(1, fakeExecutor.runDueTasks()); verify(mockListener, times(2)).onAddresses(resultCaptor.capture(), any(Attributes.class)); - assertEquals(name, mockResolver.invocations.poll()); assertAnswerMatches(answer2, 81, resultCaptor.getValue()); assertEquals(0, fakeClock.numPendingTasks()); resolver.shutdown(); + + verify(mockResolver, times(2)).resolveAddress(Matchers.anyString()); } @Test - public void jdkResolverWorks() throws Exception { - DnsNameResolver.DelegateResolver resolver = new DnsNameResolver.JdkResolver(); + public void resolveAll_nullResourceResolver() throws Exception { + final String hostname = "addr.fake"; + final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); - ResolutionResults results = resolver.resolve("localhost"); - // Just check that *something* came back. - assertThat(results.addresses).isNotEmpty(); - assertThat(results.txtRecords).isNotNull(); + AddressResolver mockResolver = mock(AddressResolver.class); + when(mockResolver.resolveAddress(Matchers.anyString())) + .thenReturn(Collections.singletonList(backendAddr)); + ResourceResolver resourceResolver = null; + + ResolutionResults res = DnsNameResolver.resolveAll(mockResolver, resourceResolver, hostname); + assertThat(res.addresses).containsExactly(backendAddr); + assertThat(res.balancerAddresses).isEmpty(); + assertThat(res.txtRecords).isEmpty(); + verify(mockResolver).resolveAddress(hostname); } @Test - public void jndiResolverWorks() throws Exception { - Assume.assumeTrue(DnsNameResolver.jndiAvailable()); - DnsNameResolver.DelegateResolver resolver = new DnsNameResolver.JndiResolver(); - ResolutionResults results = null; - try { - results = resolver.resolve("localhost"); - } catch (javax.naming.CommunicationException e) { - Assume.assumeNoException(e); - } catch (javax.naming.NameNotFoundException e) { - Assume.assumeNoException(e); - } + public void resolveAll_presentResourceResolver() throws Exception { + final String hostname = "addr.fake"; + final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); + final EquivalentAddressGroup balancerAddr = new EquivalentAddressGroup(new SocketAddress() {}); - assertThat(results.addresses).isEmpty(); - assertThat(results.txtRecords).isNotNull(); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(Matchers.anyString())) + .thenReturn(Collections.singletonList(backendAddr)); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(Matchers.anyString())) + .thenReturn(Collections.singletonList("service config")); + when(mockResourceResolver.resolveSrv(Matchers.any(AddressResolver.class), Matchers.anyString())) + .thenReturn(Collections.singletonList(balancerAddr)); + + ResolutionResults res = + DnsNameResolver.resolveAll(mockAddressResolver, mockResourceResolver, hostname); + assertThat(res.addresses).containsExactly(backendAddr); + assertThat(res.balancerAddresses).containsExactly(balancerAddr); + assertThat(res.txtRecords).containsExactly("service config"); + verify(mockAddressResolver).resolveAddress(hostname); + verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname); + verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname); } @Test - public void compositeResolverPrefersJdkAddressJndiTxt() throws Exception { - MockResolver jdkDelegate = new MockResolver(); - MockResolver jndiDelegate = new MockResolver(); - DelegateResolver resolver = new DnsNameResolver.CompositeResolver(jdkDelegate, jndiDelegate); + public void resolveAll_onlyBalancers() throws Exception { + String hostname = "addr.fake"; + EquivalentAddressGroup balancerAddr = new EquivalentAddressGroup(new SocketAddress() {}); - List jdkAnswer = createAddressList(2); - jdkDelegate.addAnswer( - jdkAnswer, - Arrays.asList("jdktxt"), - Collections.emptyList()); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(Matchers.anyString())) + .thenThrow(new UnknownHostException("I really tried")); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(Matchers.anyString())) + .thenReturn(Collections.emptyList()); + when(mockResourceResolver.resolveSrv(Matchers.any(AddressResolver.class), Matchers.anyString())) + .thenReturn(Collections.singletonList(balancerAddr)); - List jdniAnswer = createAddressList(2); - jndiDelegate.addAnswer( - jdniAnswer, - Arrays.asList("jnditxt"), - Collections.singletonList( - new EquivalentAddressGroup( - Collections.singletonList(new SocketAddress() {}), - Attributes.EMPTY))); - - ResolutionResults results = resolver.resolve("abc"); - - assertThat(results.addresses).containsExactlyElementsIn(jdkAnswer).inOrder(); - assertThat(results.txtRecords).containsExactly("jnditxt"); - assertThat(results.balancerAddresses).hasSize(1); + ResolutionResults res = + DnsNameResolver.resolveAll(mockAddressResolver, mockResourceResolver, hostname); + assertThat(res.addresses).isEmpty(); + assertThat(res.balancerAddresses).containsExactly(balancerAddr); + assertThat(res.txtRecords).isEmpty(); + verify(mockAddressResolver).resolveAddress(hostname); + verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname); + verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname); } @Test - public void compositeResolverSkipsAbsentJndi() throws Exception { - MockResolver jdkDelegate = new MockResolver(); - MockResolver jndiDelegate = null; - DelegateResolver resolver = new DnsNameResolver.CompositeResolver(jdkDelegate, jndiDelegate); + public void resolveAll_balancerLookupFails() throws Exception { + final String hostname = "addr.fake"; + final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(Matchers.anyString())) + .thenReturn(Collections.singletonList(backendAddr)); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(Matchers.anyString())) + .thenReturn(Collections.singletonList("service config")); + when(mockResourceResolver.resolveSrv(Matchers.any(AddressResolver.class), Matchers.anyString())) + .thenThrow(new Exception("something like javax.naming.NamingException")); - List jdkAnswer = createAddressList(2); - jdkDelegate.addAnswer(jdkAnswer); + ResolutionResults res = + DnsNameResolver.resolveAll(mockAddressResolver, mockResourceResolver, hostname); + assertThat(res.addresses).containsExactly(backendAddr); + assertThat(res.balancerAddresses).isEmpty(); + assertThat(res.txtRecords).containsExactly("service config"); + verify(mockAddressResolver).resolveAddress(hostname); + verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname); + verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname); + } - ResolutionResults results = resolver.resolve("abc"); + @Test + public void skipMissingJndiResolverResolver() throws Exception { + ClassLoader cl = new ClassLoader() { + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if ("io.grpc.internal.JndiResourceResolverFactory".equals(name)) { + throw new ClassNotFoundException(); + } + return super.loadClass(name, resolve); + } + }; - assertThat(results.addresses).containsExactlyElementsIn(jdkAnswer).inOrder(); - assertThat(results.txtRecords).isEmpty(); + ResourceResolverFactory factory = DnsNameResolver.getResourceResolverFactory(cl); + + assertThat(factory).isNull(); } @Test @@ -263,11 +300,12 @@ public class DnsNameResolverTest { "password"); when(alwaysDetectProxy.proxyFor(any(SocketAddress.class))) .thenReturn(proxyParameters); - DelegateResolver unusedResolver = mock(DelegateResolver.class); - DnsNameResolver resolver = newResolver(name, port, unusedResolver, alwaysDetectProxy); + DnsNameResolver resolver = newResolver(name, port, alwaysDetectProxy); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(Matchers.anyString())).thenThrow(new AssertionError()); + resolver.setAddressResolver(mockAddressResolver); resolver.start(mockListener); assertEquals(1, fakeExecutor.runDueTasks()); - verify(unusedResolver, never()).resolve(any(String.class)); verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class)); List result = resultCaptor.getValue(); @@ -280,23 +318,6 @@ public class DnsNameResolverTest { assertTrue(((InetSocketAddress) socketAddress.getAddress()).isUnresolved()); } - @Test - public void normalizeDataRemovesJndiFormattingForTxtRecords() { - assertEquals("blah", DnsNameResolver.normalizeData("TXT", "blah")); - assertEquals("", DnsNameResolver.normalizeData("TXT", "\"\"")); - assertEquals("blahblah", DnsNameResolver.normalizeData("TXT", "blah blah")); - assertEquals("blahfoo blah", DnsNameResolver.normalizeData("TXT", "blah \"foo blah\"")); - assertEquals("blah blah", DnsNameResolver.normalizeData("TXT", "\"blah blah\"")); - assertEquals("blah\"blah", DnsNameResolver.normalizeData("TXT", "\"blah\\\"blah\"")); - assertEquals("blah\\blah", DnsNameResolver.normalizeData("TXT", "\"blah\\\\blah\"")); - } - - @Test - public void normalizeDataLeavesSrvRecordsUnModified() { - assertEquals("0 0 1234 foo.bar.com", DnsNameResolver.normalizeData( - "SRV", "0 0 1234 foo.bar.com")); - } - @Test public void maybeChooseServiceConfig_failsOnMisspelling() { Map bad = new LinkedHashMap(); @@ -567,42 +588,4 @@ public class DnsNameResolverTest { assertEquals("Addr " + i, addrs.get(i), socketAddr.getAddress()); } } - - private static class MockResolver extends DnsNameResolver.DelegateResolver { - private final Queue answers = new LinkedList(); - private final Queue invocations = new LinkedList(); - - MockResolver addAnswer(List addresses) { - return addAnswer(addresses, null, null); - } - - MockResolver addAnswer( - List addresses, - List txtRecords, - List balancerAddresses) { - answers.add( - new ResolutionResults( - addresses, - MoreObjects.firstNonNull(txtRecords, Collections.emptyList()), - MoreObjects.firstNonNull( - balancerAddresses, Collections.emptyList()))); - return this; - } - - MockResolver addAnswer(UnknownHostException ex) { - answers.add(ex); - return this; - } - - @SuppressWarnings("unchecked") // explosions acceptable. - @Override - ResolutionResults resolve(String host) throws Exception { - invocations.add(host); - Object answer = answers.poll(); - if (answer instanceof UnknownHostException) { - throw (UnknownHostException) answer; - } - return (ResolutionResults) answer; - } - } } diff --git a/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java b/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java new file mode 100644 index 0000000000..80f5a7b5f3 --- /dev/null +++ b/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java @@ -0,0 +1,77 @@ +/* + * Copyright 2018 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; + +import io.grpc.EquivalentAddressGroup; +import io.grpc.internal.DnsNameResolver.AddressResolver; +import io.grpc.internal.JndiResourceResolverFactory.JndiResourceResolver; +import io.grpc.internal.JndiResourceResolverFactory.JndiResourceResolver.SrvRecord; +import java.net.InetAddress; +import java.util.List; +import org.junit.Assume; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Unit tests for {@link JndiResourceResolverFactory}. + */ +@RunWith(JUnit4.class) +public class JndiResourceResolverTest { + + @Test + public void normalizeDataRemovesJndiFormattingForTxtRecords() { + assertEquals("blah", JndiResourceResolver.unquote("blah")); + assertEquals("", JndiResourceResolver.unquote("\"\"")); + assertEquals("blahblah", JndiResourceResolver.unquote("blah blah")); + assertEquals("blahfoo blah", JndiResourceResolver.unquote("blah \"foo blah\"")); + assertEquals("blah blah", JndiResourceResolver.unquote("\"blah blah\"")); + assertEquals("blah\"blah", JndiResourceResolver.unquote("\"blah\\\"blah\"")); + assertEquals("blah\\blah", JndiResourceResolver.unquote("\"blah\\\\blah\"")); + } + + @Test + public void jndiResolverWorks() throws Exception { + Assume.assumeNoException(new JndiResourceResolverFactory().unavailabilityCause()); + + AddressResolver addressResolver = new AddressResolver() { + @Override + public List resolveAddress(String host) throws Exception { + return null; + } + }; + JndiResourceResolver resolver = new JndiResourceResolver(); + List results = null; + try { + results = resolver.resolveSrv(addressResolver, "localhost"); + } catch (javax.naming.CommunicationException e) { + Assume.assumeNoException(e); + } catch (javax.naming.NameNotFoundException e) { + Assume.assumeNoException(e); + } + } + + @Test + public void parseSrvRecord() { + SrvRecord record = JndiResourceResolver.parseSrvRecord("0 0 1234 foo.bar.com"); + assertThat(record.host).isEqualTo("foo.bar.com"); + assertThat(record.port).isEqualTo(1234); + } +}