xds: Encode the service authority in XdsNameResolver (#10207)

Encode the service authority before passing it into gRPC util in the xDS name resolver to handle xDS requests which might contain multiple slashes. Example: xds:///path/to/service:port.

As currently the underlying Java URI library does not break the encoded authority into host/port correctly simplify the check to just look for '@' as we are only interested in checking for user info to validate the authority for HTTP.

This change also leads to few changes in unit tests that relied on this check for invalid authorities which now will be considered valid.

Just like #9376, depending on Guava packages such as URLEscapers or PercentEscapers leads to internal failures(Ex: Unresolvable reference to com.google.common.escape.Escaper from io.grpc.internal.GrpcUtil). To avoid these issues create an in house version that is heavily inspired by grpc-go/grpc.
This commit is contained in:
Anirudh Ramachandra 2023-07-14 12:54:18 -07:00 committed by GitHub
parent 0aaa2e0434
commit ac35ab67f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 146 additions and 19 deletions

View File

@ -55,9 +55,11 @@ import java.net.SocketAddress;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.nio.charset.Charset; import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Set; import java.util.Set;
@ -526,8 +528,8 @@ public final class GrpcUtil {
*/ */
public static String checkAuthority(String authority) { public static String checkAuthority(String authority) {
URI uri = authorityToUri(authority); URI uri = authorityToUri(authority);
checkArgument(uri.getHost() != null, "No host in authority '%s'", authority); // Verify that the user Info is not provided.
checkArgument(uri.getUserInfo() == null, checkArgument(uri.getAuthority().indexOf('@') == -1,
"Userinfo must not be present on authority: '%s'", authority); "Userinfo must not be present on authority: '%s'", authority);
return authority; return authority;
} }
@ -859,5 +861,92 @@ public final class GrpcUtil {
return false; return false;
} }
/**
* Percent encode the {@code authority} based on
* https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
*
* <p>When escaping a String, the following rules apply:
*
* <ul>
* <li>The alphanumeric characters "a" through "z", "A" through "Z" and "0" through "9" remain
* the same.
* <li>The unreserved characters ".", "-", "~", and "_" remain the same.
* <li>The general delimiters for authority, "[", "]", "@" and ":" remain the same.
* <li>The subdelimiters "!", "$", "&amp;", "'", "(", ")", "*", "+", ",", ";", and "=" remain
* the same.
* <li>The space character " " is converted into %20.
* <li>All other characters are converted into one or more bytes using UTF-8 encoding and each
* byte is then represented by the 3-character string "%XY", where "XY" is the two-digit,
* uppercase, hexadecimal representation of the byte value.
* </ul>
*
* <p>This section does not use URLEscapers from Guava Net as its not Android-friendly thus core
* can't depend on it.
*/
public static class AuthorityEscaper {
// Escapers should output upper case hex digits.
private static final char[] UPPER_HEX_DIGITS = "0123456789ABCDEF".toCharArray();
private static final Set<Character> UNRESERVED_CHARACTERS = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList('-', '_', '.', '~')));
private static final Set<Character> SUB_DELIMS = Collections
.unmodifiableSet(new HashSet<>(
Arrays.asList('!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=')));
private static final Set<Character> AUTHORITY_DELIMS = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList(':', '[', ']', '@')));
private static boolean shouldEscape(char c) {
// Only encode ASCII.
if (c > 127) {
return false;
}
// Letters don't need an escape.
if (((c >= 'a') && (c <= 'z')) || ((c >= 'A') && (c <= 'Z'))) {
return false;
}
// Numbers don't need to be escaped.
if ((c >= '0' && c <= '9')) {
return false;
}
// Don't escape allowed characters.
if (UNRESERVED_CHARACTERS.contains(c)
|| SUB_DELIMS.contains(c)
|| AUTHORITY_DELIMS.contains(c)) {
return false;
}
return true;
}
public static String encodeAuthority(String authority) {
Preconditions.checkNotNull(authority, "authority");
int authorityLength = authority.length();
int hexCount = 0;
// Calculate how many characters actually need escaping.
for (int index = 0; index < authorityLength; index++) {
char c = authority.charAt(index);
if (shouldEscape(c)) {
hexCount++;
}
}
// If no char need escaping, just return the original string back.
if (hexCount == 0) {
return authority;
}
// Allocate enough space as encoded characters need 2 extra chars.
StringBuilder encoded_authority = new StringBuilder((2 * hexCount) + authorityLength);
for (int index = 0; index < authorityLength; index++) {
char c = authority.charAt(index);
if (shouldEscape(c)) {
encoded_authority.append('%');
encoded_authority.append(UPPER_HEX_DIGITS[c >>> 4]);
encoded_authority.append(UPPER_HEX_DIGITS[c & 0xF]);
} else {
encoded_authority.append(c);
}
}
return encoded_authority.toString();
}
}
private GrpcUtil() {} private GrpcUtil() {}
} }

View File

