Skip to content

[Fix] Validate Pipe payload lengths before allocation - #18815

Open
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/pipe-request-length-validation
Open

Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/pipe-request-length-validation

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Description

Validate length-prefixed Pipe strings and binary payloads against the remaining request body before decoding V1/V2 handshakes, slices, and receiver-runtime cleanup requests. Also reject V2 handshake parameter counts that cannot fit in the body. Malformed payloads now return the existing PIPE_ERROR response, including through compressed wrappers.

Valid wire encoding, null/empty strings, and handshake buffer positions remain compatible.

Validation

  • All 77 tests passed across PipeTransferRequestValidationTest, PipeTransferSliceReqBuilderTest, PipeTransferCompressedReqTest, PipeDataNodeThriftRequestTest, and PipeReceiverTest.
  • Full English reactor build and targeted tests:
    mvn -o test -Dtest=PipeTransferRequestValidationTest,PipeTransferSliceReqBuilderTest,PipeTransferCompressedReqTest,PipeDataNodeThriftRequestTest,PipeReceiverTest -Dsurefire.failIfNoSpecifiedTests=false -Dmaven.build.cache.enabled=false
    
  • Full Chinese reactor compilation:
    mvn -o test-compile -P with-zh-locale -DskipTests -Dmaven.build.cache.enabled=false
    
  • Spotless and Checkstyle passed. Both reactor builds used MAVEN_OPTS=-Xmx2g.

This PR has:

  • been self-reviewed.
  • added comments explaining the allocation checks.
  • added parser and receiver regression tests.
Key changed/added classes

PipeTransferPayloadReader, PipeTransferHandshakeV1Req, PipeTransferHandshakeV2Req, PipeTransferSliceReq, and PipeTransferPipeReceiverRuntimeInfoCleanupReq.

@jt2594838 jt2594838 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May push the implementation directly into ReadWriteIOUtils

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants