Skip to content

Commit

Permalink
fix telnet invoke NPE #2218 (#2273)
Browse files Browse the repository at this point in the history
* fix issue #2218
* add some unit tests
  • Loading branch information
mizhoux authored and diecui1202 committed Sep 6, 2018
1 parent 8ae95c9 commit 2ca0af7
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ private static boolean isMatch(Class<?>[] types, List<Object> args) {
for (int i = 0; i < types.length; i++) {
Class<?> type = types[i];
Object arg = args.get(i);

if (arg == null) {
// if the type is primitive, the method to invoke will cause NullPointerException definitely
// so we can offer a specified error message to the invoker in advance and avoid unnecessary invoking
if (type.isPrimitive()) {
throw new NullPointerException(String.format(
"The type of No.%d parameter is primitive(%s), but the value passed is null.", i + 1, type.getName()));
}

// if the type is not primitive, we choose to believe what the invoker want is a null value
continue;
}

if (ReflectUtils.isPrimitive(arg.getClass())) {
if (!ReflectUtils.isPrimitive(type)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@ public interface DemoService {

NonSerialized returnNonSerialized();

long add(int a, long b);

}
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,9 @@ public void nonSerializedParameter(NonSerialized ns) {
public NonSerialized returnNonSerialized() {
return new NonSerialized();
}

public long add(int a, long b) {
return a + b;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -68,6 +69,41 @@ public void testInvokeDefaultSService() throws RemotingException {
assertTrue(result.contains("Use default service com.alibaba.dubbo.rpc.protocol.dubbo.support.DemoService.\r\n\"ok\"\r\n"));
}

@SuppressWarnings("unchecked")
@Test
public void testInvokeByPassingNullValue() throws RemotingException {
mockInvoker = mock(Invoker.class);
given(mockInvoker.getInterface()).willReturn(DemoService.class);
given(mockInvoker.getUrl()).willReturn(URL.valueOf("dubbo://127.0.0.1:20883/demo"));
given(mockInvoker.invoke(any(Invocation.class))).willReturn(new RpcResult("ok"));
mockChannel = mock(Channel.class);
given(mockChannel.getAttribute("telnet.service")).willReturn("org.apache.dubbo.rpc.protocol.dubbo.support.DemoService");
given(mockChannel.getLocalAddress()).willReturn(NetUtils.toAddress("127.0.0.1:5555"));
given(mockChannel.getRemoteAddress()).willReturn(NetUtils.toAddress("127.0.0.1:20883"));
DubboProtocol.getDubboProtocol().export(mockInvoker);
// pass null value to parameter of primitive type
try {
invoke.telnet(mockChannel, "DemoService.add(null, 2)");
fail("It should cause a NullPointerException by the above code.");
} catch (NullPointerException ex) {
String message = ex.getMessage();
assertEquals("The type of No.1 parameter is primitive(int), but the value passed is null.", message);
}
try {
invoke.telnet(mockChannel, "DemoService.add(1, null)");
fail("It should cause a NullPointerException by the above code.");
} catch (NullPointerException ex) {
String message = ex.getMessage();
assertEquals("The type of No.2 parameter is primitive(long), but the value passed is null.", message);
}
// pass null value to parameter of object type
try {
invoke.telnet(mockChannel, "DemoService.sayHello(null)");
} catch (NullPointerException ex) {
fail("It shouldn't cause a NullPointerException by the above code.");
}
}

@SuppressWarnings("unchecked")
@Test
public void testInvokeAutoFindMethod() throws RemotingException {
Expand Down

0 comments on commit 2ca0af7

Please sign in to comment.