[MINOR][CORE][3.5] Validate array counts against received bytes in shuffle fetch decoders - #58617
Open
holdenk wants to merge 3 commits into
Open
[MINOR][CORE][3.5] Validate array counts against received bytes in shuffle fetch decoders#58617holdenk wants to merge 3 commits into
holdenk wants to merge 3 commits into
Conversation
Look the stream up before the client-id check so an unknown stream id is rejected either way, and enforce that a stream is only read from the connection that registered it. The mocked reverse clients in the request handler suites are bound to their channel to match TransportContext. branch-3.5 uses Guava Preconditions and JUnit 4. Co-Authored-By: Holden Karau <holden@pigscanfly.ca> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Cursor <cursoragent@cursor.com> Co-Authored-By: Holden Karau <holden@pigscanfly.ca>
… fetch decoders FetchShuffleBlocks and FetchShuffleBlockChunks decode an outer element count and eagerly allocate a 2D array of that size. Check the count against the bytes actually received before allocating: each inner group carries its own 4-byte length prefix, so a valid count never exceeds readableBytes() / 4. Uses an explicit comparison since Objects.checkFromIndexSize is unavailable on Java 8. Co-authored-by: Cursor <cursoragent@cursor.com>
sunchao
reviewed
Sep 9, 2026
sunchao
left a comment
Member
There was a problem hiding this comment.
Reviewed commit 346e465. No actionable findings.
The outer-count checks reject negative or impossible counts before allocation, preserve valid encodings (including empty inner groups), and avoid APIs unavailable on Java 8. The full displayed patch was reviewed; the stream and installer changes are already present unchanged on branch-3.5, leaving the four-file decoder/test delta.
Validation:
- Both decoder suites passed locally (4/4 tests).
- Head and base each passed 20 valid fixtures through direct decoding and message dispatch. Another 144 bounded malformed decodes per revision confirmed the new guard rejects before consuming inner groups.
- Local compilation targeted Java 8 using exact protocol sources and declared dependency versions, with existing Spark 3.5.3 jars for unrelated imports; execution used Java 17. This was a partial build, not a clean Spark build or a Java 8 runtime test.
- Exact-head CI reports confirm both decoder suites, relevant stream suites, and the full installer suite passed. Five local installer helper tests and whitespace checks also passed.
CI caveat: workflow 34228034188 has a test-report upload failure (403) and a Kubernetes integration failure whose cause remains unconfirmed. These failures were not established as defects in this patch.
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.
FetchShuffleBlocks and FetchShuffleBlockChunks decode an outer element count and eagerly allocate a 2D array of that size. Check the count against the bytes actually received before allocating: each inner group carries its own 4-byte length prefix, so a valid count never exceeds readableBytes() / 4. Uses an explicit comparison since Objects.checkFromIndexSize is unavailable on Java 8.
Backport of 9687793