From fec7c2ea4af3a8d0df8f5a0c379eb00291e65b49 Mon Sep 17 00:00:00 2001
From: Anirudh Ramachandra <anramach@google.com>
Date: Fri, 22 Sep 2023 08:25:08 -0700
Subject: [PATCH] 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.
---
 .../java/io/grpc/xds/XdsNameResolver.java     | 10 +++++---
 .../java/io/grpc/xds/XdsNameResolverTest.java | 25 +++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
index 29043b177d9..079f862aeca 100644
--- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
+++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
@@ -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;
@@ -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");
@@ -169,7 +173,7 @@ final class XdsNameResolver extends NameResolver {
 
   @Override
   public String getServiceAuthority() {
-    return serviceAuthority;
+    return encodedServiceAuthority;
   }
 
   @Override
diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
index 4b858c5716e..9865b60172a 100644
--- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
+++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
@@ -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";