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

guohao pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/dubbo.git


The following commit(s) were added to refs/heads/3.0 by this push:
     new c1b013e  [3.0-Triple] Fix unary wrapper overload method npe (#9153)
c1b013e is described below

commit c1b013e3da573c9d6804e82dbf3359345c07d48c
Author: earthchen <[email protected]>
AuthorDate: Thu Oct 28 22:02:34 2021 -0500

    [3.0-Triple] Fix unary wrapper overload method npe (#9153)
    
    * fix 9139
    
    * fix not wrapper
    
    * opt
    
    * fix 9139
    
    * fix unary condition
---
 .../rpc/protocol/tri/AbstractServerStream.java     | 139 +++++++++++++++++----
 .../dubbo/rpc/protocol/tri/ServerStream.java       |   3 +-
 .../tri/TripleHttp2FrameServerHandler.java         |   2 +-
 .../dubbo/rpc/protocol/tri/UnaryServerStream.java  |  33 ++---
 4 files changed, 125 insertions(+), 52 deletions(-)

diff --git 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractServerStream.java
 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractServerStream.java
index 17dd26d..47de41d 100644
--- 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractServerStream.java
+++ 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractServerStream.java
@@ -23,6 +23,7 @@ import 
org.apache.dubbo.common.serialize.MultipleSerialization;
 import org.apache.dubbo.remoting.Constants;
 import org.apache.dubbo.rpc.HeaderFilter;
 import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcException;
 import org.apache.dubbo.rpc.RpcInvocation;
 import org.apache.dubbo.rpc.model.FrameworkServiceRepository;
 import org.apache.dubbo.rpc.model.MethodDescriptor;
@@ -130,6 +131,12 @@ public abstract class AbstractServerStream extends 
AbstractStream implements Str
         return providerModel;
     }
 
+    /**
+     * Build the RpcInvocation with metadata and execute headerFilter
+     *
+     * @param metadata request header
+     * @return RpcInvocation
+     */
     protected RpcInvocation buildInvocation(Metadata metadata) {
         RpcInvocation inv = new RpcInvocation(getUrl().getServiceModel(),
             getMethodName(), getServiceDescriptor().getServiceName(),
@@ -139,48 +146,90 @@ public abstract class AbstractServerStream extends 
AbstractStream implements Str
 
         final Map<String, Object> attachments = 
parseMetadataToAttachmentMap(metadata);
         inv.setObjectAttachments(attachments);
+        invokeHeaderFilter(inv);
+        return inv;
+    }
 
+    /**
+     * Intercept the header to do some validation
+     * <p>
+     * for example, check the token or a user-defined permission check 
operation
+     *
+     * @param inv RPC Invocation
+     * @throws RpcException maybe throw rpcException
+     */
+    protected void invokeHeaderFilter(RpcInvocation inv) throws RpcException {
         for (HeaderFilter headerFilter : getHeaderFilters()) {
-            inv = headerFilter.invoke(getInvoker(), inv);
+            headerFilter.invoke(getInvoker(), inv);
         }
-        return inv;
     }
 
-    protected Object[] deserializeRequest(byte[] data) {
+    /**
+     * For the unary method, there may be overloaded methods,
+     * so need to parse out the Wrapper from the data and continue 
buildRpcInvocation
+     * <p>
+     * Also, to prevent serialization attacks, headerFilter needs to be 
executed
+     *
+     * @param metadata request headers
+     * @param data     request data
+     * @return RPC Invocation
+     */
+    protected RpcInvocation buildUnaryInvocation(Metadata metadata, byte[] 
data) {
         ClassLoader tccl = Thread.currentThread().getContextClassLoader();
         try {
             if (getProviderModel() != null) {
                 
ClassLoadUtil.switchContextLoader(getProviderModel().getServiceInterfaceClass().getClassLoader());
             }
-            if (getMethodDescriptor() == null || 
getMethodDescriptor().isNeedWrap()) {
-                final TripleWrapper.TripleRequestWrapper wrapper = unpack(data,
-                    TripleWrapper.TripleRequestWrapper.class);
-                if 
(!getSerializeType().equals(convertHessianFromWrapper(wrapper.getSerializeType())))
 {
-                    
transportError(GrpcStatus.fromCode(GrpcStatus.Code.INVALID_ARGUMENT)
-                        .withDescription("Received inconsistent serialization 
type from client, " +
-                            "reject to deserialize! Expected:" + 
getSerializeType() +
-                            " Actual:" + 
convertHessianFromWrapper(wrapper.getSerializeType())));
+            // For the Wrapper method,the methodDescriptor needs to get from 
data, so parse the request first
+            if (needDeserializeWrapper(getMethodDescriptor())) {
+                // the wrapper structure is first resolved without actual 
deserialization
+                TripleWrapper.TripleRequestWrapper wrapper = 
deserializeWrapperSetMdIfNeed(data);
+                if (wrapper == null) {
                     return null;
                 }
-                if (getMethodDescriptor() == null) {
-                    final String[] paramTypes = 
wrapper.getArgTypesList().toArray(new String[wrapper.getArgsCount()]);
-                    // wrapper mode the method can overload so maybe list
-                    for (MethodDescriptor descriptor : getMethodDescriptors()) 
{
-                        // params type is array
-                        if 
(Arrays.equals(descriptor.getCompatibleParamSignatures(), paramTypes)) {
-                            method(descriptor);
-                            break;
-                        }
-                    }
-                    if (getMethodDescriptor() == null) {
-                        
transportError(GrpcStatus.fromCode(GrpcStatus.Code.UNIMPLEMENTED)
-                            .withDescription("Method :" + getMethodName() + 
"[" + Arrays.toString(paramTypes) + "] " +
-                                "not found of service:" + 
getServiceDescriptor().getServiceName()));
-                        return null;
-                    }
+                RpcInvocation inv = buildInvocation(metadata);
+                inv.setArguments(unwrapReq(getUrl(), wrapper, 
getMultipleSerialization()));
+                return inv;
+            } else {
+                // Protobuf MethodDescriptor must not be null
+                RpcInvocation inv = buildInvocation(metadata);
+                inv.setArguments(new Object[]{unpack(data, 
getMethodDescriptor().getParameterClasses()[0])});
+                return inv;
+            }
+        } catch (RpcException rpcException) {
+            // for catch exceptions in headerFilter
+            transportError(GrpcStatus.getStatus(rpcException, 
rpcException.getMessage()));
+            return null;
+        } catch (Throwable throwable) {
+            LOGGER.warn("Decode request failed:", throwable);
+            transportError(GrpcStatus.fromCode(GrpcStatus.Code.INTERNAL)
+                .withDescription("Decode request failed:" + 
throwable.getMessage()));
+            return null;
+        } finally {
+            ClassLoadUtil.switchContextLoader(tccl);
+        }
+    }
+
+    /**
+     * Deserialize the stream request data
+     *
+     * @param data request data
+     * @return Deserialized object
+     */
+    protected Object[] deserializeRequest(byte[] data) {
+        ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+        try {
+            if (getProviderModel() != null) {
+                
ClassLoadUtil.switchContextLoader(getProviderModel().getServiceInterfaceClass().getClassLoader());
+            }
+            if (needDeserializeWrapper(getMethodDescriptor())) {
+                TripleWrapper.TripleRequestWrapper wrapper = 
deserializeWrapperSetMdIfNeed(data);
+                if (wrapper == null) {
+                    return null;
                 }
                 return unwrapReq(getUrl(), wrapper, 
getMultipleSerialization());
             } else {
+                // Protobuf MethodDescriptor must not be null
                 return new Object[]{unpack(data, 
getMethodDescriptor().getParameterClasses()[0])};
             }
         } catch (Throwable throwable) {
@@ -193,6 +242,42 @@ public abstract class AbstractServerStream extends 
AbstractStream implements Str
         }
     }
 
+    private boolean needDeserializeWrapper(MethodDescriptor md) {
+        if (md == null) {
+            return true;
+        }
+        return getMethodDescriptor().isNeedWrap();
+    }
+
+    private TripleWrapper.TripleRequestWrapper 
deserializeWrapperSetMdIfNeed(byte[] data) {
+        final TripleWrapper.TripleRequestWrapper wrapper = unpack(data, 
TripleWrapper.TripleRequestWrapper.class);
+        if 
(!getSerializeType().equals(convertHessianFromWrapper(wrapper.getSerializeType())))
 {
+            
transportError(GrpcStatus.fromCode(GrpcStatus.Code.INVALID_ARGUMENT)
+                .withDescription("Received inconsistent serialization type 
from client, " +
+                    "reject to deserialize! Expected:" + getSerializeType() +
+                    " Actual:" + 
convertHessianFromWrapper(wrapper.getSerializeType())));
+            return null;
+        }
+        if (getMethodDescriptor() == null) {
+            final String[] paramTypes = wrapper.getArgTypesList().toArray(new 
String[wrapper.getArgsCount()]);
+            // wrapper mode the method can overload so maybe list
+            for (MethodDescriptor descriptor : getMethodDescriptors()) {
+                // params type is array
+                if (Arrays.equals(descriptor.getCompatibleParamSignatures(), 
paramTypes)) {
+                    method(descriptor);
+                    break;
+                }
+            }
+            if (getMethodDescriptor() == null) {
+                
transportError(GrpcStatus.fromCode(GrpcStatus.Code.UNIMPLEMENTED)
+                    .withDescription("Method :" + getMethodName() + "[" + 
Arrays.toString(paramTypes) + "] " +
+                        "not found of service:" + 
getServiceDescriptor().getServiceName()));
+                return null;
+            }
+        }
+        return wrapper;
+    }
+
     private Object[] unwrapReq(URL url, TripleWrapper.TripleRequestWrapper 
wrap,
                                MultipleSerialization multipleSerialization) {
         String serializeType = 
convertHessianFromWrapper(wrap.getSerializeType());
diff --git 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ServerStream.java
 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ServerStream.java
index 5623b33..f5ff7c9 100644
--- 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ServerStream.java
+++ 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ServerStream.java
@@ -22,7 +22,6 @@ import org.apache.dubbo.common.stream.StreamObserver;
 import org.apache.dubbo.rpc.Result;
 import org.apache.dubbo.rpc.RpcContext;
 import org.apache.dubbo.rpc.RpcInvocation;
-import org.apache.dubbo.rpc.model.MethodDescriptor;
 
 public class ServerStream extends AbstractServerStream implements Stream {
     protected ServerStream(URL url) {
@@ -107,7 +106,7 @@ public class ServerStream extends AbstractServerStream 
implements Stream {
         @Override
         public void onMetadata(Metadata metadata, boolean endStream) {
             super.onMetadata(metadata, endStream);
-            if (getMethodDescriptor().getRpcType() == 
MethodDescriptor.RpcType.SERVER_STREAM) {
+            if (getMethodDescriptor().isServerStream()) {
                 return;
             }
             execute(() -> {
diff --git 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2FrameServerHandler.java
 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2FrameServerHandler.java
index f450e47..ab232f7 100644
--- 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2FrameServerHandler.java
+++ 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2FrameServerHandler.java
@@ -225,7 +225,7 @@ public class TripleHttp2FrameServerHandler extends 
ChannelDuplexHandler {
             }
         }
 
-        boolean isUnary = methodDescriptor != null && 
methodDescriptor.isUnary();
+        boolean isUnary = methodDescriptor == null || 
methodDescriptor.isUnary();
         final AbstractServerStream stream = 
AbstractServerStream.newServerStream(invoker.getUrl(), isUnary);
 
         Channel channel = ctx.channel();
diff --git 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryServerStream.java
 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryServerStream.java
index 15be193..21c418a 100644
--- 
a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryServerStream.java
+++ 
b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryServerStream.java
@@ -53,31 +53,20 @@ public class UnaryServerStream extends AbstractServerStream 
implements Stream {
 
         @Override
         public void onComplete() {
-            if (getData() != null) {
-                invoke();
-            } else {
-                onError(GrpcStatus.fromCode(GrpcStatus.Code.INTERNAL)
-                    .withDescription("Missing request data"));
-            }
+            execute(() -> {
+                if (getData() != null) {
+                    invoke();
+                } else {
+                    onError(GrpcStatus.fromCode(GrpcStatus.Code.INTERNAL)
+                        .withDescription("Missing request data"));
+                }
+            });
         }
 
         public void invoke() {
-            RpcInvocation invocation;
-            if (getMethodDescriptor().isNeedWrap()) {
-                // For wrapper overload methods, the methodDescriptor needs to 
get from data, so parse the request first
-                final Object[] arguments = deserializeRequest(getData());
-                if (arguments == null) {
-                    return;
-                }
-                invocation = buildInvocation(getHeaders());
-                invocation.setArguments(arguments);
-            } else {
-                invocation = buildInvocation(getHeaders());
-                final Object[] arguments = deserializeRequest(getData());
-                if (arguments == null) {
-                    return;
-                }
-                invocation.setArguments(arguments);
+            RpcInvocation invocation = buildUnaryInvocation(getHeaders(), 
getData());
+            if (invocation == null) {
+                return;
             }
             final Result result = getInvoker().invoke(invocation);
             CompletionStage<Object> future = 
result.thenApply(Function.identity());

Reply via email to