Skip to content

Add server.request.body.files_content AppSec address for Undertow and Play - #11726

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 5 commits into
masterfrom
alejandro.gonzalez/APPSEC-61875-files-content-undertow-play
Jul 13, 2026
Merged

Add server.request.body.files_content AppSec address for Undertow and Play#11726
gh-worker-dd-mergequeue-cf854d[bot] merged 5 commits into
masterfrom
alejandro.gonzalez/APPSEC-61875-files-content-undertow-play

Conversation

@jandro996

@jandro996jandro996 commented Jun 24, 2026

Copy link
Copy Markdown
Member

What Does This Do

Implements the server.request.body.files_content WAF address for Undertow 2.0/2.2+ and Play 2.5/2.6, enabling detection and blocking based on the content of uploaded files (not just their names).

Undertow 2.0 / 2.2+

  • New helper class FormDataContentHelper reads file content from FormData.FormValue entries:
    • Tries getPath() first (disk uploads, works in 2.0 and 2.2+)
    • Falls back to getFileItem().getInputStream() via cached reflection for in-memory uploads (Undertow 2.2+)
    • Uses Headers.CONTENT_TYPE from the part headers for charset-aware decoding via MultipartContentDecoder
  • MultiPartUploadHandlerInstrumentation now obtains requestFilesContent callback upfront and fires it after the filenames callback, gated on t == null (content is suppressed if filenames already triggered a block)
  • FormDataContentHelper declared in helperClassNames() so constants referencing Config are kept out of the advice class (avoids muzzle failures)
  • 7 unit tests in FormDataContentHelperTest: disk file read, form field skip, empty filename, truncation at MAX_CONTENT_BYTES, file count limit, mixed fields+files, graceful fallback for in-memory proxy

Play 2.5 / 2.6

  • BodyParserHelpers extended with handleMultipartFilesContent(), collectFilesContent(), readFilePartContent(), and executeFilesContentCallback()
  • Reads uploaded file content via FilePart.ref()TemporaryFile.file(), both resolved through cached static final Method reflection to avoid bytecode references to TemporaryFile that would break muzzle
  • Content block in handleMultipartFormData() fires only when pendingBlock == null (sequential: content is suppressed if filenames already blocked)
  • @VisibleForTesting added to all package-private helper methods (collectFilenames, collectFilesContent, readFilePartContent)

Test coverage

  • UndertowServletTest (2.0 and 2.2): testBodyFilesContent() enabled
  • PlayServerTest (2.5 and 2.6): testBodyFilesContent() enabled
  • PlayAsyncServerTest (2.6): testBodyFilesContent() enabled

Motivation

Part of APPSEC-61875 — server.request.body.files_content WAF address rollout across Java frameworks. Previous PRs covered commons-fileupload (#11137/#11212), Tomcat+Netty (#11198), and Jersey+RESTEasy (#11229). This PR adds Undertow and Play.

Additional Notes

The Undertow 2.0/2.2+ split requires special handling: in 2.0, all uploads are written to disk (getPath() always succeeds); in 2.2+, small uploads are kept in memory and getPath() throws. The reflection fallback targets getFileItem().getInputStream() which was added in the 2.2+ API. Since FormDataContentHelper is compiled against the 2.0 API, the method cannot be referenced at compile time.

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.

…Play
Implements the files_content WAF address for Undertow 2.0/2.2+ and Play 2.5/2.6.
- Undertow: new FormDataContentHelper reads file content via getPath() with
reflection fallback for 2.2+ in-memory uploads (getFileItem().getInputStream())
- Play 2.5/2.6: extends BodyParserHelpers with handleMultipartFilesContent(),
gated on pendingBlock == null so filenames blocking suppresses content inspection
- Content callback fires sequentially after filenames, never if already blocked
- Charset-aware decoding via MultipartContentDecoder throughout
- MAX_CONTENT_BYTES / MAX_FILES_TO_INSPECT kept in helper classes (not advice)
to avoid muzzle failures
@jandro996

Copy link
Copy Markdown
MemberAuthor

@codex review

@jandro996jandro996 added type: feature Enhancements and improvements comp: asm waf Application Security Management (WAF) labels Jun 24, 2026

@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:0e48784766

ℹ️ 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".

…ervers
The default limit of 1024 bytes caused RequestTooBigException for the
truncation test (4096+500 bytes) and max-files test. Unlimited (-1) allows
the content tests to reach the multipart handler.
@jandro996
jandro996 marked this pull request as ready for review June 25, 2026 09:07
@jandro996
jandro996 requested review from a team as code ownersJune 25, 2026 09:07
@jandro996
jandro996 requested review from claponcet, dromanol, jordan-wong and manuel-alvarez-alvarez and removed request for a teamJune 25, 2026 09:07
@dd-octo-sts

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

SuiteStatus
Startup🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
ScenarioCandidatemasterΔ (95% CI of mean)
startup:insecure-bank:iast:Agent13.94 s13.93 s[-0.6%; +0.8%] (no difference)
startup:insecure-bank:tracing:Agent12.98 s13.05 s[-1.6%; +0.6%] (no difference)
startup:petclinic:appsec:Agent16.81 s16.69 s[-0.3%; +1.7%] (no difference)
startup:petclinic:iast:Agent16.94 s16.57 s[-2.3%; +6.7%] (no difference)
startup:petclinic:profiling:Agent16.70 s16.85 s[-2.1%; +0.4%] (no difference)
startup:petclinic:sca:Agent16.33 s16.80 s[-7.1%; +1.4%] (no difference)
startup:petclinic:tracing:Agent15.59 s15.97 s[-6.6%; +2.0%] (no difference)

Commit:3f872d46 · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@jandro996
jandro996 added this pull request to the merge queueJul 13, 2026
@dd-octo-sts

Copy link
Copy Markdown
Contributor

/merge

@gh-worker-devflow-routing-ef8351

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

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-07-13 10:09:33 UTC ℹ️ Start processing command /merge


2026-07-13 10:09:38 UTC ℹ️ MergeQueue: pull request added to the queue

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


2026-07-13 11:20:05 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 13, 2026
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot merged commit f0685da into masterJul 13, 2026
583 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot deleted the alejandro.gonzalez/APPSEC-61875-files-content-undertow-play branch July 13, 2026 11:20
@github-actionsgithub-actionsBot added this to the 1.65.0 milestone Jul 13, 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