Skip to content

Commit eac5f4f

Browse files
kpumukcodex
andcommitted
THRIFT-6132: Reset Ruby Header metadata between frames
Client: rb Co-Authored-By: OpenAI Codex (GPT-5.6) <codex@openai.com>
1 parent 9d63c5b commit eac5f4f

2 files changed

Lines changed: 80 additions & 1 deletion

File tree

lib/rb/lib/thrift/transport/header_transport.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ def set_client_type(client_type)
265265

266266
# Reads the next frame, detecting client type on first read
267267
def read_frame(req_sz)
268+
@read_headers = {}
269+
268270
# Read first 4 bytes - could be frame length or protocol magic
269271
first_word = @transport.read_all(4)
270272
frame_size = first_word.unpack('N').first
@@ -367,7 +369,6 @@ def parse_header_format(buf)
367369
transforms << transform_id
368370
end
369371
# Read info headers
370-
@read_headers = {}
371372
while buf.pos < end_of_headers
372373
info_type = read_varint32(buf, end_of_headers)
373374
if info_type == 0

lib/rb/spec/header_transport_spec.rb

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,27 @@
4848
end
4949

5050
describe Thrift::HeaderTransport do
51+
def header_frame(payload, headers = {})
52+
buffer = Thrift::MemoryBufferTransport.new
53+
writer = Thrift::HeaderTransport.new(buffer)
54+
headers.each { |key, value| writer.set_header(key, value) }
55+
writer.write(payload)
56+
writer.flush
57+
buffer.read(buffer.available)
58+
end
59+
60+
def binary_message
61+
[Thrift::BinaryProtocol::VERSION_1 | Thrift::MessageTypes::CALL].pack('N')
62+
end
63+
64+
def compact_message
65+
[0x82, 0x21, 0, 0].pack('C*')
66+
end
67+
68+
def framed(message)
69+
[message.bytesize].pack('N') + message
70+
end
71+
5172
before(:each) do
5273
@underlying = Thrift::MemoryBufferTransport.new
5374
@trans = Thrift::HeaderTransport.new(@underlying)
@@ -284,6 +305,63 @@
284305
expect(headers["request-id"]).to eq("12345")
285306
end
286307

308+
{
309+
"framed binary" => [:binary_message, true],
310+
"unframed binary" => [:binary_message, false],
311+
"framed compact" => [:compact_message, true],
312+
"unframed compact" => [:compact_message, false]
313+
}.each do |legacy_name, (legacy_message, is_framed)|
314+
it "does not carry Header metadata through a #{legacy_name} protocol switch" do
315+
legacy_payload = public_send(legacy_message)
316+
bytes = header_frame("A", "request-id" => "first")
317+
bytes << (is_framed ? framed(legacy_payload) : legacy_payload)
318+
bytes << header_frame("B", "request-id" => "second")
319+
read_trans = Thrift::HeaderTransport.new(Thrift::MemoryBufferTransport.new(bytes))
320+
321+
expect(read_trans.read(1)).to eq("A")
322+
expect(read_trans.get_headers).to eq("request-id" => "first")
323+
324+
read_trans.reset_protocol
325+
expect(read_trans.read(4)).to eq(legacy_payload)
326+
expect(read_trans.get_headers).to eq({})
327+
328+
read_trans.reset_protocol
329+
expect(read_trans.read(1)).to eq("B")
330+
expect(read_trans.get_headers).to eq("request-id" => "second")
331+
end
332+
end
333+
334+
it "keeps metadata empty across multiple legacy frames" do
335+
bytes = header_frame("A", "request-id" => "first")
336+
bytes << framed(binary_message)
337+
bytes << framed(binary_message)
338+
read_trans = Thrift::HeaderTransport.new(Thrift::MemoryBufferTransport.new(bytes))
339+
340+
expect(read_trans.read(1)).to eq("A")
341+
expect(read_trans.get_headers).to eq("request-id" => "first")
342+
343+
2.times do
344+
read_trans.reset_protocol
345+
expect(read_trans.read(4)).to eq(binary_message)
346+
expect(read_trans.get_headers).to eq({})
347+
end
348+
end
349+
350+
it "clears metadata before reporting a malformed following frame" do
351+
malformed_frame = [4].pack('N') + "nope"
352+
bytes = header_frame("A", "request-id" => "first") + malformed_frame
353+
read_trans = Thrift::HeaderTransport.new(Thrift::MemoryBufferTransport.new(bytes))
354+
355+
expect(read_trans.read(1)).to eq("A")
356+
expect(read_trans.get_headers).to eq("request-id" => "first")
357+
358+
expect { read_trans.reset_protocol }.to raise_error(
359+
Thrift::TransportException,
360+
"Could not detect client transport type"
361+
)
362+
expect(read_trans.get_headers).to eq({})
363+
end
364+
287365
it "should decode signed sequence ids from Header frames" do
288366
@trans.sequence_id = -2147483648
289367
@trans.write("payload")

0 commit comments

Comments
 (0)