Skip to content

Commit fec7c2e

Browse files
authored
Add fix for xdstp replacement for encoded authorities (#10571)
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.
1 parent 88b3484 commit fec7c2e

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ final class XdsNameResolver extends NameResolver {
105105
@Nullable
106106
private final String targetAuthority;
107107
private final String serviceAuthority;
108+
// Encoded version of the service authority as per
109+
// https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
110+
private final String encodedServiceAuthority;
108111
private final String overrideAuthority;
109112
private final ServiceConfigParser serviceConfigParser;
110113
private final SynchronizationContext syncContext;
@@ -149,8 +152,9 @@ final class XdsNameResolver extends NameResolver {
149152
this.targetAuthority = targetAuthority;
150153

151154
// The name might have multiple slashes so encode it before verifying.
152-
String authority = GrpcUtil.AuthorityEscaper.encodeAuthority(checkNotNull(name, "name"));
153-
serviceAuthority = GrpcUtil.checkAuthority(authority);
155+
serviceAuthority = checkNotNull(name, "name");
156+
this.encodedServiceAuthority =
157+
GrpcUtil.checkAuthority(GrpcUtil.AuthorityEscaper.encodeAuthority(serviceAuthority));
154158

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

170174
@Override
171175
public String getServiceAuthority() {
172-
return serviceAuthority;
176+
return encodedServiceAuthority;
173177
}
174178

175179
@Override

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,31 @@ public void resolving_noTargetAuthority_templateWithXdstp() {
265265
verify(mockListener, never()).onError(any(Status.class));
266266
}
267267

268+
@Test
269+
public void resolving_noTargetAuthority_xdstpWithMultipleSlashes() {
270+
bootstrapInfo = BootstrapInfo.builder()
271+
.servers(ImmutableList.of(ServerInfo.create(
272+
"td.googleapis.com", InsecureChannelCredentials.create())))
273+
.node(Node.newBuilder().build())
274+
.clientDefaultListenerResourceNameTemplate(
275+
"xdstp://xds.authority.com/envoy.config.listener.v3.Listener/%s?id=1")
276+
.build();
277+
String serviceAuthority = "path/to/service";
278+
expectedLdsResourceName =
279+
"xdstp://xds.authority.com/envoy.config.listener.v3.Listener/"
280+
+ "path/to/service?id=1";
281+
resolver = new XdsNameResolver(
282+
null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler,
283+
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);
284+
285+
286+
// The Service Authority must be URL encoded, but unlike the LDS resource name.
287+
assertThat(resolver.getServiceAuthority()).isEqualTo("path%2Fto%2Fservice");
288+
289+
resolver.start(mockListener);
290+
verify(mockListener, never()).onError(any(Status.class));
291+
}
292+
268293
@Test
269294
public void resolving_targetAuthorityInAuthoritiesMap() {
270295
String targetAuthority = "xds.authority.com";

0 commit comments

Comments
 (0)