fix: bound inbound Artery TCP frame length at framing (#3492) - #3525
Merged
Conversation
Motivation: TcpFraming read the 4-byte frame length from the wire and passed it straight to reader.take / ByteBuffer allocation with no bound. A peer - unauthenticated on the default tcp transport - could declare an oversized or negative frame and drive a large allocation. Because the decoder and deserializer stages downstream of the MergeHub are shared by every inbound connection, an OutOfMemoryError there is fatal to the shared stream and, after inbound-max-restarts, terminates the whole ActorSystem - turning one malformed connection into node loss. Modification: Pass the configured maximum-frame-size and maximum-large-frame-size into TcpFraming and reject a frame length that is negative or exceeds the maximum for the connection's stream, before any data is buffered. The large-message stream keeps its larger bound. Rejection is a FramingException, which tears down only that connection. Result: A malformed or oversized frame is rejected per-connection instead of allocating without limit and risking a fatal error on the shared inbound stream. Tests: - sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.TcpFramingSpec" - 14 passed, incl. oversized, negative and at-limit frame cases - sbt "remote/testOnly org.apache.pekko.remote.artery.LargeMessagesStreamSpec" - 4 passed (legitimate large frames still delivered) - sbt "remote/mimaReportBinaryIssues" - no issues (TcpFraming is @internalapi; new params are defaulted) References: None - found while reviewing the draft threat model in apache#3478
Motivation: Same test-coverage gap as on main: the bound tests from apache#3492 configure only maximumFrameSize, so they cannot tell the per-stream bound selection in ReadStreamId apart from a bug that applied one bound to every stream. Modification: Add the two tests from apache#3524: with distinct maximumFrameSize and maximumLargeFrameSize configured, a frame over the ordinary maximum but within the large maximum is accepted on ArteryTransport.LargeStreamId and rejected on ArteryTransport.OrdinaryStreamId. Both frames carry their full declared payload, so a false accept cannot hide behind truncation. Result: The per-stream bound selection is covered directly on 1.7.x as well. Tests: - sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.TcpFramingSpec" - 17 passed - sbt "++ 2.12.21 remote/Test/compile" - clean - sbt "remote/scalafmtCheckAll" - clean References: Refs apache#3524
Philippus
approved these changes
Sep 3, 2026
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
2 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.
Motivation
Backport of #3492 to 1.7.x:
TcpFramingread the 4-byte frame length from the wire andpassed it straight to
reader.take/ByteBufferallocation with no bound. A peer —unauthenticated on the default
tcptransport — could declare an oversized (up to~2 GiB) or negative frame length and drive a large allocation. The decoder and
deserializer stages downstream of the inbound
MergeHubare shared by every inboundconnection, so a fatal
OutOfMemoryErrorthere fails the shared inbound stream and,after
inbound-max-restarts, terminates the wholeActorSystem— one malformedconnection escalating to node loss.
Modification
Two commits:
already-configured
maximum-frame-size/maximum-large-frame-sizeare threadedinto
TcpFraming;ReadStreamIdselects the applicable bound per stream (thelarge-message stream keeps its larger limit) and
ReadFramerejects a negative orover-bound frame length before any data is buffered, as a per-connection
FramingException.coverage: with distinct maxima configured, a frame over the ordinary maximum but
within the large maximum is accepted on
ArteryTransport.LargeStreamIdand rejectedon
ArteryTransport.OrdinaryStreamId, with full payloads so a false accept cannothide behind truncation.
New
TcpFramingconstructor params default toInt.MaxValue, so the change is source-and binary-compatible;
TcpFramingis@InternalApiregardless.Result
Same as #3492: a malformed or oversized frame is rejected per-connection instead of
allocating without limit and risking a fatal error that escalates to
ActorSystemtermination. Legitimate frames up to the configured maxima are unaffected.
Tests
sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.TcpFramingSpec"— 17 passed(the fix: bound inbound Artery TCP frame length at framing #3492 oversized/negative/at-the-limit cases plus the two test: cover the per-stream frame-size bound in TcpFraming #3524 per-stream cases)
sbt "++ 2.12.21 remote/Test/compile"— clean, validating Scala 2.12sbt "remote/scalafmtCheckAll"— clean on the touched file (the pre-existingformatting drift in
NestedPayloadDepthSpecon 1.7.x is deliberately left untouched)References
Backport of #3492; includes the tests from #3524.