Skip to content

Add server.request.body.files_content AppSec address for Akka HTTP - #11725

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 5 commits into
masterfrom
filecontent-akka
Jul 14, 2026
Merged

Add server.request.body.files_content AppSec address for Akka HTTP#11725
gh-worker-dd-mergequeue-cf854d[bot] merged 5 commits into
masterfrom
filecontent-akka

Conversation

@jandro996

@jandro996jandro996 commented Jun 24, 2026

Copy link
Copy Markdown
Member

What Does This Do

Extends the Akka HTTP AppSec instrumentation to dispatch the `server.request.body.files_content` WAF address for multipart file uploads.

  • Adds `contentCallback` (`EVENTS.requestFilesContent()`) to both multipart parsing routes in `UnmarshallerHelpers`:
    • `handleMultipartStrictFormData` — used by the `entity(as[Multipart.FormData.Strict])` / strict unmarshaller variant
    • `handleStrictFormData` — used by the `formFieldMultiMap` / `entity(as[StrictForm])` default route
  • Per-part file content is read via `ByteString.getData().take(MAX_CONTENT_BYTES).toArray()` to avoid allocating the full file in memory
  • Content is decoded using `MultipartContentDecoder.decodeBytes()` (introduced in Fix charset decoding for server.request.body.files_content in commons-fileupload #11212), with the correct `ContentType` API per route:
    • Route 1 (Java API): `entity.getContentType().toString()`
    • Route 2 (Scala API): `sentity.contentType().toString()`
  • `MAX_CONTENT_BYTES` and `MAX_FILES_TO_INSPECT` read from `Config.get()` (not hardcoded)
  • Dispatch follows the established sequential ordering: body and filenames always fire; `files_content` fires only if neither prior callback blocked the request
  • Hoists `reqCtx` in `handleStrictFormData` so it is shared by both the filenames and content dispatch blocks
  • Skips body decoding in `handleStrictFormData` when body callback is absent (avoids full-file string allocation for content-only subscribers)
  • Adds `testBodyFilesContent()` overrides in `akka-http-10.0` and `akka-http-10.6` test modules: `true` for async/bind variants, `false` for sync variants that do not process body

Motivation

Part of APPSEC-61875: extending `server.request.body.files_content` WAF address support across Java frameworks. Akka HTTP already has `server.request.body.filenames` support since #11173; this PR adds the content counterpart following the same pattern established in #11198 (Tomcat + Netty) and #11229 (Jersey + RESTEasy).

Additional Notes

`akka-http-10.6` tests require an Akka commercial repository token (`akkaRepositoryToken`) which is only available in CI. Local test runs cover `akka-http-10.0` only.

Contributor Checklist

Jira ticket: APPSEC-61875

Note:Once your PR is ready to merge, add it to the merge queue by commenting `/merge`. `/merge -c` cancels the queue request. `/merge -f --reason "reason"` skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.

…HTTP
- Extend handleMultipartStrictFormData (strictUnmarshaller) and
handleStrictFormData (formFieldMultiMap) in UnmarshallerHelpers to
accumulate file content via MultipartContentDecoder and dispatch
EVENTS.requestFilesContent() callback
- Content dispatch is sequential after filenames (fires only if no
prior block), consistent with other frameworks (Tomcat, Netty, Jersey)
- Uses ByteString.take(MAX_CONTENT_BYTES).toArray() to avoid full
allocation; MAX_* constants read from Config
- Add testBodyFilesContent() overrides to both akka-http-10.0 and
akka-http-10.6 test modules
@jandro996

Copy link
Copy Markdown
MemberAuthor

@codex review

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:ace3f1abca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jandro996jandro996 added type: feature Enhancements and improvements comp: asm waf Application Security Management (WAF) labels Jun 24, 2026
…lback absent
When only files_content is subscribed (not requestBodyProcessed),
the previous code still called getData().decodeString() on every file
body part to populate conv — decoding the full file into a String
regardless of MAX_CONTENT_BYTES. Gate decodeString and
handleArbitraryPostData on bodyCb != null to avoid unnecessary
full-file allocation.
…BindSyncTest
Sync server binding does not support body processing; override returns false
to match the pattern already used for testBodyMultipart, testBodyFilenames, etc.
…BindSyncTest (10.6)
Same fix as 10.0: sync binding does not process body, override returns false
to match testBodyMultipart, testBodyFilenames, etc.
@jandro996
jandro996 marked this pull request as ready for review June 24, 2026 13:13
@jandro996
jandro996 requested review from a team as code ownersJune 24, 2026 13:13
@jandro996
jandro996 requested review from ValentinZakharov, claponcet and manuel-alvarez-alvarez and removed request for a teamJune 24, 2026 13:13
@jandro996
jandro996 added this pull request to the merge queueJul 14, 2026
@dd-octo-sts

Copy link
Copy Markdown
Contributor

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351Bot commented Jul 14, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-07-14 07:58:17 UTC ℹ️ Start processing command /merge


2026-07-14 07:58:23 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 2h (p90).


2026-07-14 08:55:39 UTC ℹ️ MergeQueue: This merge request was merged

@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Jul 14, 2026
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot merged commit 871f5b6 into masterJul 14, 2026
589 of 592 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot deleted the filecontent-akka branch July 14, 2026 08:55
@github-actionsgithub-actionsBot added this to the 1.65.0 milestone Jul 14, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: asm wafApplication Security Management (WAF)type: featureEnhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jandro996@claponcet