Skip to content

Commit e04c26e

Browse files
committed
Use io.grpc.Uri to parse the target, if flagged
1 parent edf5464 commit e04c26e

File tree

4 files changed

+295
-23
lines changed

4 files changed

+295
-23
lines changed

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import io.grpc.NameResolverRegistry;
4848
import io.grpc.ProxyDetector;
4949
import io.grpc.StatusOr;
50+
import io.grpc.Uri;
5051
import java.lang.reflect.InvocationTargetException;
5152
import java.lang.reflect.Method;
5253
import java.net.SocketAddress;
@@ -719,8 +720,11 @@ protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) {
719720
public ManagedChannel build() {
720721
ClientTransportFactory clientTransportFactory =
721722
clientTransportFactoryBuilder.buildClientTransportFactory();
722-
ResolvedNameResolver resolvedResolver = getNameResolverProvider(
723-
target, nameResolverRegistry, clientTransportFactory.getSupportedSocketAddressTypes());
723+
ResolvedNameResolver resolvedResolver =
724+
GrpcUtil.getFlag("GRPC_ENABLE_RFC3986_URIS", false)
725+
? getNameResolverProviderNew(target, nameResolverRegistry)
726+
: getNameResolverProvider(target, nameResolverRegistry);
727+
resolvedResolver.checkAddressTypes(clientTransportFactory.getSupportedSocketAddressTypes());
724728
return new ManagedChannelOrphanWrapper(new ManagedChannelImpl(
725729
this,
726730
clientTransportFactory,
@@ -822,12 +826,25 @@ public ResolvedNameResolver(UriWrapper targetUri, NameResolverProvider provider)
822826
this.targetUri = checkNotNull(targetUri, "targetUri");
823827
this.provider = checkNotNull(provider, "provider");
824828
}
829+
830+
void checkAddressTypes(
831+
Collection<Class<? extends SocketAddress>> channelTransportSocketAddressTypes) {
832+
if (channelTransportSocketAddressTypes != null) {
833+
Collection<Class<? extends SocketAddress>> nameResolverSocketAddressTypes =
834+
provider.getProducedSocketAddressTypes();
835+
if (!channelTransportSocketAddressTypes.containsAll(nameResolverSocketAddressTypes)) {
836+
throw new IllegalArgumentException(
837+
String.format(
838+
"Address types of NameResolver '%s' for '%s' not supported by transport",
839+
provider.getDefaultScheme(), targetUri));
840+
}
841+
}
842+
}
825843
}
826844

827845
@VisibleForTesting
828846
static ResolvedNameResolver getNameResolverProvider(
829-
String target, NameResolverRegistry nameResolverRegistry,
830-
Collection<Class<? extends SocketAddress>> channelTransportSocketAddressTypes) {
847+
String target, NameResolverRegistry nameResolverRegistry) {
831848
// Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending
832849
// "dns:///".
833850
NameResolverProvider provider = null;
@@ -863,14 +880,45 @@ static ResolvedNameResolver getNameResolverProvider(
863880
target, uriSyntaxErrors.length() > 0 ? " (" + uriSyntaxErrors + ")" : ""));
864881
}
865882

866-
if (channelTransportSocketAddressTypes != null) {
867-
Collection<Class<? extends SocketAddress>> nameResolverSocketAddressTypes
868-
= provider.getProducedSocketAddressTypes();
869-
if (!channelTransportSocketAddressTypes.containsAll(nameResolverSocketAddressTypes)) {
870-
throw new IllegalArgumentException(String.format(
871-
"Address types of NameResolver '%s' for '%s' not supported by transport",
872-
targetUri.getScheme(), target));
873-
}
883+
return new ResolvedNameResolver(wrap(targetUri), provider);
884+
}
885+
886+
static ResolvedNameResolver getNameResolverProviderNew(
887+
String target, NameResolverRegistry nameResolverRegistry) {
888+
// Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending
889+
// "dns:///".
890+
NameResolverProvider provider = null;
891+
Uri targetUri = null;
892+
StringBuilder uriSyntaxErrors = new StringBuilder();
893+
try {
894+
targetUri = Uri.parse(target);
895+
} catch (URISyntaxException e) {
896+
// Can happen with ip addresses like "[::1]:1234" or 127.0.0.1:1234.
897+
uriSyntaxErrors.append(e.getMessage());
898+
}
899+
if (targetUri != null) {
900+
// For "localhost:8080" this would likely cause provider to be null, because "localhost" is
901+
// parsed as the scheme. Will hit the next case and try "dns:///localhost:8080".
902+
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
903+
}
904+
905+
if (provider == null && !URI_PATTERN.matcher(target).matches()) {
906+
// It doesn't look like a URI target. Maybe it's an authority string. Try with the default
907+
// scheme from the registry.
908+
targetUri =
909+
Uri.newBuilder()
910+
.setScheme(nameResolverRegistry.getDefaultScheme())
911+
.setHost("")
912+
.setPath("/" + target)
913+
.build();
914+
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
915+
}
916+
917+
if (provider == null) {
918+
throw new IllegalArgumentException(
919+
String.format(
920+
"Could not find a NameResolverProvider for %s%s",
921+
target, uriSyntaxErrors.length() > 0 ? " (" + uriSyntaxErrors + ")" : ""));
874922
}
875923

876924
return new ResolvedNameResolver(wrap(targetUri), provider);

core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,11 @@ public void transportDoesNotSupportAddressTypes() {
373373
ManagedChannel unused = grpcCleanupRule.register(builder.build());
374374
fail("Should fail");
375375
} catch (IllegalArgumentException e) {
376-
assertThat(e).hasMessageThat().isEqualTo(
377-
"Address types of NameResolver 'dns' for 'valid:1234' not supported by transport");
376+
assertThat(e)
377+
.hasMessageThat()
378+
.isEqualTo(
379+
"Address types of NameResolver 'dns' for 'dns:///valid:1234' not supported by"
380+
+ " transport");
378381
}
379382
}
380383

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
/*
2+
* Copyright 2015 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.internal;
18+
19+
import static com.google.common.truth.Truth.assertThat;
20+
import static io.grpc.internal.UriWrapper.wrap;
21+
import static org.junit.Assert.fail;
22+
23+
import io.grpc.NameResolver;
24+
import io.grpc.NameResolverProvider;
25+
import io.grpc.NameResolverRegistry;
26+
import io.grpc.Uri;
27+
import java.net.SocketAddress;
28+
import java.net.URI;
29+
import java.util.Collections;
30+
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.junit.runners.JUnit4;
33+
34+
/** Unit tests for ManagedChannelImplBuilder#getNameResolverProviderNew(). */
35+
@RunWith(JUnit4.class)
36+
public class ManagedChannelImplGetNameResolverNewTest {
37+
@Test
38+
public void invalidUriTarget() {
39+
testInvalidTarget("defaultscheme:///[invalid]");
40+
}
41+
42+
@Test
43+
public void validTargetWithInvalidDnsName() throws Exception {
44+
testValidTarget(
45+
"[valid]",
46+
"defaultscheme:///%5Bvalid%5D",
47+
Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("/[valid]").build());
48+
}
49+
50+
@Test
51+
public void validAuthorityTarget() throws Exception {
52+
testValidTarget(
53+
"foo.googleapis.com:8080",
54+
"defaultscheme:///foo.googleapis.com:8080",
55+
Uri.newBuilder()
56+
.setScheme("defaultscheme")
57+
.setHost("")
58+
.setPath("/foo.googleapis.com:8080")
59+
.build());
60+
}
61+
62+
@Test
63+
public void validUriTarget() throws Exception {
64+
testValidTarget(
65+
"scheme:///foo.googleapis.com:8080",
66+
"scheme:///foo.googleapis.com:8080",
67+
Uri.newBuilder()
68+
.setScheme("scheme")
69+
.setHost("")
70+
.setPath("/foo.googleapis.com:8080")
71+
.build());
72+
}
73+
74+
@Test
75+
public void validIpv4AuthorityTarget() throws Exception {
76+
testValidTarget(
77+
"127.0.0.1:1234",
78+
"defaultscheme:///127.0.0.1:1234",
79+
Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("/127.0.0.1:1234").build());
80+
}
81+
82+
@Test
83+
public void validIpv4UriTarget() throws Exception {
84+
testValidTarget(
85+
"dns:///127.0.0.1:1234",
86+
"dns:///127.0.0.1:1234",
87+
Uri.newBuilder().setScheme("dns").setHost("").setPath("/127.0.0.1:1234").build());
88+
}
89+
90+
@Test
91+
public void validIpv6AuthorityTarget() throws Exception {
92+
testValidTarget(
93+
"[::1]:1234",
94+
"defaultscheme:///%5B::1%5D:1234",
95+
Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("/[::1]:1234").build());
96+
}
97+
98+
@Test
99+
public void invalidIpv6UriTarget() throws Exception {
100+
testInvalidTarget("dns:///[::1]:1234");
101+
}
102+
103+
@Test
104+
public void validIpv6UriTarget() throws Exception {
105+
testValidTarget(
106+
"dns:///%5B::1%5D:1234",
107+
"dns:///%5B::1%5D:1234",
108+
Uri.newBuilder().setScheme("dns").setHost("").setPath("/[::1]:1234").build());
109+
}
110+
111+
@Test
112+
public void validTargetStartingWithSlash() throws Exception {
113+
testValidTarget(
114+
"/target",
115+
"defaultscheme:////target",
116+
Uri.newBuilder().setScheme("defaultscheme").setHost("").setPath("//target").build());
117+
}
118+
119+
@Test
120+
public void validTargetNoProvider() {
121+
NameResolverRegistry nameResolverRegistry = new NameResolverRegistry();
122+
try {
123+
ManagedChannelImplBuilder.getNameResolverProviderNew(
124+
"foo.googleapis.com:8080", nameResolverRegistry);
125+
fail("Should fail");
126+
} catch (IllegalArgumentException e) {
127+
// expected
128+
}
129+
}
130+
131+
@Test
132+
public void validTargetProviderAddrTypesNotSupported() {
133+
NameResolverRegistry nameResolverRegistry = getTestRegistry("testscheme");
134+
try {
135+
ManagedChannelImplBuilder.getNameResolverProviderNew(
136+
"testscheme:///foo.googleapis.com:8080", nameResolverRegistry)
137+
.checkAddressTypes(Collections.singleton(CustomSocketAddress.class));
138+
fail("Should fail");
139+
} catch (IllegalArgumentException e) {
140+
assertThat(e).hasMessageThat().isEqualTo(
141+
"Address types of NameResolver 'testscheme' for "
142+
+ "'testscheme:///foo.googleapis.com:8080' not supported by transport");
143+
}
144+
}
145+
146+
private void testValidTarget(String target, String expectedUriString, Uri expectedUri) {
147+
NameResolverRegistry nameResolverRegistry = getTestRegistry(expectedUri.getScheme());
148+
ManagedChannelImplBuilder.ResolvedNameResolver resolved =
149+
ManagedChannelImplBuilder.getNameResolverProviderNew(target, nameResolverRegistry);
150+
assertThat(resolved.provider).isInstanceOf(FakeNameResolverProvider.class);
151+
assertThat(resolved.targetUri).isEqualTo(wrap(expectedUri));
152+
assertThat(resolved.targetUri.toString()).isEqualTo(expectedUriString);
153+
}
154+
155+
private void testInvalidTarget(String target) {
156+
NameResolverRegistry nameResolverRegistry = getTestRegistry("dns");
157+
158+
try {
159+
ManagedChannelImplBuilder.ResolvedNameResolver resolved =
160+
ManagedChannelImplBuilder.getNameResolverProviderNew(target, nameResolverRegistry);
161+
FakeNameResolverProvider nameResolverProvider = (FakeNameResolverProvider) resolved.provider;
162+
fail("Should have failed, but got resolver provider " + nameResolverProvider);
163+
} catch (IllegalArgumentException e) {
164+
// expected
165+
}
166+
}
167+
168+
169+
170+
private static NameResolverRegistry getTestRegistry(String expectedScheme) {
171+
NameResolverRegistry nameResolverRegistry = new NameResolverRegistry();
172+
FakeNameResolverProvider nameResolverProvider = new FakeNameResolverProvider(expectedScheme);
173+
nameResolverRegistry.register(nameResolverProvider);
174+
return nameResolverRegistry;
175+
}
176+
177+
private static class FakeNameResolverProvider extends NameResolverProvider {
178+
final String expectedScheme;
179+
180+
FakeNameResolverProvider(String expectedScheme) {
181+
this.expectedScheme = expectedScheme;
182+
}
183+
184+
@Override
185+
public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
186+
if (expectedScheme.equals(targetUri.getScheme())) {
187+
return new FakeNameResolver(targetUri);
188+
}
189+
return null;
190+
}
191+
192+
@Override
193+
public String getDefaultScheme() {
194+
return expectedScheme;
195+
}
196+
197+
@Override
198+
protected boolean isAvailable() {
199+
return true;
200+
}
201+
202+
@Override
203+
protected int priority() {
204+
return 5;
205+
}
206+
}
207+
208+
private static class FakeNameResolver extends NameResolver {
209+
final URI uri;
210+
211+
FakeNameResolver(URI uri) {
212+
this.uri = uri;
213+
}
214+
215+
@Override public String getServiceAuthority() {
216+
return uri.getAuthority();
217+
}
218+
219+
@Override public void start(final Listener2 listener) {}
220+
221+
@Override public void shutdown() {}
222+
}
223+
224+
private static class CustomSocketAddress extends SocketAddress {}
225+
}

