kpumuk commented on code in PR #3706:
URL: https://github.com/apache/thrift/pull/3706#discussion_r3741989273


##########
lib/rb/spec/header_transport_spec.rb:
##########
@@ -178,6 +196,85 @@ def framed(message)
         @trans.write("12345")
         expect { @trans.flush }.to raise_error(Thrift::TransportException, 
/frame that is too large/)
       end
+
+      {
+        "binary" => Thrift::BinaryProtocol,
+        "compact" => Thrift::CompactProtocol
+      }.each do |protocol_name, protocol_class|
+        it "enforces max frame size for unframed #{protocol_name} messages" do
+          payload = unframed_message(protocol_class)
+
+          exact_limit = 
Thrift::HeaderTransport.new(Thrift::MemoryBufferTransport.new(payload))
+          exact_limit.set_max_frame_size(payload.bytesize)
+          
expect(read_unframed_message(Thrift::HeaderProtocol.new(exact_limit))).to 
eq("legacy_unframed")
+
+          over_limit = 
Thrift::HeaderTransport.new(Thrift::MemoryBufferTransport.new(payload))
+          over_limit.set_max_frame_size(payload.bytesize - 1)
+          protocol = Thrift::HeaderProtocol.new(over_limit)
+          expect { read_unframed_message(protocol) }.to raise_error(
+            Thrift::TransportException,
+            "Unframed message size exceeds maximum #{payload.bytesize - 1}"
+          ) do |error|
+            expect(error.type).to eq(Thrift::TransportException::SIZE_LIMIT)

Review Comment:
   Thanks. Exact exception-message assertions are an established convention in 
the Ruby specs, including neighboring HeaderTransport cases for undersized and 
fragmented frames. Keeping these assertions exact also verifies that the 
configured limit remains in the diagnostic, while the block independently 
checks `TransportException::SIZE_LIMIT`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to