You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by je...@apache.org on 2018/08/17 03:12:34 UTC

[incubator-dubbo] branch master updated: fix telnet invoke NPE #2218 (#2273)

This is an automated email from the ASF dual-hosted git repository.

jerrick pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-dubbo.git


The following commit(s) were added to refs/heads/master by this push:
     new b2ce7b2  fix telnet invoke NPE #2218 (#2273)
b2ce7b2 is described below

commit b2ce7b2c10d57d232582a185d0877cc404b352ea
Author: Michael Chow <mi...@gmail.com>
AuthorDate: Fri Aug 17 11:12:21 2018 +0800

    fix telnet invoke NPE #2218 (#2273)
    
    * fix issue #2218
    * add some unit tests
---
 .../protocol/dubbo/telnet/InvokeTelnetHandler.java | 13 +++++++
 .../rpc/protocol/dubbo/support/DemoService.java    |  2 ++
 .../protocol/dubbo/support/DemoServiceImpl.java    |  6 +++-
 .../dubbo/telnet/InvokerTelnetHandlerTest.java     | 40 ++++++++++++++++++++++
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java b/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java
index 4464add..d149a50 100644
--- a/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java
+++ b/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java
@@ -60,6 +60,19 @@ public class InvokeTelnetHandler implements TelnetHandler {
         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;
diff --git a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoService.java b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoService.java
index 2668111..357b032 100644
--- a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoService.java
+++ b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoService.java
@@ -57,4 +57,6 @@ public interface DemoService {
 
     NonSerialized returnNonSerialized();
 
+    long add(int a, long b);
+
 }
\ No newline at end of file
diff --git a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoServiceImpl.java b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoServiceImpl.java
index f1ba82e..e404af0 100644
--- a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoServiceImpl.java
+++ b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoServiceImpl.java
@@ -24,7 +24,6 @@ import java.util.Set;
 /**
  * DemoServiceImpl
  */
-
 public class DemoServiceImpl implements DemoService {
     public DemoServiceImpl() {
         super();
@@ -103,4 +102,9 @@ public class DemoServiceImpl implements DemoService {
     public NonSerialized returnNonSerialized() {
         return new NonSerialized();
     }
+
+    public long add(int a, long b) {
+        return a + b;
+    }
+
 }
\ No newline at end of file
diff --git a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokerTelnetHandlerTest.java b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokerTelnetHandlerTest.java
index e1fe2c0..6b1ffe2 100644
--- a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokerTelnetHandlerTest.java
+++ b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokerTelnetHandlerTest.java
@@ -33,6 +33,7 @@ import org.junit.Test;
 
 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;
@@ -70,6 +71,45 @@ public class InvokerTelnetHandlerTest {
 
     @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 {
         mockInvoker = mock(Invoker.class);
         given(mockInvoker.getInterface()).willReturn(DemoService.class);