From 66d25d0a68aeaef9c7359fe27a1ce5b5472b54c9 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Wed, 17 Dec 2025 01:26:59 -0800 Subject: [PATCH 1/5] api: Add newNameResolver() overload and default, best-effort impl. --- api/src/main/java/io/grpc/NameResolver.java | 35 +++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index 0e8315e812c..53dbc5d6888 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -158,6 +158,10 @@ public abstract static class Factory { * cannot be resolved by this factory. The decision should be solely based on the scheme of the * URI. * + *

This method will eventually be deprecated and removed as part of a migration from {@code + * java.net.URI} to {@code io.grpc.Uri}. Implementations will override {@link + * #newNameResolver(Uri, Args)} instead. + * * @param targetUri the target URI to be resolved, whose scheme must not be {@code null} * @param args other information that may be useful * @@ -165,6 +169,37 @@ public abstract static class Factory { */ public abstract NameResolver newNameResolver(URI targetUri, final Args args); + /** + * Creates a {@link NameResolver} for the given target URI. + * + *

Implementations return {@code null} if 'targetUri' cannot be resolved by this factory. The + * decision should be solely based on the target's scheme. + * + *

All {@link NameResolver.Factory} implementations should override this method, as it will + * eventually replace {@link #newNameResolver(URI, Args)}. For backwards compatibility, this + * default implementation delegates to {@link #newNameResolver(URI, Args)} if 'targetUri' can be + * converted to a java.net.URI. + * + *

NB: Conversion is not always possible, for example {@code scheme:#frag} is a valid {@link + * Uri} but not a valid {@link URI} because its path is empty. The default implementation throws + * IllegalArgumentException in these cases. + * + * @param targetUri the target URI to be resolved + * @param args other information that may be useful + * @throws IllegalArgumentException if targetUri does not have the expected form + * @since 1.79 + */ + public NameResolver newNameResolver(Uri targetUri, final Args args) { + // Not every io.grpc.Uri can be converted but in the ordinary ManagedChannel creation flow, + // any IllegalArgumentException thrown here would happened anyway, just earlier. That's + // because parse/toString is transparent so java.net.URI#create here sees the original target + // string just like it did before the io.grpc.Uri migration. + // + // Throwing IAE shouldn't surprise non-framework callers either. After all, many existing + // Factory impls are picky about targetUri and throw IAE when it doesn't look how they expect. + return newNameResolver(URI.create(targetUri.toString()), args); + } + /** * Returns the default scheme, which will be used to construct a URI when {@link * ManagedChannelBuilder#forTarget(String)} is given an authority string instead of a compliant From d1c5350d48d181d0c1102169914cc80100b225e8 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Thu, 18 Dec 2025 15:44:26 -0800 Subject: [PATCH 2/5] core: Create a UriWrapper abstraction for the migration # Conflicts: # core/src/main/java/io/grpc/internal/ManagedChannelImpl.java --- .../io/grpc/internal/ManagedChannelImpl.java | 8 +- .../internal/ManagedChannelImplBuilder.java | 7 +- .../java/io/grpc/internal/UriWrapper.java | 139 ++++++++++++++++++ ...ManagedChannelImplGetNameResolverTest.java | 3 +- .../ManagedChannelImplIdlenessTest.java | 3 +- .../grpc/internal/ManagedChannelImplTest.java | 13 +- .../ServiceConfigErrorHandlingTest.java | 3 +- 7 files changed, 160 insertions(+), 16 deletions(-) create mode 100644 core/src/main/java/io/grpc/internal/UriWrapper.java diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 5c3afbf8de0..e423220e3ad 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -160,7 +160,7 @@ public Result selectConfig(PickSubchannelArgs args) { @Nullable private final String authorityOverride; private final NameResolverRegistry nameResolverRegistry; - private final URI targetUri; + private final UriWrapper targetUri; private final NameResolverProvider nameResolverProvider; private final NameResolver.Args nameResolverArgs; private final LoadBalancerProvider loadBalancerFactory; @@ -538,7 +538,7 @@ ClientStream newSubstream( ManagedChannelImpl( ManagedChannelImplBuilder builder, ClientTransportFactory clientTransportFactory, - URI targetUri, + UriWrapper targetUri, NameResolverProvider nameResolverProvider, BackoffPolicy.Provider backoffPolicyProvider, ObjectPool balancerRpcExecutorPool, @@ -667,9 +667,9 @@ public CallTracer create() { @VisibleForTesting static NameResolver getNameResolver( - URI targetUri, @Nullable final String overrideAuthority, + UriWrapper targetUri, @Nullable final String overrideAuthority, NameResolverProvider provider, NameResolver.Args nameResolverArgs) { - NameResolver resolver = provider.newNameResolver(targetUri, nameResolverArgs); + NameResolver resolver = targetUri.newNameResolver(provider, nameResolverArgs); if (resolver == null) { throw new IllegalArgumentException("cannot create a NameResolver for " + targetUri); } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 1773c04388d..944fea9c512 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static io.grpc.internal.UriWrapper.wrap; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -814,10 +815,10 @@ int getDefaultPort() { @VisibleForTesting static class ResolvedNameResolver { - public final URI targetUri; + public final UriWrapper targetUri; public final NameResolverProvider provider; - public ResolvedNameResolver(URI targetUri, NameResolverProvider provider) { + public ResolvedNameResolver(UriWrapper targetUri, NameResolverProvider provider) { this.targetUri = checkNotNull(targetUri, "targetUri"); this.provider = checkNotNull(provider, "provider"); } @@ -872,7 +873,7 @@ static ResolvedNameResolver getNameResolverProvider( } } - return new ResolvedNameResolver(targetUri, provider); + return new ResolvedNameResolver(wrap(targetUri), provider); } private static class DirectAddressNameResolverProvider extends NameResolverProvider { diff --git a/core/src/main/java/io/grpc/internal/UriWrapper.java b/core/src/main/java/io/grpc/internal/UriWrapper.java new file mode 100644 index 00000000000..ca5835cabd8 --- /dev/null +++ b/core/src/main/java/io/grpc/internal/UriWrapper.java @@ -0,0 +1,139 @@ +/* + * Copyright 2025 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import static com.google.common.base.Preconditions.checkNotNull; + +import io.grpc.NameResolver; +import io.grpc.Uri; +import java.net.URI; +import javax.annotation.Nullable; + +/** Temporary wrapper for a URI-like object to ease the migration to io.grpc.Uri. */ +interface UriWrapper { + + static UriWrapper wrap(URI uri) { + return new JavaNetUriWrapper(uri); + } + + static UriWrapper wrap(Uri uri) { + return new IoGrpcUriWrapper(uri); + } + + /** Uses the given factory and args to create a {@link NameResolver} for this URI. */ + NameResolver newNameResolver(NameResolver.Factory factory, NameResolver.Args args); + + /** Returns the scheme component of this URI, e.g. "http", "mailto" or "dns". */ + String getScheme(); + + /** + * Returns the authority component of this URI, e.g. "google.com", "127.0.0.1:8080", or null if + * not present. + */ + @Nullable + String getAuthority(); + + /** Wraps an instance of java.net.URI. */ + final class JavaNetUriWrapper implements UriWrapper { + private final URI uri; + + private JavaNetUriWrapper(URI uri) { + this.uri = checkNotNull(uri); + } + + @Override + public NameResolver newNameResolver(NameResolver.Factory factory, NameResolver.Args args) { + return factory.newNameResolver(uri, args); + } + + @Override + public String getScheme() { + return uri.getScheme(); + } + + @Override + public String getAuthority() { + return uri.getAuthority(); + } + + @Override + public String toString() { + return uri.toString(); + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + if (!(other instanceof JavaNetUriWrapper)) { + return false; + } + return uri.equals(((JavaNetUriWrapper) other).uri); + } + + @Override + public int hashCode() { + return uri.hashCode(); + } + } + + /** Wraps an instance of io.grpc.Uri. */ + final class IoGrpcUriWrapper implements UriWrapper { + private final Uri uri; + + private IoGrpcUriWrapper(Uri uri) { + this.uri = checkNotNull(uri); + } + + @Override + public NameResolver newNameResolver(NameResolver.Factory factory, NameResolver.Args args) { + return factory.newNameResolver(uri, args); + } + + @Override + public String getScheme() { + return uri.getScheme(); + } + + @Override + public String getAuthority() { + return uri.getAuthority(); + } + + @Override + public String toString() { + return uri.toString(); + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + if (!(other instanceof IoGrpcUriWrapper)) { + return false; + } + return uri.equals(((IoGrpcUriWrapper) other).uri); + } + + @Override + public int hashCode() { + return uri.hashCode(); + } + } +} diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java index a0bd388b1b6..8ec522260ae 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java @@ -17,6 +17,7 @@ package io.grpc.internal; import static com.google.common.truth.Truth.assertThat; +import static io.grpc.internal.UriWrapper.wrap; import static org.junit.Assert.fail; import io.grpc.NameResolver; @@ -125,7 +126,7 @@ private void testValidTarget(String target, String expectedUriString, URI expect ManagedChannelImplBuilder.getNameResolverProvider( target, nameResolverRegistry, Collections.singleton(InetSocketAddress.class)); assertThat(resolved.provider).isInstanceOf(FakeNameResolverProvider.class); - assertThat(resolved.targetUri).isEqualTo(expectedUri); + assertThat(resolved.targetUri).isEqualTo(wrap(expectedUri)); assertThat(resolved.targetUri.toString()).isEqualTo(expectedUriString); } diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java index 293d0e70961..97e92be7fdd 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import static io.grpc.ConnectivityState.READY; import static io.grpc.ConnectivityState.TRANSIENT_FAILURE; +import static io.grpc.internal.UriWrapper.wrap; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -185,7 +186,7 @@ public void setUp() { NameResolverProvider nameResolverProvider = builder.nameResolverRegistry.getProviderForScheme(targetUri.getScheme()); channel = new ManagedChannelImpl( - builder, mockTransportFactory, targetUri, nameResolverProvider, + builder, mockTransportFactory, wrap(targetUri), nameResolverProvider, new FakeBackoffPolicyProvider(), oobExecutorPool, timer.getStopwatchSupplier(), Collections.emptyList(), diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 24ea4fa03d9..30b53e99946 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -28,6 +28,7 @@ import static io.grpc.EquivalentAddressGroup.ATTR_AUTHORITY_OVERRIDE; import static io.grpc.PickSubchannelArgsMatcher.eqPickSubchannelArgs; import static io.grpc.internal.ClientStreamListener.RpcProgress.PROCESSED; +import static io.grpc.internal.UriWrapper.wrap; import static junit.framework.TestCase.assertNotSame; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -315,7 +316,7 @@ private void createChannel(boolean nameResolutionExpectedToFail, NameResolverProvider nameResolverProvider = channelBuilder.nameResolverRegistry.getProviderForScheme(expectedUri.getScheme()); channel = new ManagedChannelImpl( - channelBuilder, mockTransportFactory, expectedUri, nameResolverProvider, + channelBuilder, mockTransportFactory, wrap(expectedUri), nameResolverProvider, new FakeBackoffPolicyProvider(), balancerRpcExecutorPool, timer.getStopwatchSupplier(), Arrays.asList(interceptors), timer.getTimeProvider()); @@ -504,7 +505,7 @@ public void startCallBeforeNameResolution() throws Exception { when(mockTransportFactory.getSupportedSocketAddressTypes()).thenReturn(Collections.singleton( InetSocketAddress.class)); channel = new ManagedChannelImpl( - channelBuilder, mockTransportFactory, expectedUri, nameResolverFactory, + channelBuilder, mockTransportFactory, wrap(expectedUri), nameResolverFactory, new FakeBackoffPolicyProvider(), balancerRpcExecutorPool, timer.getStopwatchSupplier(), Collections.emptyList(), timer.getTimeProvider()); @@ -569,7 +570,7 @@ public void newCallWithConfigSelector() { when(mockTransportFactory.getSupportedSocketAddressTypes()).thenReturn(Collections.singleton( InetSocketAddress.class)); channel = new ManagedChannelImpl( - channelBuilder, mockTransportFactory, expectedUri, nameResolverFactory, + channelBuilder, mockTransportFactory, wrap(expectedUri), nameResolverFactory, new FakeBackoffPolicyProvider(), balancerRpcExecutorPool, timer.getStopwatchSupplier(), Collections.emptyList(), timer.getTimeProvider()); @@ -4416,11 +4417,11 @@ public void validAuthorityTarget_overrideAuthority() throws Exception { URI targetUri = new URI("defaultscheme", "", "/foo.googleapis.com:8080", null); NameResolver nameResolver = ManagedChannelImpl.getNameResolver( - targetUri, null, nameResolverProvider, NAMERESOLVER_ARGS); + wrap(targetUri), null, nameResolverProvider, NAMERESOLVER_ARGS); assertThat(nameResolver.getServiceAuthority()).isEqualTo(serviceAuthority); nameResolver = ManagedChannelImpl.getNameResolver( - targetUri, overrideAuthority, nameResolverProvider, NAMERESOLVER_ARGS); + wrap(targetUri), overrideAuthority, nameResolverProvider, NAMERESOLVER_ARGS); assertThat(nameResolver.getServiceAuthority()).isEqualTo(overrideAuthority); } @@ -4449,7 +4450,7 @@ public String getDefaultScheme() { }; try { ManagedChannelImpl.getNameResolver( - URI.create("defaultscheme:///foo.gogoleapis.com:8080"), + wrap(URI.create("defaultscheme:///foo.gogoleapis.com:8080")), null, nameResolverProvider, NAMERESOLVER_ARGS); fail("Should fail"); } catch (IllegalArgumentException e) { diff --git a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java index 6f255763d30..0daee676b82 100644 --- a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java +++ b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static io.grpc.internal.UriWrapper.wrap; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalAnswers.delegatesTo; @@ -168,7 +169,7 @@ private void createChannel(ClientInterceptor... interceptors) { new ManagedChannelImpl( channelBuilder, mockTransportFactory, - expectedUri, + wrap(expectedUri), nameResolverProvider, new FakeBackoffPolicyProvider(), balancerRpcExecutorPool, From d133984f53c2246c73bc489a12deb71d505b675e Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 5 Jan 2026 23:06:48 -0800 Subject: [PATCH 3/5] core: add a FlagResetRule test fixture --- .../java/io/grpc/internal/FlagResetRule.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 core/src/testFixtures/java/io/grpc/internal/FlagResetRule.java diff --git a/core/src/testFixtures/java/io/grpc/internal/FlagResetRule.java b/core/src/testFixtures/java/io/grpc/internal/FlagResetRule.java new file mode 100644 index 00000000000..dbcaca79358 --- /dev/null +++ b/core/src/testFixtures/java/io/grpc/internal/FlagResetRule.java @@ -0,0 +1,66 @@ +/* + * Copyright 2026 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import java.util.ArrayDeque; +import java.util.Deque; +import org.junit.rules.ExternalResource; + +/** + * A {@link org.junit.rules.TestRule} that lets you set one or more feature flags just for + * the duration of the current test case. + * + *

Flags and other global variables must be reset to ensure no state leaks across tests. + */ +public final class FlagResetRule extends ExternalResource { + + /** A functional interface representing a standard gRPC feature flag setter. */ + public interface SetterMethod { + /** Sets a flag for testing and returns its previous value. */ + T set(T val); + } + + private final Deque toRunAfter = new ArrayDeque<>(); + + /** + * Sets a global feature flag to 'value' using 'setter' and arranges for its previous value to be + * unconditionally restored when the test completes. + */ + public void setFlagForTest(SetterMethod setter, T value) { + final T oldValue = setter.set(value); + toRunAfter.push(() -> setter.set(oldValue)); + } + + @Override + protected void after() { + RuntimeException toThrow = null; + while (!toRunAfter.isEmpty()) { + try { + toRunAfter.pop().run(); + } catch (RuntimeException e) { + if (toThrow == null) { + toThrow = e; + } else { + toThrow.addSuppressed(e); + } + } + } + if (toThrow != null) { + throw toThrow; + } + } +} From 5a9d0a1fbd24fa478a84511e1747022c25d7935a Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 5 Jan 2026 23:07:58 -0800 Subject: [PATCH 4/5] core: Use io.grpc.Uri to parse the target, if flagged --- .../internal/ManagedChannelImplBuilder.java | 83 +++++- .../ManagedChannelImplBuilderTest.java | 24 +- ...ChannelImplGetNameResolverRfc3986Test.java | 243 ++++++++++++++++++ ...ManagedChannelImplGetNameResolverTest.java | 36 ++- 4 files changed, 361 insertions(+), 25 deletions(-) create mode 100644 core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverRfc3986Test.java diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 944fea9c512..628224b826e 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -47,6 +47,7 @@ import io.grpc.NameResolverRegistry; import io.grpc.ProxyDetector; import io.grpc.StatusOr; +import io.grpc.Uri; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.net.SocketAddress; @@ -105,6 +106,16 @@ public static ManagedChannelBuilder forTarget(String target) { */ static final long IDLE_MODE_MIN_TIMEOUT_MILLIS = TimeUnit.SECONDS.toMillis(1); + private static boolean enableRfc3986Uris = GrpcUtil.getFlag("GRPC_ENABLE_RFC3986_URIS", false); + + /** Whether to parse targets as RFC 3986 URIs (true), or use {@link java.net.URI} (false). */ + @VisibleForTesting + static boolean setRfc3986UrisEnabled(boolean value) { + boolean prevValue = ManagedChannelImplBuilder.enableRfc3986Uris; + ManagedChannelImplBuilder.enableRfc3986Uris = value; + return prevValue; + } + private static final ObjectPool DEFAULT_EXECUTOR_POOL = SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR); @@ -719,8 +730,11 @@ protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { public ManagedChannel build() { ClientTransportFactory clientTransportFactory = clientTransportFactoryBuilder.buildClientTransportFactory(); - ResolvedNameResolver resolvedResolver = getNameResolverProvider( - target, nameResolverRegistry, clientTransportFactory.getSupportedSocketAddressTypes()); + ResolvedNameResolver resolvedResolver = + enableRfc3986Uris + ? getNameResolverProviderRfc3986(target, nameResolverRegistry) + : getNameResolverProvider(target, nameResolverRegistry); + resolvedResolver.checkAddressTypes(clientTransportFactory.getSupportedSocketAddressTypes()); return new ManagedChannelOrphanWrapper(new ManagedChannelImpl( this, clientTransportFactory, @@ -822,12 +836,25 @@ public ResolvedNameResolver(UriWrapper targetUri, NameResolverProvider provider) this.targetUri = checkNotNull(targetUri, "targetUri"); this.provider = checkNotNull(provider, "provider"); } + + void checkAddressTypes( + Collection> channelTransportSocketAddressTypes) { + if (channelTransportSocketAddressTypes != null) { + Collection> nameResolverSocketAddressTypes = + provider.getProducedSocketAddressTypes(); + if (!channelTransportSocketAddressTypes.containsAll(nameResolverSocketAddressTypes)) { + throw new IllegalArgumentException( + String.format( + "Address types of NameResolver '%s' for '%s' not supported by transport", + provider.getDefaultScheme(), targetUri)); + } + } + } } @VisibleForTesting static ResolvedNameResolver getNameResolverProvider( - String target, NameResolverRegistry nameResolverRegistry, - Collection> channelTransportSocketAddressTypes) { + String target, NameResolverRegistry nameResolverRegistry) { // Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending // "dns:///". NameResolverProvider provider = null; @@ -863,14 +890,46 @@ static ResolvedNameResolver getNameResolverProvider( target, uriSyntaxErrors.length() > 0 ? " (" + uriSyntaxErrors + ")" : "")); } - if (channelTransportSocketAddressTypes != null) { - Collection> nameResolverSocketAddressTypes - = provider.getProducedSocketAddressTypes(); - if (!channelTransportSocketAddressTypes.containsAll(nameResolverSocketAddressTypes)) { - throw new IllegalArgumentException(String.format( - "Address types of NameResolver '%s' for '%s' not supported by transport", - targetUri.getScheme(), target)); - } + return new ResolvedNameResolver(wrap(targetUri), provider); + } + + @VisibleForTesting + static ResolvedNameResolver getNameResolverProviderRfc3986( + String target, NameResolverRegistry nameResolverRegistry) { + // Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending + // "dns:///". + NameResolverProvider provider = null; + Uri targetUri = null; + StringBuilder uriSyntaxErrors = new StringBuilder(); + try { + targetUri = Uri.parse(target); + } catch (URISyntaxException e) { + // Can happen with ip addresses like "[::1]:1234" or 127.0.0.1:1234. + uriSyntaxErrors.append(e.getMessage()); + } + if (targetUri != null) { + // For "localhost:8080" this would likely cause provider to be null, because "localhost" is + // parsed as the scheme. Will hit the next case and try "dns:///localhost:8080". + provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme()); + } + + if (provider == null && !URI_PATTERN.matcher(target).matches()) { + // It doesn't look like a URI target. Maybe it's an authority string. Try with the default + // scheme from the registry. + targetUri = + Uri.newBuilder() + .setScheme(nameResolverRegistry.getDefaultScheme()) + .setHost("") + .setPath("/" + target) + .build(); + provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme()); + } + + if (provider == null) { + throw new IllegalArgumentException( + String.format( + "Could not find a NameResolverProvider for %s%s", + target, uriSyntaxErrors.length() > 0 ? " (" + uriSyntaxErrors + ")" : "")); } return new ResolvedNameResolver(wrap(targetUri), provider); diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java index a054e65a6e8..8bf10de3949 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java @@ -69,13 +69,15 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; /** Unit tests for {@link ManagedChannelImplBuilder}. */ -@RunWith(JUnit4.class) +@RunWith(Parameterized.class) public class ManagedChannelImplBuilderTest { private static final int DUMMY_PORT = 42; private static final String DUMMY_TARGET = "fake-target"; @@ -98,8 +100,16 @@ public ClientCall interceptCall( } }; + @Parameters(name = "enableRfc3986UrisParam={0}") + public static Iterable data() { + return Arrays.asList(new Object[][] {{true}, {false}}); + } + + @Parameter public boolean enableRfc3986UrisParam; + @Rule public final MockitoRule mocks = MockitoJUnit.rule(); @Rule public final GrpcCleanupRule grpcCleanupRule = new GrpcCleanupRule(); + @Rule public final FlagResetRule flagResetRule = new FlagResetRule(); @Mock private ClientTransportFactory mockClientTransportFactory; @Mock private ClientTransportFactoryBuilder mockClientTransportFactoryBuilder; @@ -117,6 +127,9 @@ public ClientCall interceptCall( @Before public void setUp() throws Exception { + flagResetRule.setFlagForTest( + ManagedChannelImplBuilder::setRfc3986UrisEnabled, enableRfc3986UrisParam); + builder = new ManagedChannelImplBuilder( DUMMY_TARGET, new UnsupportedClientTransportFactoryBuilder(), @@ -373,8 +386,11 @@ public void transportDoesNotSupportAddressTypes() { ManagedChannel unused = grpcCleanupRule.register(builder.build()); fail("Should fail"); } catch (IllegalArgumentException e) { - assertThat(e).hasMessageThat().isEqualTo( - "Address types of NameResolver 'dns' for 'valid:1234' not supported by transport"); + assertThat(e) + .hasMessageThat() + .isEqualTo( + "Address types of NameResolver 'dns' for 'dns:///valid:1234' not supported by" + + " transport"); } } diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverRfc3986Test.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverRfc3986Test.java new file mode 100644 index 00000000000..5bcf24a30e2 --- /dev/null +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverRfc3986Test.java @@ -0,0 +1,243 @@ +/* + * Copyright 2015 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import static com.google.common.truth.Truth.assertThat; +import static io.grpc.internal.UriWrapper.wrap; +import static org.junit.Assert.fail; + +import io.grpc.NameResolver; +import io.grpc.NameResolverProvider; +import io.grpc.NameResolverRegistry; +import io.grpc.Uri; +import java.net.SocketAddress; +import java.net.URI; +import java.util.Collections; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for ManagedChannelImplBuilder#getNameResolverProviderNew(). */ +@RunWith(JUnit4.class) +public class ManagedChannelImplGetNameResolverRfc3986Test { + @Test + public void invalidUriTarget() { + testInvalidTarget("defaultscheme:///[invalid]"); + } + + @Test + public void invalidUnescapedSquareBracketsInRfc3986UriFragment() { + testInvalidTarget("defaultscheme://8.8.8.8/host#section[1]"); + } + + @Test + public void invalidUnescapedSquareBracketsInRfc3986UriQuery() { + testInvalidTarget("dns://8.8.8.8/path?section=[1]"); + } + + @Test + public void validTargetWithInvalidDnsName() throws Exception { + testValidTarget( + "[valid]", + "defaultscheme:///%5Bvalid%5D", + Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("/[valid]").build()); + } + + @Test + public void validAuthorityTarget() throws Exception { + testValidTarget( + "foo.googleapis.com:8080", + "defaultscheme:///foo.googleapis.com:8080", + Uri.newBuilder() + .setScheme("defaultscheme") + .setHost("") + .setPath("/foo.googleapis.com:8080") + .build()); + } + + @Test + public void validUriTarget() throws Exception { + testValidTarget( + "scheme:///foo.googleapis.com:8080", + "scheme:///foo.googleapis.com:8080", + Uri.newBuilder() + .setScheme("scheme") + .setHost("") + .setPath("/foo.googleapis.com:8080") + .build()); + } + + @Test + public void validIpv4AuthorityTarget() throws Exception { + testValidTarget( + "127.0.0.1:1234", + "defaultscheme:///127.0.0.1:1234", + Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("/127.0.0.1:1234").build()); + } + + @Test + public void validIpv4UriTarget() throws Exception { + testValidTarget( + "dns:///127.0.0.1:1234", + "dns:///127.0.0.1:1234", + Uri.newBuilder().setScheme("dns").setHost("").setPath("/127.0.0.1:1234").build()); + } + + @Test + public void validIpv6AuthorityTarget() throws Exception { + testValidTarget( + "[::1]:1234", + "defaultscheme:///%5B::1%5D:1234", + Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("/[::1]:1234").build()); + } + + @Test + public void invalidIpv6UriTarget() throws Exception { + testInvalidTarget("dns:///[::1]:1234"); + } + + @Test + public void invalidIpv6UriWithUnescapedScope() { + testInvalidTarget("dns://[::1%eth0]:53/host"); + } + + @Test + public void validIpv6UriTarget() throws Exception { + testValidTarget( + "dns:///%5B::1%5D:1234", + "dns:///%5B::1%5D:1234", + Uri.newBuilder().setScheme("dns").setHost("").setPath("/[::1]:1234").build()); + } + + @Test + public void validTargetStartingWithSlash() throws Exception { + testValidTarget( + "/target", + "defaultscheme:////target", + Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("//target").build()); + } + + @Test + public void validTargetNoProvider() { + NameResolverRegistry nameResolverRegistry = new NameResolverRegistry(); + try { + ManagedChannelImplBuilder.getNameResolverProviderRfc3986( + "foo.googleapis.com:8080", nameResolverRegistry); + fail("Should fail"); + } catch (IllegalArgumentException e) { + // expected + } + } + + @Test + public void validTargetProviderAddrTypesNotSupported() { + NameResolverRegistry nameResolverRegistry = getTestRegistry("testscheme"); + try { + ManagedChannelImplBuilder.getNameResolverProviderRfc3986( + "testscheme:///foo.googleapis.com:8080", nameResolverRegistry) + .checkAddressTypes(Collections.singleton(CustomSocketAddress.class)); + fail("Should fail"); + } catch (IllegalArgumentException e) { + assertThat(e) + .hasMessageThat() + .isEqualTo( + "Address types of NameResolver 'testscheme' for " + + "'testscheme:///foo.googleapis.com:8080' not supported by transport"); + } + } + + private void testValidTarget(String target, String expectedUriString, Uri expectedUri) { + NameResolverRegistry nameResolverRegistry = getTestRegistry(expectedUri.getScheme()); + ManagedChannelImplBuilder.ResolvedNameResolver resolved = + ManagedChannelImplBuilder.getNameResolverProviderRfc3986(target, nameResolverRegistry); + assertThat(resolved.provider).isInstanceOf(FakeNameResolverProvider.class); + assertThat(resolved.targetUri).isEqualTo(wrap(expectedUri)); + assertThat(resolved.targetUri.toString()).isEqualTo(expectedUriString); + } + + private void testInvalidTarget(String target) { + NameResolverRegistry nameResolverRegistry = getTestRegistry("dns"); + + try { + ManagedChannelImplBuilder.ResolvedNameResolver resolved = + ManagedChannelImplBuilder.getNameResolverProviderRfc3986(target, nameResolverRegistry); + FakeNameResolverProvider nameResolverProvider = (FakeNameResolverProvider) resolved.provider; + fail("Should have failed, but got resolver provider " + nameResolverProvider); + } catch (IllegalArgumentException e) { + // expected + } + } + + private static NameResolverRegistry getTestRegistry(String expectedScheme) { + NameResolverRegistry nameResolverRegistry = new NameResolverRegistry(); + FakeNameResolverProvider nameResolverProvider = new FakeNameResolverProvider(expectedScheme); + nameResolverRegistry.register(nameResolverProvider); + return nameResolverRegistry; + } + + private static class FakeNameResolverProvider extends NameResolverProvider { + final String expectedScheme; + + FakeNameResolverProvider(String expectedScheme) { + this.expectedScheme = expectedScheme; + } + + @Override + public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) { + if (expectedScheme.equals(targetUri.getScheme())) { + return new FakeNameResolver(targetUri); + } + return null; + } + + @Override + public String getDefaultScheme() { + return expectedScheme; + } + + @Override + protected boolean isAvailable() { + return true; + } + + @Override + protected int priority() { + return 5; + } + } + + private static class FakeNameResolver extends NameResolver { + final URI uri; + + FakeNameResolver(URI uri) { + this.uri = uri; + } + + @Override + public String getServiceAuthority() { + return uri.getAuthority(); + } + + @Override + public void start(final Listener2 listener) {} + + @Override + public void shutdown() {} + } + + private static class CustomSocketAddress extends SocketAddress {} +} diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java index 8ec522260ae..792f4daca4e 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java @@ -23,7 +23,6 @@ import io.grpc.NameResolver; import io.grpc.NameResolverProvider; import io.grpc.NameResolverRegistry; -import java.net.InetSocketAddress; import java.net.SocketAddress; import java.net.URI; import java.util.Collections; @@ -39,6 +38,21 @@ public void invalidUriTarget() { testInvalidTarget("defaultscheme:///[invalid]"); } + @Test + public void validSquareBracketsInRfc2396UriFragment() throws Exception { + testValidTarget("dns://8.8.8.8/host#section[1]", + "dns://8.8.8.8/host#section[1]", + new URI("dns", "8.8.8.8", "/host", null, "section[1]")); + } + + + @Test + public void validSquareBracketsInRfc2396UriQuery() throws Exception { + testValidTarget("dns://8.8.8.8/host?section=[1]", + "dns://8.8.8.8/host?section=[1]", + new URI("dns", "8.8.8.8", "/host", "section=[1]", null)); + } + @Test public void validTargetWithInvalidDnsName() throws Exception { testValidTarget("[valid]", "defaultscheme:///%5Bvalid%5D", @@ -75,6 +89,13 @@ public void validIpv6AuthorityTarget() throws Exception { new URI("defaultscheme", "", "/[::1]:1234", null)); } + @Test + public void validIpv6UriWithJavaNetUriScopeName() throws Exception { + testValidTarget("dns://[::1%eth0]:53/host", + "dns://[::1%eth0]:53/host", + new URI("dns", "[::1%eth0]:53", "/host", null, null)); + } + @Test public void invalidIpv6UriTarget() throws Exception { testInvalidTarget("dns:///[::1]:1234"); @@ -97,8 +118,7 @@ public void validTargetNoProvider() { NameResolverRegistry nameResolverRegistry = new NameResolverRegistry(); try { ManagedChannelImplBuilder.getNameResolverProvider( - "foo.googleapis.com:8080", nameResolverRegistry, - Collections.singleton(InetSocketAddress.class)); + "foo.googleapis.com:8080", nameResolverRegistry); fail("Should fail"); } catch (IllegalArgumentException e) { // expected @@ -110,8 +130,8 @@ public void validTargetProviderAddrTypesNotSupported() { NameResolverRegistry nameResolverRegistry = getTestRegistry("testscheme"); try { ManagedChannelImplBuilder.getNameResolverProvider( - "testscheme:///foo.googleapis.com:8080", nameResolverRegistry, - Collections.singleton(CustomSocketAddress.class)); + "testscheme:///foo.googleapis.com:8080", nameResolverRegistry) + .checkAddressTypes(Collections.singleton(CustomSocketAddress.class)); fail("Should fail"); } catch (IllegalArgumentException e) { assertThat(e).hasMessageThat().isEqualTo( @@ -123,8 +143,7 @@ public void validTargetProviderAddrTypesNotSupported() { private void testValidTarget(String target, String expectedUriString, URI expectedUri) { NameResolverRegistry nameResolverRegistry = getTestRegistry(expectedUri.getScheme()); ManagedChannelImplBuilder.ResolvedNameResolver resolved = - ManagedChannelImplBuilder.getNameResolverProvider( - target, nameResolverRegistry, Collections.singleton(InetSocketAddress.class)); + ManagedChannelImplBuilder.getNameResolverProvider(target, nameResolverRegistry); assertThat(resolved.provider).isInstanceOf(FakeNameResolverProvider.class); assertThat(resolved.targetUri).isEqualTo(wrap(expectedUri)); assertThat(resolved.targetUri.toString()).isEqualTo(expectedUriString); @@ -135,8 +154,7 @@ private void testInvalidTarget(String target) { try { ManagedChannelImplBuilder.ResolvedNameResolver resolved = - ManagedChannelImplBuilder.getNameResolverProvider( - target, nameResolverRegistry, Collections.singleton(InetSocketAddress.class)); + ManagedChannelImplBuilder.getNameResolverProvider(target, nameResolverRegistry); FakeNameResolverProvider nameResolverProvider = (FakeNameResolverProvider) resolved.provider; fail("Should have failed, but got resolver provider " + nameResolverProvider); } catch (IllegalArgumentException e) { From e228d26325bf0bbbec10502e70295a988332fa90 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 5 Jan 2026 17:04:45 -0800 Subject: [PATCH 5/5] binder: Migrate IntentNameResolverProvider to io.grpc.Uri --- .../internal/IntentNameResolverProvider.java | 17 ++++++++++++++--- .../IntentNameResolverProviderTest.java | 19 +++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/IntentNameResolverProvider.java b/binder/src/main/java/io/grpc/binder/internal/IntentNameResolverProvider.java index 67859045dba..5a3c9fcc986 100644 --- a/binder/src/main/java/io/grpc/binder/internal/IntentNameResolverProvider.java +++ b/binder/src/main/java/io/grpc/binder/internal/IntentNameResolverProvider.java @@ -20,6 +20,7 @@ import android.content.Intent; import com.google.common.collect.ImmutableSet; import io.grpc.NameResolver; +import io.grpc.Uri; import io.grpc.NameResolver.Args; import io.grpc.NameResolverProvider; import io.grpc.binder.AndroidComponentAddress; @@ -46,7 +47,17 @@ public String getDefaultScheme() { @Override public NameResolver newNameResolver(URI targetUri, final Args args) { if (Objects.equals(targetUri.getScheme(), ANDROID_INTENT_SCHEME)) { - return new IntentNameResolver(parseUriArg(targetUri), args); + return new IntentNameResolver(parseUriArg(targetUri.toString()), args); + } else { + return null; + } + } + + @Nullable + @Override + public NameResolver newNameResolver(Uri targetUri, final Args args) { + if (Objects.equals(targetUri.getScheme(), ANDROID_INTENT_SCHEME)) { + return new IntentNameResolver(parseUriArg(targetUri.toString()), args); } else { return null; } @@ -67,9 +78,9 @@ public ImmutableSet> getProducedSocketAddressType return ImmutableSet.of(AndroidComponentAddress.class); } - private static Intent parseUriArg(URI targetUri) { + private static Intent parseUriArg(String targetUri) { try { - return Intent.parseUri(targetUri.toString(), URI_INTENT_SCHEME); + return Intent.parseUri(targetUri, URI_INTENT_SCHEME); } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } diff --git a/binder/src/test/java/io/grpc/binder/internal/IntentNameResolverProviderTest.java b/binder/src/test/java/io/grpc/binder/internal/IntentNameResolverProviderTest.java index aa75ba84b09..2809a72fee1 100644 --- a/binder/src/test/java/io/grpc/binder/internal/IntentNameResolverProviderTest.java +++ b/binder/src/test/java/io/grpc/binder/internal/IntentNameResolverProviderTest.java @@ -30,6 +30,7 @@ import io.grpc.NameResolver.ServiceConfigParser; import io.grpc.NameResolverProvider; import io.grpc.SynchronizationContext; +import io.grpc.Uri; import io.grpc.binder.ApiConstants; import java.net.URI; import org.junit.Before; @@ -70,18 +71,32 @@ public void testProviderScheme_returnsIntentScheme() throws Exception { @Test public void testNoResolverForUnknownScheme_returnsNull() throws Exception { - assertThat(provider.newNameResolver(new URI("random://uri"), args)).isNull(); + assertThat(provider.newNameResolver(Uri.create("random://uri"), args)).isNull(); } @Test public void testResolutionWithBadUri_throwsIllegalArg() throws Exception { assertThrows( IllegalArgumentException.class, - () -> provider.newNameResolver(new URI("intent:xxx#Intent;e.x=1;end;"), args)); + () -> provider.newNameResolver(Uri.create("intent:xxx#Intent;e.x=1;end;"), args)); } @Test public void testResolverForIntentScheme_returnsResolver() throws Exception { + Uri uri = Uri.create("intent:#Intent;action=action;end"); + NameResolver resolver = provider.newNameResolver(uri, args); + assertThat(resolver).isNotNull(); + assertThat(resolver.getServiceAuthority()).isEqualTo("localhost"); + syncContext.execute(() -> resolver.start(mockListener)); + shadowOf(getMainLooper()).idle(); + verify(mockListener).onResult2(resultCaptor.capture()); + assertThat(resultCaptor.getValue().getAddressesOrError()).isNotNull(); + syncContext.execute(resolver::shutdown); + shadowOf(getMainLooper()).idle(); + } + + @Test + public void testResolverForIntentScheme_returnsResolver_javaNetUri() throws Exception { URI uri = new URI("intent://authority/path#Intent;action=action;scheme=scheme;end"); NameResolver resolver = provider.newNameResolver(uri, args); assertThat(resolver).isNotNull();