pjfanning opened a new pull request, #3525:
URL: https://github.com/apache/pekko/pull/3525
### Motivation
Backport of #3492 to 1.7.x: `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
(up to
~2 GiB) or negative frame length and drive a large allocation. The decoder
and
deserializer stages downstream of the inbound `MergeHub` are shared by every
inbound
connection, so a fatal `OutOfMemoryError` there fails the shared inbound
stream and,
after `inbound-max-restarts`, terminates the whole `ActorSystem` — one
malformed
connection escalating to node loss.
### Modification
Two commits:
1. Cherry-pick of fcc340ce9a (#3492) — applied clean, no conflicts. The
already-configured `maximum-frame-size` / `maximum-large-frame-size` are
threaded
into `TcpFraming`; `ReadStreamId` selects the applicable bound per stream
(the
large-message stream keeps its larger limit) and `ReadFrame` rejects a
negative or
over-bound frame length before any data is buffered, as a per-connection
`FramingException`.
2. The two per-stream bound tests just added on main in #3524, so 1.7.x gets
the same
coverage: with distinct maxima configured, a frame over the ordinary
maximum but
within the large maximum is accepted on `ArteryTransport.LargeStreamId`
and rejected
on `ArteryTransport.OrdinaryStreamId`, with full payloads so a false
accept cannot
hide behind truncation.
New `TcpFraming` constructor params default to `Int.MaxValue`, so the change
is source-
and binary-compatible; `TcpFraming` is `@InternalApi` regardless.
### 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
`ActorSystem`
termination. Legitimate frames up to the configured maxima are unaffected.
### Tests
- `sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.TcpFramingSpec"`
— 17 passed
(the #3492 oversized/negative/at-the-limit cases plus the two #3524
per-stream cases)
- `sbt "++ 2.12.21 remote/Test/compile"` — clean, validating Scala 2.12
- `sbt "remote/scalafmtCheckAll"` — clean on the touched file (the
pre-existing
formatting drift in `NestedPayloadDepthSpec` on 1.7.x is deliberately left
untouched)
### References
Backport of #3492; includes the tests from #3524.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