@ -163,6 +163,42 @@ public class GrpcUtilTest {
assertFalse(GrpcUtil.isGrpcContentType("application/bad")); assertFalse(GrpcUtil.isGrpcContentType("application/bad"));
} }
@Test
public void urlAuthorityEscape_ipv6Address() {
assertEquals("[::1]", GrpcUtil.AuthorityEscaper.encodeAuthority("[::1]"));
}
@Test
public void urlAuthorityEscape_userInAuthority() {
assertEquals("user@host", GrpcUtil.AuthorityEscaper.encodeAuthority("user@host"));
}
@Test
public void urlAuthorityEscape_slashesAreEncoded() {
assertEquals(
"project%2F123%2Fnetwork%2Fabc%2Fservice",
GrpcUtil.AuthorityEscaper.encodeAuthority("project/123/network/abc/service"));
}
@Test
public void urlAuthorityEscape_allowedCharsAreNotEncoded() {
assertEquals(
"-._~!$&'()*+,;=@:[]", GrpcUtil.AuthorityEscaper.encodeAuthority("-._~!$&'()*+,;=@:[]"));
}
@Test
public void urlAuthorityEscape_allLettersAndNumbers() {
assertEquals(
"abcdefghijklmnopqrstuvwxyz0123456789",
GrpcUtil.AuthorityEscaper.encodeAuthority("abcdefghijklmnopqrstuvwxyz0123456789"));
}
@Test
public void urlAuthorityEscape_unicodeAreNotEncoded() {
assertEquals(
"ö®", GrpcUtil.AuthorityEscaper.encodeAuthority("ö®"));
}
@Test @Test
public void checkAuthority_failsOnNull() { public void checkAuthority_failsOnNull() {
thrown.expect(NullPointerException.class); thrown.expect(NullPointerException.class);
@ -199,13 +235,6 @@ public class GrpcUtilTest {
GrpcUtil.checkAuthority("[ : : 1]"); GrpcUtil.checkAuthority("[ : : 1]");
} }
@Test
public void checkAuthority_failsOnInvalidHost() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("No host in authority");
GrpcUtil.checkAuthority("bad_host");
}
@Test @Test
public void checkAuthority_userInfoNotAllowed() { public void checkAuthority_userInfoNotAllowed() {

View File

@ -368,7 +368,7 @@ public class ManagedChannelImplBuilderTest {
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void overrideAuthority_invalid() { public void overrideAuthority_invalid() {
builder.overrideAuthority("not_allowed"); builder.overrideAuthority("user@not_allowed");
} }
@Test @Test

View File

@ -147,7 +147,11 @@ final class XdsNameResolver extends NameResolver {
XdsClientPoolFactory xdsClientPoolFactory, ThreadSafeRandom random, XdsClientPoolFactory xdsClientPoolFactory, ThreadSafeRandom random,
FilterRegistry filterRegistry, @Nullable Map<String, ?> bootstrapOverride) { FilterRegistry filterRegistry, @Nullable Map<String, ?> bootstrapOverride) {
this.targetAuthority = targetAuthority; this.targetAuthority = targetAuthority;
serviceAuthority = GrpcUtil.checkAuthority(checkNotNull(name, "name"));
// The name might have multiple slashes so encode it before verifying.
String authority = GrpcUtil.AuthorityEscaper.encodeAuthority(checkNotNull(name, "name"));
serviceAuthority = GrpcUtil.checkAuthority(authority);
this.overrideAuthority = overrideAuthority; this.overrideAuthority = overrideAuthority;
this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser"); this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser");
this.syncContext = checkNotNull(syncContext, "syncContext"); this.syncContext = checkNotNull(syncContext, "syncContext");

View File

@ -110,14 +110,19 @@ public class XdsNameResolverProviderTest {
} }
@Test @Test
public void invalidName_hostnameContainsUnderscore() { public void validName_urlExtractedAuthorityInvalidWithoutEncoding() {
URI uri = URI.create("xds:///foo_bar.googleapis.com"); XdsNameResolver resolver =
try { provider.newNameResolver(URI.create("xds:///1234/path/foo.googleapis.com:8080"), args);
provider.newNameResolver(uri, args); assertThat(resolver).isNotNull();
fail("Expected IllegalArgumentException"); assertThat(resolver.getServiceAuthority()).isEqualTo("1234%2Fpath%2Ffoo.googleapis.com:8080");
} catch (IllegalArgumentException e) { }
// Expected
} @Test
public void validName_urlwithTargetAuthorityAndExtractedAuthorityInvalidWithoutEncoding() {
XdsNameResolver resolver = provider.newNameResolver(URI.create(
"xds://trafficdirector.google.com/1234/path/foo.googleapis.com:8080"), args);
assertThat(resolver).isNotNull();
assertThat(resolver.getServiceAuthority()).isEqualTo("1234%2Fpath%2Ffoo.googleapis.com:8080");
} }
@Test @Test