Skip to content

Commit

Permalink
api: Target schema should be case insensitive
Browse files Browse the repository at this point in the history
URI schema are case-insensitive. Previously the code would do
case-sensitive matching. We expect NameResolverProviders to return the
typical canonical scheme formatting, which is lower-case. If a
NameResolverProvider returns an unexpected string (upper case, unicode,
etc), then it simply won't ever match. Channel users, however, can use
either casing in target strings.

The code implicitly already handled relative URIs by returning null, as
Map.get(null) returned null.
  • Loading branch information
ejona86 committed Feb 17, 2023
1 parent b481f34 commit 305dfee
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
2 changes: 2 additions & 0 deletions api/src/main/java/io/grpc/NameResolverProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public abstract class NameResolverProvider extends NameResolver.Factory {
* delegates to {@link Factory#getDefaultScheme()} before {@link NameResolver.Factory} is
* deprecated in https://github.com/grpc/grpc-java/issues/7133.
*
* <p>The scheme should be lower-case.
*
* @since 1.40.0
* */
protected String getScheme() {
Expand Down
7 changes: 6 additions & 1 deletion api/src/main/java/io/grpc/NameResolverRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -162,7 +163,11 @@ private final class NameResolverFactory extends NameResolver.Factory {
@Override
@Nullable
public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
NameResolverProvider provider = providers().get(targetUri.getScheme());
String scheme = targetUri.getScheme();
if (scheme == null) {
return null;
}
NameResolverProvider provider = providers().get(scheme.toLowerCase(Locale.US));
return provider == null ? null : provider.newNameResolver(targetUri, args);
}

Expand Down
4 changes: 4 additions & 0 deletions api/src/test/java/io/grpc/NameResolverRegistryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,12 @@ public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs)
});

assertThat(registry.asFactory().newNameResolver(uri, args)).isNull();
assertThat(registry.asFactory().newNameResolver(URI.create("/0.0.0.0:80"), args)).isNull();
assertThat(registry.asFactory().newNameResolver(URI.create("///0.0.0.0:80"), args)).isNull();
assertThat(registry.asFactory().newNameResolver(URI.create("other:///0.0.0.0:80"), args))
.isSameInstanceAs(nr);
assertThat(registry.asFactory().newNameResolver(URI.create("OTHER:///0.0.0.0:80"), args))
.isSameInstanceAs(nr);
assertThat(registry.asFactory().getDefaultScheme()).isEqualTo("dns");
}

Expand Down

0 comments on commit 305dfee

Please sign in to comment.