Skip to content

Commit a4e481b

Browse files
carl-mastrangelocarl-mastrangelo
authored
core: make ServiceDescriptor check input values
Fixes: grpc#2357
1 parent 737cd16 commit a4e481b

File tree

3 files changed

+122
-2
lines changed

3 files changed

+122
-2
lines changed

‎core/src/main/java/io/grpc/ServerServiceDefinition.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ public static Builder builder(ServiceDescriptor serviceDescriptor) {
5959
private ServerServiceDefinition(
6060
ServiceDescriptor serviceDescriptor, Map<String, ServerMethodDefinition<?, ?>> methods) {
6161
this.serviceDescriptor = checkNotNull(serviceDescriptor, "serviceDescriptor");
62-
this.methods = Collections.unmodifiableMap(
63-
new HashMap<String, ServerMethodDefinition<?, ?>>(methods));
62+
this.methods =
63+
Collections.unmodifiableMap(new HashMap<String, ServerMethodDefinition<?, ?>>(methods));
6464
}
6565

6666
/**

‎core/src/main/java/io/grpc/ServiceDescriptor.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,16 @@
3131

3232
package io.grpc;
3333

34+
import static com.google.common.base.Preconditions.checkArgument;
35+
3436
import com.google.common.base.Preconditions;
3537

3638
import java.util.ArrayList;
3739
import java.util.Arrays;
3840
import java.util.Collection;
3941
import java.util.Collections;
42+
import java.util.HashSet;
43+
import java.util.Set;
4044

4145
import javax.annotation.Nullable;
4246

@@ -66,6 +70,8 @@ public ServiceDescriptor(String name, Object marshallerDescriptor,
6670
public ServiceDescriptor(String name, Object marshallerDescriptor,
6771
Collection<MethodDescriptor<?, ?>> methods) {
6872
this.name = Preconditions.checkNotNull(name, "name");
73+
Preconditions.checkNotNull(methods, "methods");
74+
validateMethodNames(name, methods);
6975
this.marshallerDescriptor = marshallerDescriptor;
7076
this.methods = Collections.unmodifiableList(new ArrayList<MethodDescriptor<?, ?>>(methods));
7177
}
@@ -92,4 +98,16 @@ public String getName() {
9298
public Object getMarshallerDescriptor() {
9399
return marshallerDescriptor;
94100
}
101+
102+
static void validateMethodNames(String serviceName, Collection<MethodDescriptor<?, ?>> methods) {
103+
Set<String> allNames = new HashSet<String>(methods.size());
104+
for (MethodDescriptor<?, ?> method : methods) {
105+
String methodServiceName =
106+
MethodDescriptor.extractFullServiceName(method.getFullMethodName());
107+
checkArgument(serviceName.equals(methodServiceName),
108+
"service names %s != %s", methodServiceName, serviceName);
109+
checkArgument(allNames.add(method.getFullMethodName()),
110+
"duplicate name %s", method.getFullMethodName());
111+
}
112+
}
95113
}

‎core/src/test/java/io/grpc/ServiceDescriptorTest.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright 2017, Google Inc. All rights reserved.
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions are
6+
* met:
7+
*
8+
* * Redistributions of source code must retain the above copyright
9+
* notice, this list of conditions and the following disclaimer.
10+
* * Redistributions in binary form must reproduce the above
11+
* copyright notice, this list of conditions and the following disclaimer
12+
* in the documentation and/or other materials provided with the
13+
* distribution.
14+
*
15+
* * Neither the name of Google Inc. nor the names of its
16+
* contributors may be used to endorse or promote products derived from
17+
* this software without specific prior written permission.
18+
*
19+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
20+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
21+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
22+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
23+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
24+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
25+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
26+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
27+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
28+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
29+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
30+
*/
31+
32+
package io.grpc;
33+
34+
import io.grpc.MethodDescriptor.MethodType;
35+
import io.grpc.testing.TestMethodDescriptors;
36+
37+
import org.junit.Rule;
38+
import org.junit.Test;
39+
import org.junit.rules.ExpectedException;
40+
import org.junit.runner.RunWith;
41+
import org.junit.runners.JUnit4;
42+
43+
import java.util.Arrays;
44+
import java.util.Collection;
45+
import java.util.Collections;
46+
47+
/**
48+
* Tests for {@link ServiceDescriptor}.
49+
*/
50+
@RunWith(JUnit4.class)
51+
public class ServiceDescriptorTest {
52+
53+
@Rule
54+
public final ExpectedException thrown = ExpectedException.none();
55+
56+
@Test
57+
public void failsOnNullName() {
58+
thrown.expect(NullPointerException.class);
59+
thrown.expectMessage("name");
60+
61+
new ServiceDescriptor(null, Collections.emptyList());
62+
}
63+
64+
@Test
65+
public void failsOnNullMethods() {
66+
thrown.expect(NullPointerException.class);
67+
thrown.expectMessage("methods");
68+
69+
new ServiceDescriptor("name", (Collection<MethodDescriptor<?, ?>>)null);
70+
}
71+
72+
@Test
73+
public void failsOnNonMatchingNames() {
74+
thrown.expect(IllegalArgumentException.class);
75+
thrown.expectMessage("service names");
76+
77+
new ServiceDescriptor("name", Collections.<MethodDescriptor<?, ?>>singletonList(
78+
MethodDescriptor.create(
79+
MethodType.UNARY,
80+
MethodDescriptor.generateFullMethodName("wrongservice", "method"),
81+
TestMethodDescriptors.noopMarshaller(),
82+
TestMethodDescriptors.noopMarshaller())));
83+
}
84+
85+
@Test
86+
public void failsOnNonDuplicateNames() {
87+
thrown.expect(IllegalArgumentException.class);
88+
thrown.expectMessage("duplicate");
89+
90+
new ServiceDescriptor("name", Arrays.<MethodDescriptor<?, ?>>asList(
91+
MethodDescriptor.create(
92+
MethodType.UNARY,
93+
MethodDescriptor.generateFullMethodName("name", "method"),
94+
TestMethodDescriptors.noopMarshaller(),
95+
TestMethodDescriptors.noopMarshaller()),
96+
MethodDescriptor.create(
97+
MethodType.UNARY,
98+
MethodDescriptor.generateFullMethodName("name", "method"),
99+
TestMethodDescriptors.noopMarshaller(),
100+
TestMethodDescriptors.noopMarshaller())));
101+
}
102+
}

0 commit comments

Comments
 (0)