Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -24,7 +24,6 @@
/**
* DemoServiceImpl
*/

public class DemoServiceImpl implements DemoService {
public DemoServiceImpl() {
super();
Expand Down Expand Up @@ -103,4 +102,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,45 @@ public void testInvokeDefaultSService() throws RemotingException {
assertTrue(result.contains("Use default service org.apache.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 here.");
} 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 here.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the message right ? "It shouldn't cause a NullPointerException here."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message is right, because we expect the code invoke.telnet(mockChannel, "DemoService.add(1, null)") to throw a NullPointerException. I think it was the word "here" made you confused, so I changed the word "here" to "by the above code" now. Please review it again.

} 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 here.");
}
}

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