Skip to content

feat: bound the number of parts in a multipart entity - #1266

Open
pjfanning wants to merge 2 commits into
apache:mainfrom
pjfanning:bound-multipart-part-count
Open

feat: bound the number of parts in a multipart entity#1266
pjfanning wants to merge 2 commits into
apache:mainfrom
pjfanning:bound-multipart-part-count

Conversation

@pjfanning

Copy link
Copy Markdown
Member

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–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 (a VectorBuilder, a field map, a HttpEntity.Strict per part).

max-content-length bounds the bytes but not that amplification. Peer frameworks grew an explicit part limit for the same reason — Commons FileUpload's FileCountLimit (FILEUPLOAD-279), Spring's maxParts, Jetty's maxFormKeys.

Modification

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

On the default

10000 is deliberately generous, and I'd welcome a steer on it. It is chosen to stay above the largest part count the test suite exercises — an existing case, "many small parts received in one go", that parses 5000 parts — 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.

I did not want to silently drop a documented capability for a low-severity amplification issue, which is why I sized the default around that test rather than adjusting the test. If you'd rather have a tighter default (1000 would match Jetty's form-key limit and is ample for real forms), that test can raise its own limit via config instead — say the word and I'll flip it.

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 ...MultipartUnmarshallersSpec* ...MarshallingSpec ...FileUploadDirectivesSpec" — pass (95 tests), including the existing 5000-part 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 (following the existing max-chunk-count.excludes precedent).
  • sbt "+http-core/compile" — pass on 2.13.18 and 3.3.8.
  • Native scalafmt clean.

References

None - bounds the number of parts in a multipart entity

🤖 Generated with Claude Code

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
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.
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.

1 participant