core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import io.grpc.NameResolver;
2424
import io.grpc.NameResolverProvider;
2525
import io.grpc.NameResolverRegistry;
26-
import java.net.InetSocketAddress;
2726
import java.net.SocketAddress;
2827
import java.net.URI;
2928
import java.util.Collections;
@@ -97,8 +96,7 @@ public void validTargetNoProvider() {
9796
NameResolverRegistry nameResolverRegistry = new NameResolverRegistry();
9897
try {
9998
ManagedChannelImplBuilder.getNameResolverProvider(
100-
"foo.googleapis.com:8080", nameResolverRegistry,
101-
Collections.singleton(InetSocketAddress.class));
99+
"foo.googleapis.com:8080", nameResolverRegistry);
102100
fail("Should fail");
103101
} catch (IllegalArgumentException e) {
104102
// expected
@@ -110,8 +108,8 @@ public void validTargetProviderAddrTypesNotSupported() {
110108
NameResolverRegistry nameResolverRegistry = getTestRegistry("testscheme");
111109
try {
112110
ManagedChannelImplBuilder.getNameResolverProvider(
113-
"testscheme:///foo.googleapis.com:8080", nameResolverRegistry,
114-
Collections.singleton(CustomSocketAddress.class));
111+
"testscheme:///foo.googleapis.com:8080", nameResolverRegistry)
112+
.checkAddressTypes(Collections.singleton(CustomSocketAddress.class));
115113
fail("Should fail");
116114
} catch (IllegalArgumentException e) {
117115
assertThat(e).hasMessageThat().isEqualTo(
@@ -123,8 +121,7 @@ public void validTargetProviderAddrTypesNotSupported() {
123121
private void testValidTarget(String target, String expectedUriString, URI expectedUri) {
124122
NameResolverRegistry nameResolverRegistry = getTestRegistry(expectedUri.getScheme());
125123
ManagedChannelImplBuilder.ResolvedNameResolver resolved =
126-
ManagedChannelImplBuilder.getNameResolverProvider(
127-
target, nameResolverRegistry, Collections.singleton(InetSocketAddress.class));
124+
ManagedChannelImplBuilder.getNameResolverProvider(target, nameResolverRegistry);
128125
assertThat(resolved.provider).isInstanceOf(FakeNameResolverProvider.class);
129126
assertThat(resolved.targetUri).isEqualTo(wrap(expectedUri));
130127
assertThat(resolved.targetUri.toString()).isEqualTo(expectedUriString);
@@ -135,8 +132,7 @@ private void testInvalidTarget(String target) {
135132

136133
try {
137134
ManagedChannelImplBuilder.ResolvedNameResolver resolved =
138-
ManagedChannelImplBuilder.getNameResolverProvider(
139-
target, nameResolverRegistry, Collections.singleton(InetSocketAddress.class));
135+
ManagedChannelImplBuilder.getNameResolverProvider(target, nameResolverRegistry);
140136
FakeNameResolverProvider nameResolverProvider = (FakeNameResolverProvider) resolved.provider;
141137
fail("Should have failed, but got resolver provider " + nameResolverProvider);
142138
} catch (IllegalArgumentException e) {

0 commit comments

Comments
 (0)