[common] Reject out-of-range range-bitmap chunk-size option - #9765
Draft
LuciferYang wants to merge 5 commits into
Draft
[common] Reject out-of-range range-bitmap chunk-size option#9765LuciferYang wants to merge 5 commits into
LuciferYang wants to merge 5 commits into
Conversation
RangeBitmapFileIndex parsed the user-facing chunk-size option with (int) MemorySize.parse(...).getBytes(), so a legitimate value such as "2g" silently narrowed to a negative int and the index writer crashed with an unrelated failure deep inside the first chunk. The chunk size also becomes an eagerly allocated per-chunk buffer, so values beyond int range can never work. Validate the option instead: reject anything above Integer.MAX_VALUE with a message naming the option and the offending value. Assisted-by: GLM-5.3
LuciferYang
marked this pull request as draft
September 13, 2026 03:06
"2g" narrows to a negative int and crashes on the first chunk allocation, but "4g" narrows to 0 and "5g" to 1g: those build a silently wrong index without failing anywhere, which is the case the guard is really there for. Add the "4g" case and say so in the comment. Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
The "4g" case re-entered the same comparison as "2g", so it pinned nothing; mutating the guard from > to >= went undetected. Assert the boundary itself instead. The comment also claimed more than the code does. Chunk size 0 is a supported mode (ChunkedDictionaryTest builds one deliberately) and 1g is a value the guard accepts, so neither "4g" nor "5g" produces a wrong index: narrowing substitutes a different size than the one configured, and only the negative case fails. Co-Authored-By: Claude Code <noreply@anthropic.com>
…hunk-size-truncation
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
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.
Purpose
close #9764
(int) MemorySize.parse(chunkSize).getBytes()narrowed thechunk-sizeoption to an int, and what the narrowing produces depends on the value:2gByteBuffer.allocate4g,8g5gThe crashing case is the one you notice. The other two are the reason to validate, because they produce a working-looking index from a configuration that was never honoured. The chunk size becomes an eagerly allocated per-chunk buffer, so nothing above int range can work in the first place.
This PR rejects anything above
Integer.MAX_VALUEat writer creation with a message naming the option and the offending value.chunk-sizeis read as a free-form string with no typedConfigOption, andMemorySize.parseBytesrejects a negative or overflowing value itself, so the accepted set after this guard is exactly[0, Integer.MAX_VALUE], which is exactly the set that narrows to itself.Tests
RangeBitmapFileIndexTest#testChunkSizeBeyondIntRangeRejectedpins the boundary rather than a sample either side of it:2147483648 bytesis rejected with a message containingchunk-size,2147483647 bytesis accepted, and16mbstill writes and serializes.2gand4gwere dropped from it because they reach the guard through the same comparison as2147483648 bytes, and their narrowing consequences cannot be asserted once the guard prevents them.Verified on JDK 11 by mutating the comparison rather than reasoning about it. 18 tests pass as they stand. With the guard removed the test fails with
Expecting code to raise a throwable, which also shows that on mastercreateWriter()returns normally and the damage happens later during the write. With>changed to>=it fails on the accepted case withThe 'chunk-size' option must not exceed 2147483647 bytes, but was '2147483647 bytes'.API and Format
A new validation on an existing user option. Values that previously worked are unaffected; values above 2GB either crashed mid-write or silently built an index that ignored the setting.
Documentation
None needed. The behaviour on out-of-range values was a crash or silent misconfiguration, not a documented feature.