grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

core: change DNS error handling to fail on all invalid configs

This commit is contained in:
Carl Mastrangelo 2019-03-19 09:22:02 -07:00 committed by GitHub
parent 7df2d5feeb
commit 6b4a3796a6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 112 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
core/src/main/java/io/grpc/internal/DnsNameResolver.java
View File

@ -27,6 +27,7 @@ import com.google.common.base.Verify;
import io.grpc.Attributes;
import io.grpc.EquivalentAddressGroup;
import io.grpc.NameResolver;
import io.grpc.NameResolver.Helper.ConfigOrError;
import io.grpc.ProxiedSocketAddress;
import io.grpc.ProxyDetector;
import io.grpc.Status;
@ -285,25 +286,16 @@ final class DnsNameResolver extends NameResolver {
Attributes.Builder attrs = Attributes.newBuilder();
if (!resolutionResults.txtRecords.isEmpty()) {
Map<String, ?> serviceConfig = null;
try {
for (Map<String, ?> possibleConfig : parseTxtResults(resolutionResults.txtRecords)) {
try {
serviceConfig =
maybeChooseServiceConfig(possibleConfig, random, getLocalHostname());
} catch (RuntimeException e) {
logger.log(Level.WARNING, "Bad service config choice " + possibleConfig, e);
}
ConfigOrError<Map<String, ?>> serviceConfig =
parseServiceConfig(resolutionResults.txtRecords, random, getLocalHostname());
if (serviceConfig != null) {
break;
if (serviceConfig.getError() != null) {
savedListener.onError(serviceConfig.getError());
return;
} else {
attrs.set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig.getConfig());
}
}
} catch (RuntimeException e) {
logger.log(Level.WARNING, "Can't parse service Configs", e);
}
if (serviceConfig != null) {
attrs.set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig);
}
} else {
logger.log(Level.FINE, "No TXT records found for {0}", new Object[]{host});
}
@ -311,6 +303,35 @@ final class DnsNameResolver extends NameResolver {
}
}
@Nullable
static ConfigOrError<Map<String, ?>> parseServiceConfig(
List<String> rawTxtRecords, Random random, String localHostname) {
List<Map<String, ?>> possibleServiceConfigChoices;
try {
possibleServiceConfigChoices = parseTxtResults(rawTxtRecords);
} catch (IOException | RuntimeException e) {
return ConfigOrError.fromError(
Status.UNKNOWN.withDescription("failed to parse TXT records").withCause(e));
}
Map<String, ?> possibleServiceConfig = null;
for (Map<String, ?> possibleServiceConfigChoice : possibleServiceConfigChoices) {
try {
possibleServiceConfig =
maybeChooseServiceConfig(possibleServiceConfigChoice, random, localHostname);
} catch (RuntimeException e) {
return ConfigOrError.fromError(
Status.UNKNOWN.withDescription("failed to pick service config choice").withCause(e));
}
if (possibleServiceConfig != null) {
break;
}
}
if (possibleServiceConfig == null) {
return null;
}
return ConfigOrError.<Map<String, ?>>fromConfig(possibleServiceConfig);
}
private void resolve() {
if (resolving || shutdown || !cacheRefreshRequired()) {
return;
@ -403,35 +424,27 @@ final class DnsNameResolver extends NameResolver {
return new ResolutionResults(addresses, txtRecords, balancerAddresses);
}
/**
*
* @throws IOException if one of the txt records contains improperly formatted JSON.
*/
@SuppressWarnings("unchecked")
@VisibleForTesting
static List<Map<String, ?>> parseTxtResults(List<String> txtRecords) {
List<Map<String, ?>> serviceConfigs = new ArrayList<>();
static List<Map<String, ?>> parseTxtResults(List<String> txtRecords) throws IOException {
List<Map<String, ?>> possibleServiceConfigChoices = new ArrayList<>();
for (String txtRecord : txtRecords) {
if (txtRecord.startsWith(SERVICE_CONFIG_PREFIX)) {
List<Map<String, ?>> choices;
try {
Object rawChoices = JsonParser.parse(txtRecord.substring(SERVICE_CONFIG_PREFIX.length()));
if (!(rawChoices instanceof List)) {
throw new IOException("wrong type " + rawChoices);
}
List<?> listChoices = (List<?>) rawChoices;
for (Object obj : listChoices) {
if (!(obj instanceof Map)) {
throw new IOException("wrong element type " + rawChoices);
}
}
choices = (List<Map<String, ?>>) listChoices;
} catch (IOException e) {
logger.log(Level.WARNING, "Bad service config: " + txtRecord, e);
if (!txtRecord.startsWith(SERVICE_CONFIG_PREFIX)) {
logger.log(Level.FINE, "Ignoring non service config {0}", new Object[]{txtRecord});
continue;
}
serviceConfigs.addAll(choices);
} else {
logger.log(Level.FINE, "Ignoring non service config {0}", new Object[]{txtRecord});
Object rawChoices = JsonParser.parse(txtRecord.substring(SERVICE_CONFIG_PREFIX.length()));
if (!(rawChoices instanceof List)) {
throw new ClassCastException("wrong type " + rawChoices);
}
List<?> listChoices = (List<?>) rawChoices;
possibleServiceConfigChoices.addAll(ServiceConfigUtil.checkObjectList(listChoices));
}
return serviceConfigs;
return possibleServiceConfigChoices;
}
@Nullable

2
core/src/main/java/io/grpc/internal/JsonParser.java
View File

@ -40,7 +40,7 @@ public final class JsonParser {
private JsonParser() {}
/**
* Parses a json string, returning either a {@code Map<String, ?>}, {@code List<Object>},
* Parses a json string, returning either a {@code Map<String, ?>}, {@code List<?>},
* {@code String}, {@code Double}, {@code Boolean}, or {@code null}.
*/
@SuppressWarnings("unchecked")

2
core/src/main/java/io/grpc/internal/ServiceConfigUtil.java
View File

@ -530,7 +530,7 @@ public final class ServiceConfigUtil {
}
@SuppressWarnings("unchecked")
private static List<Map<String, ?>> checkObjectList(List<?> rawList) {
static List<Map<String, ?>> checkObjectList(List<?> rawList) {
for (int i = 0; i < rawList.size(); i++) {
if (!(rawList.get(i) instanceof Map)) {
throw new ClassCastException(

77
core/src/test/java/io/grpc/internal/DnsNameResolverTest.java
View File

@ -32,6 +32,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.net.InetAddresses;
import com.google.common.testing.FakeTicker;
@ -39,6 +40,7 @@ import io.grpc.Attributes;
import io.grpc.EquivalentAddressGroup;
import io.grpc.HttpConnectProxiedSocketAddress;
import io.grpc.NameResolver;
import io.grpc.NameResolver.Helper.ConfigOrError;
import io.grpc.ProxyDetector;
import io.grpc.Status;
import io.grpc.Status.Code;
@ -58,6 +60,7 @@ 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.List;
@ -878,7 +881,7 @@ public class DnsNameResolverTest {
}
@Test
public void parseTxtResults_misspelledName() {
public void parseTxtResults_misspelledName() throws Exception {
List<String> txtRecords = new ArrayList<>();
txtRecords.add("some_record");
txtRecords.add("_grpc_config=[]");
@ -889,50 +892,36 @@ public class DnsNameResolverTest {
}
@Test
public void parseTxtResults_badTypeIgnored() {
Logger logger = Logger.getLogger(DnsNameResolver.class.getName());
Level level = logger.getLevel();
logger.setLevel(Level.SEVERE);
try {
public void parseTxtResults_badTypeFails() throws Exception {
thrown.expect(ClassCastException.class);
thrown.expectMessage("wrong type");
List<String> txtRecords = new ArrayList<>();
txtRecords.add("some_record");
txtRecords.add("grpc_config={}");
List<? extends Map<String, ?>> results = DnsNameResolver.parseTxtResults(txtRecords);
assertThat(results).isEmpty();
} finally {
logger.setLevel(level);
}
DnsNameResolver.parseTxtResults(txtRecords);
}
@Test
public void parseTxtResults_badInnerTypeIgnored() {
Logger logger = Logger.getLogger(DnsNameResolver.class.getName());
Level level = logger.getLevel();
logger.setLevel(Level.SEVERE);
try {
public void parseTxtResults_badInnerTypeFails() throws Exception {
thrown.expect(ClassCastException.class);
thrown.expectMessage("not object");
List<String> txtRecords = new ArrayList<>();
txtRecords.add("some_record");
txtRecords.add("grpc_config=[\"bogus\"]");
List<? extends Map<String, ?>> results = DnsNameResolver.parseTxtResults(txtRecords);
assertThat(results).isEmpty();
} finally {
logger.setLevel(level);
}
DnsNameResolver.parseTxtResults(txtRecords);
}
@Test
public void parseTxtResults_combineAll() {
public void parseTxtResults_combineAll() throws Exception {
Logger logger = Logger.getLogger(DnsNameResolver.class.getName());
Level level = logger.getLevel();
logger.setLevel(Level.SEVERE);
try {
List<String> txtRecords = new ArrayList<>();
txtRecords.add("some_record");
txtRecords.add("grpc_config=[\"bogus\", {}]");
txtRecords.add("grpc_config=[{}, {}]"); // 2 records
txtRecords.add("grpc_config=[{\"\":{}}]"); // 1 record
@ -1017,6 +1006,44 @@ public class DnsNameResolverTest {
enableJndi, enableJndiLocalhost, "2001-db8-1234--as3.ipv6-literal.net"));
}
@Test
public void parseServiceConfig_capturesParseError() {
ConfigOrError<Map<String, ?>> result = DnsNameResolver.parseServiceConfig(
Arrays.asList("grpc_config=bogus"), new Random(), "localhost");
assertThat(result).isNotNull();
assertThat(result.getError().getCode()).isEqualTo(Status.Code.UNKNOWN);
assertThat(result.getError().getDescription()).contains("failed to parse TXT records");
}
@Test
public void parseServiceConfig_capturesChoiceError() {
ConfigOrError<Map<String, ?>> result = DnsNameResolver.parseServiceConfig(
Arrays.asList("grpc_config=[{\"hi\":{}}]"), new Random(), "localhost");
assertThat(result).isNotNull();
assertThat(result.getError().getCode()).isEqualTo(Status.Code.UNKNOWN);
assertThat(result.getError().getDescription()).contains("failed to pick");
}
@Test
public void parseServiceConfig_noChoiceIsNull() {
ConfigOrError<Map<String, ?>> result = DnsNameResolver.parseServiceConfig(
Arrays.asList("grpc_config=[]"), new Random(), "localhost");
assertThat(result).isNull();
}
@Test
public void parseServiceConfig_matches() {
ConfigOrError<Map<String, ?>> result = DnsNameResolver.parseServiceConfig(
Arrays.asList("grpc_config=[{\"serviceConfig\":{}}]"), new Random(), "localhost");
assertThat(result).isNotNull();
assertThat(result.getError()).isNull();
assertThat(result.getConfig()).isEqualTo(ImmutableMap.of());
}
private void testInvalidUri(URI uri) {
try {
provider.newNameResolver(uri, helper);