Skip to content

Commit 3508f0e

Browse files
gfeyerGabriel Feyer
andauthored
AVRO-4228: [c++] Fix BinaryDecoder::arrayNext() to handle negative block counts (#3646)
* AVRO-4228: Fix BinaryDecoder::arrayNext() to handle negative block counts * AVRO-4228: Add test for arrayNext() with negative block counts * AVRO-4228: Move negative block count to second block in test * AVRO-4228: Avoid undefined behavior when negating INT64_MIN in doDecodeItemCount --------- Co-authored-by: Gabriel Feyer <gabriel.feyer@indexexchange.com>
1 parent 58d20ab commit 3508f0e

2 files changed

Lines changed: 37 additions & 2 deletions

File tree

lang/c++/impl/BinaryDecoder.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,13 @@ size_t BinaryDecoder::doDecodeItemCount() {
164164
auto result = doDecodeLong();
165165
if (result < 0) {
166166
doDecodeLong();
167-
return static_cast<size_t>(-result);
167+
return static_cast<size_t>(-(result + 1)) + 1;
168168
}
169169
return static_cast<size_t>(result);
170170
}
171171

172172
size_t BinaryDecoder::arrayNext() {
173-
return static_cast<size_t>(doDecodeLong());
173+
return doDecodeItemCount();
174174
}
175175

176176
size_t BinaryDecoder::skipArray() {

lang/c++/test/CodecTests.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,6 +2100,40 @@ static void testJsonCodecReinit() {
21002100
}
21012101
}
21022102

2103+
static void testArrayNegativeBlockCount() {
2104+
// Array of ints [10, 20, 30, 40, 50] encoded with a negative block count
2105+
// in the second block, which exercises arrayNext().
2106+
// Per the Avro spec, a negative count means: abs(count) items follow,
2107+
// preceded by a long byte-size of the block.
2108+
//
2109+
// Block 1: count=2, items: 10, 20 (read by arrayStart)
2110+
// Block 2: count=-3, bytesize=3, items: 30, 40, 50 (read by arrayNext)
2111+
// Terminal: count=0
2112+
const uint8_t data[] = {
2113+
0x04, // zigzag(2) = 4: block count = 2
2114+
0x14, 0x28, // zigzag ints: 10, 20
2115+
0x05, // zigzag(-3) = 5: block count = -3
2116+
0x06, // zigzag(3) = 6: byte-size of block
2117+
0x3c, 0x50, 0x64, // zigzag ints: 30, 40, 50
2118+
0x00 // terminal
2119+
};
2120+
2121+
InputStreamPtr is = memoryInputStream(data, sizeof(data));
2122+
DecoderPtr d = binaryDecoder();
2123+
d->init(*is);
2124+
2125+
std::vector<int32_t> result;
2126+
for (size_t n = d->arrayStart(); n != 0; n = d->arrayNext()) {
2127+
for (size_t i = 0; i < n; ++i) {
2128+
result.push_back(d->decodeInt());
2129+
}
2130+
}
2131+
2132+
const std::vector<int32_t> expected = {10, 20, 30, 40, 50};
2133+
BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(),
2134+
expected.begin(), expected.end());
2135+
}
2136+
21032137
static void testByteCount() {
21042138
OutputStreamPtr os1 = memoryOutputStream();
21052139
EncoderPtr e1 = binaryEncoder();
@@ -2125,6 +2159,7 @@ init_unit_test_suite(int, char *[]) {
21252159
ts->add(BOOST_PARAM_TEST_CASE(&avro::testJson, avro::jsonData,
21262160
ENDOF(avro::jsonData)));
21272161
ts->add(BOOST_TEST_CASE(avro::testJsonCodecReinit));
2162+
ts->add(BOOST_TEST_CASE(avro::testArrayNegativeBlockCount));
21282163
ts->add(BOOST_TEST_CASE(avro::testByteCount));
21292164

21302165
return ts;

0 commit comments

Comments
 (0)