Skip to content

Commit

Permalink
Add fix for xdstp replacement for encoded authorities (#10571)
Browse files Browse the repository at this point in the history
In ac35ab6 the logic in xDS Name resolver was changed to support encoded
authorities. This seems to cause an issue for xdstp replacements which
would percent encode the authority for the replacement causing double
encoding.

For example:

URI = xds:///path/to/service
Authority = path%2Fto%2Fservice
xdstp resource = xdstp:///envoy.config.listener.v3.Listener/path%252Fto%252Fservice

Here the authority is encoded due to slashes and during replacement we
percent encode it again causing %2F to change to %252F. To avoid this
issue, use the encoded authority only for the getServiceAuthority() API
and for all other use cases retain the unencoded authority.
  • Loading branch information
anicr7 authored Sep 22, 2023
1 parent 88b3484 commit fec7c2e
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
10 changes: 7 additions & 3 deletions xds/src/main/java/io/grpc/xds/XdsNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ final class XdsNameResolver extends NameResolver {
@Nullable
private final String targetAuthority;
private final String serviceAuthority;
// Encoded version of the service authority as per
// https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
private final String encodedServiceAuthority;
private final String overrideAuthority;
private final ServiceConfigParser serviceConfigParser;
private final SynchronizationContext syncContext;
Expand Down Expand Up @@ -149,8 +152,9 @@ final class XdsNameResolver extends NameResolver {
this.targetAuthority = targetAuthority;

// The name might have multiple slashes so encode it before verifying.
String authority = GrpcUtil.AuthorityEscaper.encodeAuthority(checkNotNull(name, "name"));
serviceAuthority = GrpcUtil.checkAuthority(authority);
serviceAuthority = checkNotNull(name, "name");
this.encodedServiceAuthority =
GrpcUtil.checkAuthority(GrpcUtil.AuthorityEscaper.encodeAuthority(serviceAuthority));

this.overrideAuthority = overrideAuthority;
this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser");
Expand All @@ -169,7 +173,7 @@ final class XdsNameResolver extends NameResolver {

@Override
public String getServiceAuthority() {
return serviceAuthority;
return encodedServiceAuthority;
}

@Override
Expand Down
25 changes: 25 additions & 0 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,31 @@ public void resolving_noTargetAuthority_templateWithXdstp() {
verify(mockListener, never()).onError(any(Status.class));
}

@Test
public void resolving_noTargetAuthority_xdstpWithMultipleSlashes() {
bootstrapInfo = BootstrapInfo.builder()
.servers(ImmutableList.of(ServerInfo.create(
"td.googleapis.com", InsecureChannelCredentials.create())))
.node(Node.newBuilder().build())
.clientDefaultListenerResourceNameTemplate(
"xdstp://xds.authority.com/envoy.config.listener.v3.Listener/%s?id=1")
.build();
String serviceAuthority = "path/to/service";
expectedLdsResourceName =
"xdstp://xds.authority.com/envoy.config.listener.v3.Listener/"
+ "path/to/service?id=1";
resolver = new XdsNameResolver(
null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);


// The Service Authority must be URL encoded, but unlike the LDS resource name.
assertThat(resolver.getServiceAuthority()).isEqualTo("path%2Fto%2Fservice");

resolver.start(mockListener);
verify(mockListener, never()).onError(any(Status.class));
}

@Test
public void resolving_targetAuthorityInAuthoritiesMap() {
String targetAuthority = "xds.authority.com";
Expand Down

0 comments on commit fec7c2e

Please sign in to comment.