The GitHub Actions job "Validate and test" on pekko-http.git/main has failed.
Run started by GitHub user pjfanning (triggered by pjfanning).

Head commit for run:
ff9f88e1a8dcb715798d32e955a72f5cf4e9537f / PJ Fanning 
<[email protected]>
feat: bound the number of parts in a multipart entity (#1266)

* feat: bound the number of parts in a multipart entity

Motivation:
`BodyPartParser` had no limit on how many body parts one multipart
entity may contain. Each part costs a set of parsed headers and an
entity of its own, so a body packed with minimal parts - a boundary, a
short Content-Disposition and an empty body, on the order of 40 to 60
bytes each - amplifies the work and allocation a request of a given
size causes. A body at the default `max-content-length` of 8m runs to
well over a hundred thousand parts, all of which the strict and form
field paths materialise. `max-content-length` bounds the bytes but not
that amplification, and peers such as Commons FileUpload and Spring
grew an explicit part limit for the same reason.

Modification:
Add a `max-part-count` parser setting, enforced where the parser
starts the headers of a new part, and fail the entity once it is
exceeded. The three call sites that begin a part now go through
`parsePartHeaderLines`; the recursive calls that continue the headers
of the current part are unaffected, as is the closing boundary, which
does not start a part.

The default of 10000 is deliberately generous. It is chosen to stay
above the largest part count the test suite exercises, an existing
case that parses 5000 parts in one go, rather than to be the tightest
useful bound - it still cuts the worst case by more than an order of
magnitude, and an application that knows its forms are small can set
it far lower. A tighter default would be defensible if the 5000 part
case is not a capability worth keeping.

Result:
A multipart body can no longer be packed with an unbounded number of
parts; the count is bounded by configuration.

Tests:
- sbt "http-tests/testOnly 
org.apache.pekko.http.scaladsl.unmarshalling.*MultipartUnmarshallersSpec* 
org.apache.pekko.http.scaladsl.marshalling.MarshallingSpec 
org.apache.pekko.http.scaladsl.server.directives.FileUploadDirectivesSpec" - 
pass (95 tests), including the existing "many small parts received in one go" 
case. A new test configures a limit of 2, sends 3 parts and expects the entity 
to fail; verified it fails with the limit check disabled, in both the CRLF and 
LF variants.
- sbt http-core/mimaReportBinaryIssues, sbt http/mimaReportBinaryIssues - pass, 
with an exclude for the new method on the internal BodyPartParser.Settings
- sbt "+http-core/compile" - pass on 2.13.18 and 3.3.8

References:
None - bounds the number of parts in a multipart entity

* Add the Scala 3 MiMa excludes for the new ParserSettings members

The excludes for the two ParserSettings members are only needed on
Scala 3; the 2.13 check filters them under a broader rule, so a scoped
run on the default Scala version alone did not surface them.

* fix: count empty parts towards max-part-count

A run of consecutive boundaries, with no headers or body between them,
starts each of its parts in the `BoundaryHeader` branch of
`parseHeaderLines`, which recursed into `parseHeaderLines` directly and so
never went through `parsePartHeaderLines`. `partCount` was incremented once
for the whole run and the configured limit went unenforced -- for the very
shape that amplifies hardest, since an empty part costs only a boundary and
an end-of-line.

Count the part where that branch begins a new one instead. The counting is
split out of `parsePartHeaderLines` into `startPart`/`failMaxPartCount` so
the branch can keep its self-recursive call to `parseHeaderLines`: that call
is what makes the method `@tailrec`, and routing it through
`parsePartHeaderLines` would turn it into an unoptimised mutual recursion of
up to `max-part-count` frames -- the stack overflow the trampoline in
`parseEntity` already guards against.

Reported by samueleresca in review of #1266.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

Report URL: https://github.com/apache/pekko-http/actions/runs/34528677461

With regards,
GitHub Actions via GitBox


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to