Add server.request.body.filenames support for Undertow and Play#11174
Draft
Add server.request.body.filenames support for Undertow and Play#11174
Conversation
- Undertow: extract filenames from FormData attachments in MultiPartUploadHandlerInstrumentation - Play 2.5/2.6: extract filenames from MultipartFormData.files() in BodyParserHelpers Both implementations fire the requestFilesFilenames() IG event and support blocking on malicious filenames. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In undertow 2.2+, FormValueImpl.isFile() returns false for in-memory file uploads (file size below fileSizeThreshold) because it checks fileItem.isInMemory(). Use getFileName() to identify file uploads regardless of storage, which works across all undertow versions. Also check the filenames callback before building the list to avoid allocations on requests where the feature is inactive.
jandro996
added
type: enhancement
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f340ebfab9
ℹ️ 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".
…Undertow Both callbacks are now fetched upfront; the method only returns early when both are null. Previously an early return on requestBodyProcessed==null silently skipped filename detection, breaking deployments with filename-only WAF rules.
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 61 metrics, 10 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.059 s) : 0, 1059242
Total [baseline] (11.108 s) : 0, 11108243
Agent [candidate] (1.06 s) : 0, 1059912
Total [candidate] (11.009 s) : 0, 11009129
section appsec
Agent [baseline] (1.257 s) : 0, 1257225
Total [baseline] (10.906 s) : 0, 10906308
Agent [candidate] (1.267 s) : 0, 1267058
Total [candidate] (10.974 s) : 0, 10974324
section iast
Agent [baseline] (1.23 s) : 0, 1230066
Total [baseline] (11.313 s) : 0, 11313011
Agent [candidate] (1.23 s) : 0, 1230231
Total [candidate] (11.281 s) : 0, 11280735
section profiling
Agent [baseline] (1.186 s) : 0, 1186323
Total [baseline] (11.075 s) : 0, 11074586
Agent [candidate] (1.193 s) : 0, 1192602
Total [candidate] (11.001 s) : 0, 11000963
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.23 ms) : 0, 1230
crashtracking [candidate] (1.224 ms) : 0, 1224
BytebuddyAgent [baseline] (632.117 ms) : 0, 632117
BytebuddyAgent [candidate] (632.756 ms) : 0, 632756
AgentMeter [baseline] (29.573 ms) : 0, 29573
AgentMeter [candidate] (29.593 ms) : 0, 29593
GlobalTracer [baseline] (249.088 ms) : 0, 249088
GlobalTracer [candidate] (249.532 ms) : 0, 249532
AppSec [baseline] (32.378 ms) : 0, 32378
AppSec [candidate] (32.459 ms) : 0, 32459
Debugger [baseline] (60.064 ms) : 0, 60064
Debugger [candidate] (59.785 ms) : 0, 59785
Remote Config [baseline] (598.558 µs) : 0, 599
Remote Config [candidate] (604.534 µs) : 0, 605
Telemetry [baseline] (8.802 ms) : 0, 8802
Telemetry [candidate] (8.039 ms) : 0, 8039
Flare Poller [baseline] (9.24 ms) : 0, 9240
Flare Poller [candidate] (9.815 ms) : 0, 9815
section appsec
crashtracking [baseline] (1.211 ms) : 0, 1211
crashtracking [candidate] (1.226 ms) : 0, 1226
BytebuddyAgent [baseline] (671.909 ms) : 0, 671909
BytebuddyAgent [candidate] (678.418 ms) : 0, 678418
AgentMeter [baseline] (12.151 ms) : 0, 12151
AgentMeter [candidate] (12.204 ms) : 0, 12204
GlobalTracer [baseline] (247.926 ms) : 0, 247926
GlobalTracer [candidate] (249.694 ms) : 0, 249694
IAST [baseline] (24.225 ms) : 0, 24225
IAST [candidate] (24.217 ms) : 0, 24217
AppSec [baseline] (185.916 ms) : 0, 185916
AppSec [candidate] (186.059 ms) : 0, 186059
Debugger [baseline] (65.782 ms) : 0, 65782
Debugger [candidate] (66.789 ms) : 0, 66789
Remote Config [baseline] (567.335 µs) : 0, 567
Remote Config [candidate] (566.85 µs) : 0, 567
Telemetry [baseline] (7.881 ms) : 0, 7881
Telemetry [candidate] (7.899 ms) : 0, 7899
Flare Poller [baseline] (3.473 ms) : 0, 3473
Flare Poller [candidate] (3.463 ms) : 0, 3463
section iast
crashtracking [baseline] (1.225 ms) : 0, 1225
crashtracking [candidate] (1.217 ms) : 0, 1217
BytebuddyAgent [baseline] (806.987 ms) : 0, 806987
BytebuddyAgent [candidate] (808.394 ms) : 0, 808394
AgentMeter [baseline] (11.396 ms) : 0, 11396
AgentMeter [candidate] (11.397 ms) : 0, 11397
GlobalTracer [baseline] (238.344 ms) : 0, 238344
GlobalTracer [candidate] (238.983 ms) : 0, 238983
IAST [baseline] (28.235 ms) : 0, 28235
IAST [candidate] (30.795 ms) : 0, 30795
AppSec [baseline] (28.89 ms) : 0, 28890
AppSec [candidate] (27.379 ms) : 0, 27379
Debugger [baseline] (67.087 ms) : 0, 67087
Debugger [candidate] (64.453 ms) : 0, 64453
Remote Config [baseline] (540.863 µs) : 0, 541
Remote Config [candidate] (528.483 µs) : 0, 528
Telemetry [baseline] (7.919 ms) : 0, 7919
Telemetry [candidate] (7.769 ms) : 0, 7769
Flare Poller [baseline] (3.439 ms) : 0, 3439
Flare Poller [candidate] (3.385 ms) : 0, 3385
section profiling
crashtracking [baseline] (1.188 ms) : 0, 1188
crashtracking [candidate] (1.194 ms) : 0, 1194
BytebuddyAgent [baseline] (691.772 ms) : 0, 691772
BytebuddyAgent [candidate] (697.171 ms) : 0, 697171
AgentMeter [baseline] (9.239 ms) : 0, 9239
AgentMeter [candidate] (9.268 ms) : 0, 9268
GlobalTracer [baseline] (207.497 ms) : 0, 207497
GlobalTracer [candidate] (208.306 ms) : 0, 208306
AppSec [baseline] (32.877 ms) : 0, 32877
AppSec [candidate] (33.158 ms) : 0, 33158
Debugger [baseline] (66.191 ms) : 0, 66191
Debugger [candidate] (66.052 ms) : 0, 66052
Remote Config [baseline] (580.743 µs) : 0, 581
Remote Config [candidate] (619.362 µs) : 0, 619
Telemetry [baseline] (7.902 ms) : 0, 7902
Telemetry [candidate] (7.984 ms) : 0, 7984
Flare Poller [baseline] (3.586 ms) : 0, 3586
Flare Poller [candidate] (3.636 ms) : 0, 3636
ProfilingAgent [baseline] (94.152 ms) : 0, 94152
ProfilingAgent [candidate] (93.758 ms) : 0, 93758
Profiling [baseline] (94.716 ms) : 0, 94716
Profiling [candidate] (94.313 ms) : 0, 94313
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.059 s) : 0, 1058687
Total [baseline] (8.837 s) : 0, 8837039
Agent [candidate] (1.06 s) : 0, 1060007
Total [candidate] (8.819 s) : 0, 8819478
section iast
Agent [baseline] (1.228 s) : 0, 1227776
Total [baseline] (9.528 s) : 0, 9528220
Agent [candidate] (1.227 s) : 0, 1227496
Total [candidate] (9.577 s) : 0, 9576770
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.229 ms) : 0, 1229
crashtracking [candidate] (1.227 ms) : 0, 1227
BytebuddyAgent [baseline] (633.98 ms) : 0, 633980
BytebuddyAgent [candidate] (636.473 ms) : 0, 636473
AgentMeter [baseline] (29.521 ms) : 0, 29521
AgentMeter [candidate] (29.731 ms) : 0, 29731
GlobalTracer [baseline] (249.341 ms) : 0, 249341
GlobalTracer [candidate] (250.24 ms) : 0, 250240
AppSec [baseline] (32.439 ms) : 0, 32439
AppSec [candidate] (32.581 ms) : 0, 32581
Debugger [baseline] (59.158 ms) : 0, 59158
Debugger [candidate] (59.171 ms) : 0, 59171
Remote Config [baseline] (589.017 µs) : 0, 589
Remote Config [candidate] (589.228 µs) : 0, 589
Telemetry [baseline] (8.013 ms) : 0, 8013
Telemetry [candidate] (8.0 ms) : 0, 8000
Flare Poller [baseline] (8.258 ms) : 0, 8258
Flare Poller [candidate] (5.786 ms) : 0, 5786
section iast
crashtracking [baseline] (1.241 ms) : 0, 1241
crashtracking [candidate] (1.227 ms) : 0, 1227
BytebuddyAgent [baseline] (806.476 ms) : 0, 806476
BytebuddyAgent [candidate] (808.145 ms) : 0, 808145
AgentMeter [baseline] (11.351 ms) : 0, 11351
AgentMeter [candidate] (11.404 ms) : 0, 11404
GlobalTracer [baseline] (238.579 ms) : 0, 238579
GlobalTracer [candidate] (237.87 ms) : 0, 237870
IAST [baseline] (29.229 ms) : 0, 29229
IAST [candidate] (31.712 ms) : 0, 31712
AppSec [baseline] (28.427 ms) : 0, 28427
AppSec [candidate] (27.086 ms) : 0, 27086
Debugger [baseline] (64.684 ms) : 0, 64684
Debugger [candidate] (62.541 ms) : 0, 62541
Remote Config [baseline] (531.742 µs) : 0, 532
Remote Config [candidate] (531.056 µs) : 0, 531
Telemetry [baseline] (7.76 ms) : 0, 7760
Telemetry [candidate] (7.63 ms) : 0, 7630
Flare Poller [baseline] (3.45 ms) : 0, 3450
Flare Poller [candidate] (3.449 ms) : 0, 3449
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 4 performance regressions! Performance is the same for 14 metrics, 15 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (1.287 ms) : 1275, 1300
. : milestone, 1287,
iast (3.292 ms) : 3245, 3339
. : milestone, 3292,
iast_FULL (5.927 ms) : 5867, 5986
. : milestone, 5927,
iast_GLOBAL (3.617 ms) : 3558, 3677
. : milestone, 3617,
profiling (2.248 ms) : 2227, 2270
. : milestone, 2248,
tracing (1.994 ms) : 1976, 2012
. : milestone, 1994,
section candidate
no_agent (1.237 ms) : 1225, 1249
. : milestone, 1237,
iast (3.476 ms) : 3422, 3530
. : milestone, 3476,
iast_FULL (6.197 ms) : 6135, 6260
. : milestone, 6197,
iast_GLOBAL (3.833 ms) : 3767, 3899
. : milestone, 3833,
profiling (2.481 ms) : 2455, 2508
. : milestone, 2481,
tracing (1.9 ms) : 1883, 1916
. : milestone, 1900,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (18.142 ms) : 17959, 18325
. : milestone, 18142,
appsec (19.858 ms) : 19652, 20064
. : milestone, 19858,
code_origins (17.958 ms) : 17780, 18136
. : milestone, 17958,
iast (18.132 ms) : 17952, 18312
. : milestone, 18132,
profiling (19.314 ms) : 19121, 19507
. : milestone, 19314,
tracing (17.811 ms) : 17636, 17986
. : milestone, 17811,
section candidate
no_agent (18.185 ms) : 18003, 18367
. : milestone, 18185,
appsec (18.7 ms) : 18512, 18888
. : milestone, 18700,
code_origins (18.289 ms) : 18109, 18470
. : milestone, 18289,
iast (18.029 ms) : 17852, 18205
. : milestone, 18029,
profiling (18.463 ms) : 18280, 18646
. : milestone, 18463,
tracing (17.915 ms) : 17734, 18096
. : milestone, 17915,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (1.495 ms) : 1483, 1507
. : milestone, 1495,
appsec (2.546 ms) : 2491, 2601
. : milestone, 2546,
iast (2.278 ms) : 2209, 2348
. : milestone, 2278,
iast_GLOBAL (2.325 ms) : 2255, 2395
. : milestone, 2325,
profiling (2.114 ms) : 2059, 2170
. : milestone, 2114,
tracing (2.104 ms) : 2050, 2158
. : milestone, 2104,
section candidate
no_agent (1.492 ms) : 1480, 1504
. : milestone, 1492,
appsec (3.86 ms) : 3637, 4084
. : milestone, 3860,
iast (2.294 ms) : 2225, 2364
. : milestone, 2294,
iast_GLOBAL (2.334 ms) : 2264, 2405
. : milestone, 2334,
profiling (2.113 ms) : 2058, 2168
. : milestone, 2113,
tracing (2.089 ms) : 2035, 2143
. : milestone, 2089,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~d33f38425a, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (15.707 s) : 15707000, 15707000
. : milestone, 15707000,
appsec (14.376 s) : 14376000, 14376000
. : milestone, 14376000,
iast (19.214 s) : 19214000, 19214000
. : milestone, 19214000,
iast_GLOBAL (18.069 s) : 18069000, 18069000
. : milestone, 18069000,
profiling (15.073 s) : 15073000, 15073000
. : milestone, 15073000,
tracing (15.029 s) : 15029000, 15029000
. : milestone, 15029000,
section candidate
no_agent (14.878 s) : 14878000, 14878000
. : milestone, 14878000,
appsec (14.953 s) : 14953000, 14953000
. : milestone, 14953000,
iast (18.314 s) : 18314000, 18314000
. : milestone, 18314000,
iast_GLOBAL (17.982 s) : 17982000, 17982000
. : milestone, 17982000,
profiling (14.567 s) : 14567000, 14567000
. : milestone, 14567000,
tracing (14.945 s) : 14945000, 14945000
. : milestone, 14945000,
|
…port Use reflection to invoke MultipartFormData.files() so the bytecode does not embed a hard reference to the Scala 2.11/2.12 return type (Lscala/collection/Seq;). In Scala 2.13 (Play 2.7+) the method returns scala.collection.immutable.Seq, causing muzzle to disable the entire PlayBodyParsersInstrumentation and breaking all body-parsing features. Also enable testBodyFilenames() in Play 2.5/2.6/2.7 test suites.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Instruments Undertow and Play multipart request handling to fire the
requestFilesFilenamesAppSec gateway event, enabling WAF rules that act on uploaded file names.Undertow (
undertow-2.0instrumentation, applied to 2.0–2.2+):The
MultiPartParserDefinition$MultiPartUploadHandler.parseBlocking()exit advice already firesrequestBodyProcessedviaFormDataMap. This PR extends it to also firerequestFilesFilenamesby iterating the parsedFormDataand collecting values wheregetFileName()is non-null. The callback check is done before building the filenames list to avoid allocations on requests where the feature is inactive.A secondary fix was required in
FormDataMap: in undertow 2.0,FormValueImpl.isFile()returnstruefor all file uploads (they always go to disk). In undertow 2.2+, theFileItemabstraction was introduced to support in-memory storage below a threshold;isFile()now returnsfalsefor in-memory files even thoughvalue == null. The old!isFile()guard causedgetValue()to throwIllegalStateExceptionon every multipart request with small file attachments. Switching togetFileName() == nullcorrectly identifies text fields regardless of undertow version.Play 2.5 / 2.6 (
play-appsec-2.5,play-appsec-2.6):BodyParserHelpers.handleMultipartFormData()is the central point where Play assembles the body result. Both versions already call ahandleMultipartBodyMap()helper; this PR adds a symmetrichandleMultipartFilenames()that iteratesdata.files(), extractsFilePart.filename(), and fires the callback throughexecuteFilenamesCallback()with blocking support. Play 2.6 uses the same approach compiled against the 2.6 API.undertow-2.0MultiPartUploadHandlerInstrumentation+FormDataMapfixundertow-2.2play-appsec-2.5BodyParserHelpers.handleMultipartFilenames()play-appsec-2.6BodyParserHelpers.handleMultipartFilenames()Motivation
Part of APPSEC-61873 —
server.request.body.filenamesimplementation across server frameworks.Additional Notes
Depends on #10949 and #10973 (both merged into master).
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue