Skip to content

Commit 7941f54

Browse files
kpumukcodex
kpumuk
and
codex
authored
THRIFT-6153: Expand Ruby Compact varint write coverage (#4017)
Client: rb Assert literal wire bytes and one write per varint across signed width boundaries, sequence IDs, collection sizes, field IDs, and binary lengths. Co-authored-by: OpenAI Codex (gpt-6.1-sol) <codex@openai.com>
1 parent ab0a540 commit 7941f54

1 file changed

Lines changed: 150 additions & 25 deletions

File tree

‎lib/rb/spec/compact_protocol_spec.rb‎

Lines changed: 150 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,162 @@
3434
i64: [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01],
3535
}
3636

37+
INTEGER_VARINT_ENCODINGS = {
38+
0 => [0x00],
39+
-1 => [0x01],
40+
1 => [0x02],
41+
63 => [0x7e],
42+
64 => [0x80, 0x01],
43+
-64 => [0x7f],
44+
-65 => [0x81, 0x01],
45+
150 => [0xac, 0x02],
46+
-150 => [0xab, 0x02],
47+
8191 => [0xfe, 0x7f],
48+
8192 => [0x80, 0x80, 0x01],
49+
-8192 => [0xff, 0x7f],
50+
-8193 => [0x81, 0x80, 0x01],
51+
-32_768 => [0xff, 0xff, 0x03],
52+
32_767 => [0xfe, 0xff, 0x03],
53+
1_048_575 => [0xfe, 0xff, 0x7f],
54+
1_048_576 => [0x80, 0x80, 0x80, 0x01],
55+
-1_048_576 => [0xff, 0xff, 0x7f],
56+
-1_048_577 => [0x81, 0x80, 0x80, 0x01],
57+
134_217_727 => [0xfe, 0xff, 0xff, 0x7f],
58+
134_217_728 => [0x80, 0x80, 0x80, 0x80, 0x01],
59+
-134_217_728 => [0xff, 0xff, 0xff, 0x7f],
60+
-134_217_729 => [0x81, 0x80, 0x80, 0x80, 0x01],
61+
-2_147_483_648 => [0xff, 0xff, 0xff, 0xff, 0x0f],
62+
2_147_483_647 => [0xfe, 0xff, 0xff, 0xff, 0x0f],
63+
17_179_869_183 => [0xfe, 0xff, 0xff, 0xff, 0x7f],
64+
17_179_869_184 => [0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
65+
-17_179_869_184 => [0xff, 0xff, 0xff, 0xff, 0x7f],
66+
-17_179_869_185 => [0x81, 0x80, 0x80, 0x80, 0x80, 0x01],
67+
2_199_023_255_551 => [0xfe, 0xff, 0xff, 0xff, 0xff, 0x7f],
68+
2_199_023_255_552 => [0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
69+
-2_199_023_255_552 => [0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
70+
-2_199_023_255_553 => [0x81, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
71+
281_474_976_710_655 => [0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
72+
281_474_976_710_656 => [0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
73+
-281_474_976_710_656 => [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
74+
-281_474_976_710_657 => [0x81, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
75+
36_028_797_018_963_967 => [0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
76+
36_028_797_018_963_968 => [0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
77+
-36_028_797_018_963_968 => [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
78+
-36_028_797_018_963_969 => [0x81, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
79+
4_611_686_018_427_387_903 => [0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
80+
4_611_686_018_427_387_904 => [0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
81+
-4_611_686_018_427_387_904 => [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f],
82+
-4_611_686_018_427_387_905 => [0x81, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01],
83+
-9_223_372_036_854_775_808 => [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01],
84+
9_223_372_036_854_775_807 => [0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01],
85+
}
86+
3787
VARINT32_SIZE_ENCODINGS = {
88+
0 => [0x00],
89+
15 => [0x0f],
90+
127 => [0x7f],
91+
128 => [0x80, 0x01],
92+
150 => [0x96, 0x01],
93+
16_383 => [0xff, 0x7f],
94+
16_384 => [0x80, 0x80, 0x01],
95+
(2**21) - 1 => [0xff, 0xff, 0x7f],
96+
2**21 => [0x80, 0x80, 0x80, 0x01],
97+
(2**28) - 1 => [0xff, 0xff, 0xff, 0x7f],
98+
2**28 => [0x80, 0x80, 0x80, 0x80, 0x01],
3899
(2**31) - 1 => [0xff, 0xff, 0xff, 0xff, 0x07],
39100
2**31 => [0x80, 0x80, 0x80, 0x80, 0x08],
40101
(2**32) - 1 => [0xff, 0xff, 0xff, 0xff, 0x0f],
41102
}
42103

104+
describe "varint writes" do
105+
let(:writes) { [] }
106+
let(:transport) { double("transport", message_boundaries?: false) }
107+
let(:protocol) { described_class.new(transport) }
108+
109+
before do
110+
allow(transport).to receive(:write) do |buffer|
111+
writes << buffer
112+
nil
113+
end
114+
end
115+
116+
INTEGER_BOUNDARY_TESTS.slice(:i16, :i32, :i64).each do |type, (minimum, maximum)|
117+
it "writes #{type} extrema and every width transition as one binary chunk" do
118+
examples = INTEGER_VARINT_ENCODINGS.select { |value, _| value.between?(minimum, maximum) }
119+
120+
examples.each do |value, expected_bytes|
121+
writes.clear
122+
aggregate_failures("#{type} #{value}") do
123+
protocol.public_send("write_#{type}", value)
124+
expect(writes.map(&:bytes)).to eq([expected_bytes])
125+
expect(writes.first.encoding).to eq(Encoding::BINARY)
126+
end
127+
end
128+
end
129+
end
130+
131+
it "keeps sequence ids unsigned on the wire and separate from envelope chunks" do
132+
{
133+
0 => [0x00],
134+
127 => [0x7f],
135+
128 => [0x80, 0x01],
136+
150 => [0x96, 0x01],
137+
16_383 => [0xff, 0x7f],
138+
16_384 => [0x80, 0x80, 0x01],
139+
(2**21) - 1 => [0xff, 0xff, 0x7f],
140+
2**21 => [0x80, 0x80, 0x80, 0x01],
141+
(2**28) - 1 => [0xff, 0xff, 0xff, 0x7f],
142+
2**28 => [0x80, 0x80, 0x80, 0x80, 0x01],
143+
(2**31) - 1 => [0xff, 0xff, 0xff, 0xff, 0x07],
144+
-2**31 => [0x80, 0x80, 0x80, 0x80, 0x08],
145+
-1 => [0xff, 0xff, 0xff, 0xff, 0x0f],
146+
}.each do |sequence_id, expected_bytes|
147+
writes.clear
148+
protocol.write_message_begin("x", Thrift::MessageTypes::CALL, sequence_id)
149+
expected = [[0x82], [0x21], expected_bytes, [1], [0x78]]
150+
expect(writes.map(&:bytes)).to eq(expected)
151+
end
152+
end
153+
154+
it "batches collection sizes while preserving their type headers" do
155+
[
156+
15, 127, 128, 150, 16_383, 16_384, (2**21) - 1, 2**21, (2**28) - 1, 2**28, (2**31) - 1,
157+
].each do |size|
158+
expected_bytes = VARINT32_SIZE_ENCODINGS.fetch(size)
159+
[:write_list_begin, :write_set_begin].each do |method|
160+
writes.clear
161+
protocol.public_send(method, Thrift::Types::I32, size)
162+
expect(writes.map(&:bytes)).to eq([[0xf5], expected_bytes])
163+
end
164+
writes.clear
165+
protocol.write_map_begin(Thrift::Types::I32, Thrift::Types::I64, size)
166+
expect(writes.map(&:bytes)).to eq([expected_bytes, [0x56]])
167+
end
168+
end
169+
170+
it "batches non-delta field ids after their type header" do
171+
{
172+
-32_768 => [0xff, 0xff, 0x03],
173+
32_767 => [0xfe, 0xff, 0x03],
174+
}.each do |field_id, expected_bytes|
175+
writes.clear
176+
protocol.write_struct_begin("Value")
177+
protocol.write_field_begin("value", Thrift::Types::I32, field_id)
178+
expect(writes.map(&:bytes)).to eq([[0x05], expected_bytes])
179+
protocol.write_struct_end
180+
end
181+
end
182+
183+
it "batches binary lengths while keeping the payload in its own write" do
184+
[0, 127, 128, 150, 16_383, 16_384].each do |size|
185+
writes.clear
186+
payload = "x".b * size
187+
protocol.write_binary(payload)
188+
expect(writes).to eq([VARINT32_SIZE_ENCODINGS.fetch(size).pack("C*"), payload])
189+
end
190+
end
191+
end
192+
43193
TESTS = {
44194
byte: (-127..127).to_a,
45195
i16: (0..14).map { |shift| [1 << shift, -(1 << shift)] }.flatten.sort,
@@ -247,31 +397,6 @@
247397
end
248398
end
249399

250-
it "should batch each native multibyte varint into one transport write" do
251-
probe = Thrift::CompactProtocol.new(Thrift::MemoryBufferTransport.new)
252-
skip "requires the native Compact Protocol" unless probe.native?
253-
254-
recording_transport_class = Class.new(Thrift::BaseTransport) do
255-
attr_reader :writes
256-
257-
def initialize
258-
@writes = []
259-
end
260-
261-
def write(data)
262-
@writes << data
263-
end
264-
end
265-
266-
INTEGER_MINIMUM_ENCODINGS.each_pair do |primitive_type, expected_bytes|
267-
trans = recording_transport_class.new
268-
proto = Thrift::CompactProtocol.new(trans)
269-
270-
proto.send(writer(primitive_type), INTEGER_BOUNDARY_TESTS.fetch(primitive_type).first)
271-
expect(trans.writes).to eq([expected_bytes.pack("C*")])
272-
end
273-
end
274-
275400
it "should use Ruby truthiness when writing bools" do
276401
{
277402
true => Thrift::CompactProtocol::CompactTypes::BOOLEAN_TRUE,

0 commit comments

Comments
 (0)