googleapis: C2P resolver updates on query params handling (#12855)

Since all queries should be dropped entirely rather than just stripping
`force-xds`, I have unconditionally set `setRawQuery(null)` in
GoogleCloudToProdNameResolver when building the modified target URI.

Author: shivaspeaks

googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java

@@ -135,8 +135,6 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
     QueryParams queryParams = QueryParams.fromRawQuery(grpcUri.getRawQuery());
     this.forceXds = checkForceXds(queryParams);
     this.schemeOverride = (forceXds || isOnGcp) ? "xds" : "dns";
-    stripForceXds(queryParams);
-    String newQuery = queryParams.toRawQuery();
 
     Preconditions.checkArgument(
         targetPath.startsWith("/"),
@@ -147,7 +145,7 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
     syncContext = checkNotNull(args, "args").getSynchronizationContext();
 
     Uri.Builder modifiedTargetBuilder = grpcUri.toBuilder().setScheme(schemeOverride);
-    modifiedTargetBuilder.setRawQuery(newQuery);
+    modifiedTargetBuilder.setRawQuery(null);
     if (schemeOverride.equals("xds")) {
       modifiedTargetBuilder.setRawAuthority(C2P_AUTHORITY);
     }
@@ -180,8 +178,6 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
     QueryParams queryParams = QueryParams.fromRawQuery(targetUri.getRawQuery());
     this.forceXds = checkForceXds(queryParams);
     this.schemeOverride = (forceXds || isOnGcp) ? "xds" : "dns";
-    stripForceXds(queryParams);
-    String newQuery = queryParams.toRawQuery();
 
     Preconditions.checkArgument(
         targetUri.isPathAbsolute(),
@@ -195,11 +191,7 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
     authority = GrpcUtil.checkAuthority(pathSegments.get(0));
     syncContext = checkNotNull(args, "args").getSynchronizationContext();
     Uri.Builder modifiedTargetBuilder = targetUri.toBuilder().setScheme(schemeOverride);
-    if (newQuery != null) {
-      modifiedTargetBuilder.setRawQuery(newQuery);
-    } else {
-      modifiedTargetBuilder.setRawQuery(null);
-    }
+    modifiedTargetBuilder.setRawQuery(null);
 
     if (schemeOverride.equals("xds")) {
       modifiedTargetBuilder.setRawAuthority(C2P_AUTHORITY);
@@ -411,9 +403,7 @@ private static boolean checkForceXds(QueryParams params) {
     return false;
   }
 
-  private static void stripForceXds(QueryParams params) {
-    params.asList().removeIf(entry -> "force-xds".equals(entry.getKey()));
-  }
+
 
   private enum HttpConnectionFactory implements HttpConnectionProvider {
     INSTANCE;

googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java

@@ -212,9 +212,9 @@ public void notOnGcpButForceXds_DelegateToXds() {
   }
 
   @Test
-  public void notOnGcpButForceXds_KeyValueTrue_DelegateToXds() {
+  public void notOnGcpButForceXds_WithValue_DelegateToXds() {
     GoogleCloudToProdNameResolver.isOnGcp = false;
-    String target = TARGET_URI + "?force-xds=true";
+    String target = TARGET_URI + "?force-xds=foo";
     resolver = enableRfc3986UrisParam
         ? new GoogleCloudToProdNameResolver(
             Uri.create(target), args, fakeExecutorResource, nsRegistry.asFactory())
@@ -235,6 +235,53 @@ public void notOnGcpButForceXds_KeyValueTrue_DelegateToXds() {
     }
   }
 
+  @Test
+  public void notOnGcpButForceXds_PercentEncoded_DelegateToXds() {
+    GoogleCloudToProdNameResolver.isOnGcp = false;
+    String target = TARGET_URI + "?force%2Dxds";
+    resolver = enableRfc3986UrisParam
+        ? new GoogleCloudToProdNameResolver(
+            Uri.create(target), args, fakeExecutorResource, nsRegistry.asFactory())
+        : new GoogleCloudToProdNameResolver(
+            URI.create(target), args, fakeExecutorResource, nsRegistry.asFactory());
+    resolver.start(mockListener);
+    fakeExecutor.runDueTasks();
+    assertThat(delegatedResolver.keySet()).containsExactly("xds");
+
+    if (enableRfc3986UrisParam) {
+      Uri delegatedRfcUriValue = delegatedRfcUri.get("xds");
+      assertThat(delegatedRfcUriValue).isNotNull();
+      assertThat(delegatedRfcUriValue.getRawQuery()).isNull();
+    } else {
+      URI delegatedUriValue = delegatedUri.get("xds");
+      assertThat(delegatedUriValue).isNotNull();
+      assertThat(delegatedUriValue.getQuery()).isNull();
+    }
+  }
+
+  @Test
+  public void notOnGcpButForceXds_DuplicateKeys_DelegateToXds() {
+    GoogleCloudToProdNameResolver.isOnGcp = false;
+    String target = TARGET_URI + "?force-xds=&force-xds=true";
+    resolver = enableRfc3986UrisParam
+        ? new GoogleCloudToProdNameResolver(
+            Uri.create(target), args, fakeExecutorResource, nsRegistry.asFactory())
+        : new GoogleCloudToProdNameResolver(
+            URI.create(target), args, fakeExecutorResource, nsRegistry.asFactory());
+    resolver.start(mockListener);
+    fakeExecutor.runDueTasks();
+    assertThat(delegatedResolver.keySet()).containsExactly("xds");
+
+    if (enableRfc3986UrisParam) {
+      Uri delegatedRfcUriValue = delegatedRfcUri.get("xds");
+      assertThat(delegatedRfcUriValue).isNotNull();
+      assertThat(delegatedRfcUriValue.getRawQuery()).isNull();
+    } else {
+      URI delegatedUriValue = delegatedUri.get("xds");
+      assertThat(delegatedUriValue).isNotNull();
+      assertThat(delegatedUriValue.getQuery()).isNull();
+    }
+  }
 
   @Test
   public void notOnGcpButForceXds_WithMultipleParams_DelegateToXds() {
@@ -252,11 +299,11 @@ public void notOnGcpButForceXds_WithMultipleParams_DelegateToXds() {
     if (enableRfc3986UrisParam) {
       Uri delegatedRfcUriValue = delegatedRfcUri.get("xds");
       assertThat(delegatedRfcUriValue).isNotNull();
-      assertThat(delegatedRfcUriValue.getRawQuery()).isEqualTo("foo=bar&baz=qux");
+      assertThat(delegatedRfcUriValue.getRawQuery()).isNull();
     } else {
       URI delegatedUriValue = delegatedUri.get("xds");
       assertThat(delegatedUriValue).isNotNull();
-      assertThat(delegatedUriValue.getQuery()).isEqualTo("foo=bar&baz=qux");
+      assertThat(delegatedUriValue.getQuery()).isNull();
     }
   }
 
@@ -276,11 +323,11 @@ public void notOnGcpButForceXds_WithEncodedAmpersand_DelegateToXds() {
     if (enableRfc3986UrisParam) {
       Uri delegatedRfcUriValue = delegatedRfcUri.get("xds");
       assertThat(delegatedRfcUriValue).isNotNull();
-      assertThat(delegatedRfcUriValue.getRawQuery()).isEqualTo("foo=bar%26baz");
+      assertThat(delegatedRfcUriValue.getRawQuery()).isNull();
     } else {
       URI delegatedUriValue = delegatedUri.get("xds");
       assertThat(delegatedUriValue).isNotNull();
-      assertThat(delegatedUriValue.getRawQuery()).isEqualTo("foo=bar%26baz");
+      assertThat(delegatedUriValue.getRawQuery()).isNull();
     }
   }
 
@@ -299,11 +346,11 @@ public void notOnGcpButForceXds_CaseSensitive_DelegateToDns() {
     if (enableRfc3986UrisParam) {
       Uri delegatedRfcUriValue = delegatedRfcUri.get("dns");
       assertThat(delegatedRfcUriValue).isNotNull();
-      assertThat(delegatedRfcUriValue.getRawQuery()).isEqualTo("FORCE-XDS");
+      assertThat(delegatedRfcUriValue.getRawQuery()).isNull();
     } else {
       URI delegatedUriValue = delegatedUri.get("dns");
       assertThat(delegatedUriValue).isNotNull();
-      assertThat(delegatedUriValue.getQuery()).isEqualTo("FORCE-XDS");
+      assertThat(delegatedUriValue.getQuery()).isNull();
     }
   }