fix(#50): getInt/decodeCount reject stored values longer than 8 bytes - #65
Merged
Conversation
Previously, both ReadTransaction::getInt() and HighContentionAllocator::decodeCount()
silently truncated values larger than 8 bytes via unpack('P'), which reads only
the first 8 bytes. A malformed or oversized stored value returned a wrong integer
without any error.
Now both methods throw a clear RuntimeException describing the actual stored
length when it exceeds the 8-byte little-endian integer contract.
Changes:
- Extract a testable protected static helper decodeLittleEndianInt() on
ReadTransaction that performs the bounds check; getInt() delegates to it.
- decodeCount() in HighContentionAllocator gains the same explicit length
guard with the same exception semantics.
- New IntDecodeTest covers boundaries: empty, 1, 4, 8, max uint64, >8 (rejected),
long input (rejected), both helpers.
- New integration tests in DatabaseConvenienceTest:
* getIntRoundTripsViaAtomicAdd — atomic add then getInt
* getIntRejectsOversizedStoredValue — oversize value throws
* getIntReadBreakdownBySizeCases — 1/4/8-byte raw packs decode correctly
- CHANGELOG and docs/atomic-operations.md updated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #50
Problem
ReadTransaction::getInt()andHighContentionAllocator::decodeCount()silently truncated stored values larger than 8 bytes viaunpack('P'), which reads only the first 8 bytes. A malformed or oversized stored value was misread without any error — a wrong integer could be returned for any data that was not actual little-endian-packed 8 bytes.Fix
Both decoders now explicitly check the stored length and throw a clear
RuntimeExceptionwhen it exceeds 8 bytes, instead of silently returning a wrong value.ReadTransaction::decodeLittleEndianInt()(protected static) centralizes the bounds-checked decode;getInt()delegates to it.HighContentionAllocator::decodeCount()gets the same explicit length guard with the same exception semantics.Acceptance Criteria
docs/atomic-operations.mddescribes the > 8-byte -> RuntimeException behavior.[Unreleased] / Fixedreferencing [[Bug] getInt/decodeCount silently truncate values longer than 8 bytes #50].tests/Unit/IntDecodeTest.phpcovers boundary cases for both helpers (empty, 1, 4, 8, max-uint64, 9-byte, 100-byte). 339/339 unit tests pass (12 new).tests/Integration/DatabaseConvenienceTestgainsgetIntRoundTripsViaAtomicAdd(atomic add + getInt end-to-end),getIntRejectsOversizedStoredValue, andgetIntReadBreakdownBySizeCases.composer testend-to-end pass — unit tests run locally; integration suite requires the docker FDB cluster (CI).Quality gate
composer lint(PHPCS + Rector + PHPStan level 9) — green.composer test:unit— 339/339 pass, 654 assertions.