fix decoding of provider error response (envoyproxy#19361)

There is no response type in the non `Ok` dubbo response. So we should not try to decode response type from non `Ok` dubbo response to avoid the `Cannot parse RpcResult type from buffer` exception. Risk Level: Low. Testing: Added. Docs Changes: N/A. Release Notes: N/A. Platform Specific Features: N/A. Signed-off-by: wbpcode <[email protected]>
twghu · Jan 14, 2022 · 0a4e3d9 · 0a4e3d9
1 parent 10e77b8
commit 0a4e3d9
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 0 deletions.
diff --git a/source/extensions/filters/network/dubbo_proxy/dubbo_protocol_impl.cc b/source/extensions/filters/network/dubbo_proxy/dubbo_protocol_impl.cc
@@ -153,6 +153,11 @@ bool DubboProtocolImpl::decodeData(Buffer::Instance& buffer, ContextSharedPtr co
     break;
   }
   case MessageType::Response: {
+    // Non `Ok` response body has no response type info and skip deserialization.
+    if (metadata->responseStatus() != ResponseStatus::Ok) {
+      metadata->setMessageType(MessageType::Exception);
+      break;
+    }
     auto ret = serializer_->deserializeRpcResult(buffer, context);
     if (!ret.second) {
       return false;

diff --git a/test/extensions/filters/network/dubbo_proxy/dubbo_protocol_impl_test.cc b/test/extensions/filters/network/dubbo_proxy/dubbo_protocol_impl_test.cc
@@ -1,4 +1,5 @@
 #include "source/extensions/filters/network/dubbo_proxy/dubbo_protocol_impl.h"
+#include "source/extensions/filters/network/dubbo_proxy/hessian_utils.h"
 #include "source/extensions/filters/network/dubbo_proxy/protocol.h"
 
 #include "test/extensions/filters/network/dubbo_proxy/mocks.h"
@@ -33,6 +34,7 @@ TEST(DubboProtocolImplTest, Name) {
 
 TEST(DubboProtocolImplTest, Normal) {
   DubboProtocolImpl dubbo_protocol;
+  dubbo_protocol.initSerializer(SerializationType::Hessian2);
   // Normal dubbo request message
   {
     Buffer::OwnedImpl buffer;
@@ -63,6 +65,69 @@ TEST(DubboProtocolImplTest, Normal) {
     EXPECT_EQ(1, context->bodySize());
     EXPECT_EQ(MessageType::Response, metadata->messageType());
   }
+
+  // Normal dubbo response with exception.
+  {
+    Buffer::OwnedImpl buffer;
+    Buffer::OwnedImpl body_buffer;
+    buffer.add(std::string({'\xda', '\xbb', 0x42, 20}));
+    addInt64(buffer, 1);
+
+    Hessian2::Encoder encoder(std::make_unique<BufferWriter>(body_buffer));
+    // Encode the fake response type. `0` means the response is an exception without attachments.
+    encoder.encode<int32_t>(0);
+    encoder.encode<std::string>("fake_exception");
+
+    auto body_size = body_buffer.length();
+    addInt32(buffer, body_size);
+    buffer.move(body_buffer);
+
+    MessageMetadataSharedPtr metadata = std::make_shared<MessageMetadata>();
+    auto result = dubbo_protocol.decodeHeader(buffer, metadata);
+    auto context = result.first;
+    EXPECT_TRUE(result.second);
+    EXPECT_EQ(1, metadata->requestId());
+    EXPECT_EQ(body_size, context->bodySize());
+    EXPECT_EQ(MessageType::Response, metadata->messageType());
+
+    context->originMessage().move(buffer, context->headerSize());
+
+    auto body_result = dubbo_protocol.decodeData(buffer, context, metadata);
+
+    EXPECT_TRUE(body_result);
+    EXPECT_EQ(MessageType::Exception, metadata->messageType());
+  }
+
+  // Normal dubbo response with error.
+  {
+    Buffer::OwnedImpl buffer;
+    Buffer::OwnedImpl body_buffer;
+    buffer.add(std::string({'\xda', '\xbb', 0x42, 40}));
+    addInt64(buffer, 1);
+
+    Hessian2::Encoder encoder(std::make_unique<BufferWriter>(body_buffer));
+    // No response type in the body for non `20` response.
+    encoder.encode<std::string>("error_string");
+
+    auto body_size = body_buffer.length();
+    addInt32(buffer, body_size);
+    buffer.move(body_buffer);
+
+    MessageMetadataSharedPtr metadata = std::make_shared<MessageMetadata>();
+    auto result = dubbo_protocol.decodeHeader(buffer, metadata);
+    auto context = result.first;
+    EXPECT_TRUE(result.second);
+    EXPECT_EQ(1, metadata->requestId());
+    EXPECT_EQ(body_size, context->bodySize());
+    EXPECT_EQ(MessageType::Response, metadata->messageType());
+
+    context->originMessage().move(buffer, context->headerSize());
+
+    auto body_result = dubbo_protocol.decodeData(buffer, context, metadata);
+
+    EXPECT_TRUE(body_result);
+    EXPECT_EQ(MessageType::Exception, metadata->messageType());
+  }
 }
 
 TEST(DubboProtocolImplTest, InvalidProtocol) {