AVRO-4276: [C++] Fix INT64_MIN overflow in doDecodeItemCount()#3843
AVRO-4276: [C++] Fix INT64_MIN overflow in doDecodeItemCount()#3843iemejia wants to merge 1 commit into
Conversation
iemejia
changed the title
iemejia
force-pushed
the
avro-4275-cpp-int64min-overflow
branch
from
399568b to
5855770
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a C++ undefined-behavior edge case in BinaryDecoder::doDecodeItemCount() where a zigzag-decoded block count of INT64_MIN could trigger signed overflow and yield an invalid item count.
Changes:
- Add an explicit
INT64_MINguard inBinaryDecoder::doDecodeItemCount()and throw anExceptioninstead of overflowing. - Simplify negative-count handling to use
-resultfor all other negative values. - Add unit tests to ensure
arrayStart()/mapStart()reject a zigzag-varint encoding ofINT64_MIN.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lang/c++/impl/BinaryDecoder.cc | Adds an INT64_MIN rejection path and avoids signed overflow when converting negative block counts to size_t. |
| lang/c++/test/CodecTests.cc | Adds regression tests covering INT64_MIN as an array/map block count to ensure an exception is thrown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (result == INT64_MIN) { | ||
| throw Exception("Invalid negative block count"); | ||
| } |
There was a problem hiding this comment.
Fixed — now includes the offending value in the message ("Invalid negative block count: {}") consistent with other exceptions in this file.
Replace the -(result + 1) + 1 idiom (which still overflows on the final + 1 when result == INT64_MIN) with an explicit INT64_MIN guard that throws an exception, followed by simple negation for all other negative values. Add unit tests for both arrayStart() and mapStart() to verify that a zigzag-encoded INT64_MIN block count is properly rejected. Assisted-by: GitHub Copilot:claude-opus-4.6
iemejia
force-pushed
the
avro-4275-cpp-int64min-overflow
branch
from
5855770 to
fa3e46e
Compare
|
@martin-g @RyanSkraba apparently the previous fix for AVRO-4228 #3646 was missing one corner case. @RyanSkraba both of those fixes should be cherry-picked into the next 1.12 release |
Summary
Fix signed integer overflow in
BinaryDecoder::doDecodeItemCount()when the decoded block count equalsINT64_MIN.Problem
The existing negation idiom
-(result + 1) + 1still overflows on the final+ 1whenresult == INT64_MIN(since-INT64_MINis not representable inint64_t). This is undefined behavior in C++ and could result in the decoder returning2^63as the item count for both arrays and maps.Fix
result == INT64_MIN, throw anExceptionimmediately.-resultwhich is well-defined.Tests
Added two new test cases:
testArrayInt64MinBlockCount— verifiesarrayStart()throws on a zigzag-encodedINT64_MINblock count.testMapInt64MinBlockCount— verifiesmapStart()throws on the same input.This is the C++ counterpart to the C fix in commit 271de63.