HADOOP-18896. NegativeArraySizeException thrown in FSOutputSummer.jav… - #8622
Open
anshuksi282-ksolves wants to merge 2 commits into
Open
HADOOP-18896. NegativeArraySizeException thrown in FSOutputSummer.jav…#8622anshuksi282-ksolves wants to merge 2 commits into
anshuksi282-ksolves wants to merge 2 commits into
Conversation
…a given large file.bytes-per-checksum
Contributor
Author
|
Hi all, I'm new to contributing to Hadoop, so please let me know if any changes are needed. Thanks in advance! cc: @ayushtkn @dineshchitlangia @brahmareddybattula @pjfanning @slfan1989 |
pjfanning
reviewed
Jul 22, 2026
| protected FSOutputSummer(DataChecksum sum) { | ||
| this.sum = sum; | ||
| Preconditions.checkArgument( | ||
| sum.getBytesPerChecksum() * BUFFER_NUM_CHUNKS > 0, |
Member
There was a problem hiding this comment.
I would do something like:
int bufArraySize;
try {
bufArraySize = Math.multiplyExact(sum.getBytesPerChecksum(), BUFFER_NUM_CHUNKS);
} catch (ArithmeticException ae) {
throw new IllegalArgumentException("The calculated buffer array size for FSOutputSummer is too large", ae);
}
if (bufArraySize < 0) {
throw new IllegalArgumentException("The calculated buffer array size for FSOutputSummer is negative");
}
this.buf = new byte[bufArraySize];
I would suggest that you also check the array size in a similar way for this.checksum.
Contributor
Author
There was a problem hiding this comment.
Thanks for the review @pjfanning!
Updated to use Math.multiplyExact for both buf and checksum buffer size checks, as suggested.
I did try adding a dedicated test for the checksum buffer overflow path, but getChecksumSize() returns a fixed value from DataChecksum.Type (e.g. 4 for CRC32), so it can never actually be large enough to overflow — that path isn't reachable in practice, just defensive. So I've kept the existing unit test covering the buf overflow case, and re-ran TestFSOutputSummer, TestChecksumFileSystem, and TestChecksumFs to confirm no regressions.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
Hi @dineshchitlangia @pjfanning, thanks for the reviews and the approval! Since the CI is completely green and the requested changes have been addressed, could you please help get this merged? Let me know if there is anything else required from my end. |
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
4 participants
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.
Description of PR
This PR fixes HADOOP-18896.
The buffer size in
FSOutputSummeris computed assum.getBytesPerChecksum() * BUFFER_NUM_CHUNKS. Whenfile.bytes-per-checksumis configured to a very large value (e.g. 238609295), this multiplication overflows a signed 32-bit int and wraps around to a negative number, causing aNegativeArraySizeExceptionwhen allocating the internal buffer array.This PR adds a
Preconditions.checkArgumentcheck in theFSOutputSummerconstructor to validate that the computed buffer size is positive, failing fast with a clearIllegalArgumentExceptioninstead of an obscureNegativeArraySizeException.This continues the work started in #6064 by @teamconfx, which went stale. This PR additionally adds a unit test, as the original PR's
test4testscheck had failed for not including one.How was this patch tested?
TestFSOutputSummerinhadoop-commonwithtestLargeBytesPerChecksumOverflow, using a minimal concrete subclass ofFSOutputSummerto verify the constructor fails withIllegalArgumentException(rather thanNegativeArraySizeException) whenbytesPerChecksum * BUFFER_NUM_CHUNKSoverflows.NegativeArraySizeExceptionwhen the fix is reverted, and passes with the fix applied.mvn -pl hadoop-common-project/hadoop-common test -Dtest=TestFSOutputSummerpasses.Note: the JIRA's original reproduction steps reference an HDFS test (
TestDecommissionWithStriped), but since the buffer overflow bug lives entirely inhadoop-common'sFSOutputSummerclass, this PR adds a focused unit test inhadoop-commoninstead, avoiding an unnecessary cross-module HDFS test dependency.AI Tooling
Contains content generated by Claude (Anthropic). Used to help investigate the root cause, draft the fix, and write/verify the accompanying unit test.