THRIFT-6158: Classify malformed request arguments as protocol errors (#3748)

Client: rb

Co-authored-by: OpenAI Codex (GPT-5.6) <codex@openai.com>

Author: kpumuk

lib/rb/lib/thrift/processor.rb

@@ -19,9 +19,14 @@
 #
 
 require "logger"
+require "thrift/protocol/base_protocol"
 
 module Thrift
   module Processor
+    class ArgumentProtocolException < ProtocolException
+    end
+    private_constant :ArgumentProtocolException
+
     def initialize(handler, logger = nil)
       @handler = handler
       if logger.nil?
@@ -48,6 +53,9 @@ def process(iprot, oprot)
       if respond_to?("process_#{name}")
         begin
           send("process_#{name}", seqid, iprot, oprot)
+        rescue ArgumentProtocolException => e
+          x = ApplicationException.new(ApplicationException::PROTOCOL_ERROR, e.message)
+          write_error(x, oprot, name, seqid)
         rescue => e
           x = ApplicationException.new(ApplicationException::INTERNAL_ERROR, "Internal error")
           @logger.debug "Internal error : #{e.message}\n#{e.backtrace.join("\n")}"
@@ -68,6 +76,8 @@ def read_args(iprot, args_class)
       args.read(iprot)
       iprot.read_message_end
       args
+    rescue ProtocolException => e
+      raise ArgumentProtocolException.new(e.type, e.message)
     end
 
     def write_result(result, oprot, name, seqid)

lib/rb/spec/processor_spec.rb

@@ -167,6 +167,49 @@ def output_protocol
       end
     end
 
+    it "returns protocol errors for malformed request arguments" do
+      handler = double("Handler")
+      expect(handler).not_to receive(:sleep)
+      processor = SpecNamespace::NonblockingService::Processor.new(handler)
+      input = Thrift::JsonProtocol.new(
+        Thrift::MemoryBufferTransport.new('[1,"sleep",1,17,{"1":{"dbl":x}}]'),
+      )
+      output_transport = Thrift::MemoryBufferTransport.new
+
+      expect(processor.process(input, Thrift::JsonProtocol.new(output_transport))).to be true
+
+      response = Thrift::JsonProtocol.new(output_transport)
+      expect(response.read_message_begin).to eq(["sleep", Thrift::MessageTypes::EXCEPTION, 17])
+      exception = Thrift::ApplicationException.new
+      exception.read(response)
+      response.read_message_end
+
+      expect(exception.type).to eq(Thrift::ApplicationException::PROTOCOL_ERROR)
+      expect(exception.message).to eq('Expected numeric value; got ""')
+    end
+
+    it "keeps protocol exceptions raised by handlers as internal errors" do
+      handler = double("Handler")
+      expect(handler).to receive(:sleep).with(3.0).and_raise(
+        Thrift::ProtocolException.new(Thrift::ProtocolException::INVALID_DATA, "handler failed"),
+      )
+      processor = SpecNamespace::NonblockingService::Processor.new(handler)
+      args = SpecNamespace::NonblockingService::Sleep_args.new(seconds: 3.0)
+      input = input_protocol("sleep", Thrift::MessageTypes::CALL, 18, args)
+      output_transport, output = output_protocol
+
+      expect(processor.process(input, output)).to be true
+
+      response = Thrift::BinaryProtocol.new(output_transport)
+      expect(response.read_message_begin).to eq(["sleep", Thrift::MessageTypes::EXCEPTION, 18])
+      exception = Thrift::ApplicationException.new
+      exception.read(response)
+      response.read_message_end
+
+      expect(exception.type).to eq(Thrift::ApplicationException::INTERNAL_ERROR)
+      expect(exception.message).to eq("Internal error")
+    end
+
     it "should write out a reply when asked" do
       expect(@prot).to receive(:write_message_begin).with("testMessage", Thrift::MessageTypes::REPLY, 23).ordered
       result = double("MockResult")