diff --git a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java index 10ba586ab47..488eaa8c75f 100644 --- a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java +++ b/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; diff --git a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java b/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java index bbd3ba3ef05..0251b3477b5 100644 --- a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java +++ b/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(); } }