AVRO-4228: [c++] Fix BinaryDecoder::arrayNext() to handle negative block counts

[gfeyer]

What is the purpose of the change

BinaryDecoder::arrayNext() calls doDecodeLong() directly instead of doDecodeItemCount(), causing it to mishandle negative array block counts. Per the Avro spec, a negative block count means the absolute value is the item count followed by an additional long for the byte-size of the block. When arrayNext() reads a negative count, static_cast<size_t>(-100) produces a huge value and the byte-size long is left unconsumed, corrupting the stream position.

doDecodeItemCount() already handles this correctly and is used by arrayStart(), mapStart(), and mapNext(). Only arrayNext() bypassed it. The fix changes arrayNext() to call doDecodeItemCount() for consistency.

This affects any array large enough to be encoded in multiple blocks with negative counts. ClickHouse independently found the same bug (ClickHouse/ClickHouse#60438, ClickHouse#23).

Verifying this change

• Added testArrayNegativeBlockCount in CodecTests.cc that decodes a hand-crafted array with negative block counts and verifies all items are read correctly
• The fix was also verified against production Avro messages containing arrays with 266+ items encoded in multiple blocks with negative counts, which previously failed ~20% of the time in our systems and now decode correctly.

Documentation

• Does this pull request introduce a new feature? no

[martin-g]

This test pass even without the fix above.
The reason is that the negative count is in the first block and this uses arrayStart() which already delegates to doDecodeItemCount().

Please update the test to use a negative count [also] in the second block.

[martin-g]

This may lead to overflow if result is size_t's min value.
Possible improvement:

Suggested change

	return static_cast<size_t>(-(result + 1)) + 1;

[gfeyer]

In practice this would mean an array with 2^63 items? Quite large for a prod system in my view, but technically it's undefined behavior so you're right, needs fixing, good catch.

[martin-g]

I don't expect that such big number will be used in normal conditions!
But an attacker can craft it manually and pass it to a system to cause problems.

[martin-g]

Suggested change

	BOOST_CHECK_EQUAL(result[0], 10);
	BOOST_CHECK_EQUAL(result[1], 20);
	BOOST_CHECK_EQUAL(result[2], 30);
	BOOST_CHECK_EQUAL(result[3], 40);
	BOOST_CHECK_EQUAL(result[4], 50);
	const std::vector<int32_t> expected = {10, 20, 30, 40, 50};
	BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end());