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 64 metrics, 7 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.058 s) : 0, 1058379
Total [baseline] (8.804 s) : 0, 8803692
Agent [candidate] (1.075 s) : 0, 1074840
Total [candidate] (8.879 s) : 0, 8878571
section iast
Agent [baseline] (1.229 s) : 0, 1228708
Total [baseline] (9.521 s) : 0, 9520944
Agent [candidate] (1.238 s) : 0, 1238158
Total [candidate] (9.588 s) : 0, 9588387
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.248 ms) : 0, 1248
crashtracking [candidate] (1.245 ms) : 0, 1245
BytebuddyAgent [baseline] (633.547 ms) : 0, 633547
BytebuddyAgent [candidate] (642.163 ms) : 0, 642163
AgentMeter [baseline] (29.758 ms) : 0, 29758
AgentMeter [candidate] (30.254 ms) : 0, 30254
GlobalTracer [baseline] (250.15 ms) : 0, 250150
GlobalTracer [candidate] (253.726 ms) : 0, 253726
AppSec [baseline] (32.769 ms) : 0, 32769
AppSec [candidate] (33.151 ms) : 0, 33151
Debugger [baseline] (59.332 ms) : 0, 59332
Debugger [candidate] (60.514 ms) : 0, 60514
Remote Config [baseline] (595.768 µs) : 0, 596
Remote Config [candidate] (616.522 µs) : 0, 617
Telemetry [baseline] (8.002 ms) : 0, 8002
Telemetry [candidate] (8.279 ms) : 0, 8279
Flare Poller [baseline] (6.596 ms) : 0, 6596
Flare Poller [candidate] (8.442 ms) : 0, 8442
section iast
crashtracking [baseline] (1.239 ms) : 0, 1239
crashtracking [candidate] (1.233 ms) : 0, 1233
BytebuddyAgent [baseline] (808.06 ms) : 0, 808060
BytebuddyAgent [candidate] (813.18 ms) : 0, 813180
AgentMeter [baseline] (11.43 ms) : 0, 11430
AgentMeter [candidate] (11.459 ms) : 0, 11459
GlobalTracer [baseline] (238.402 ms) : 0, 238402
GlobalTracer [candidate] (241.333 ms) : 0, 241333
AppSec [baseline] (27.429 ms) : 0, 27429
AppSec [candidate] (27.518 ms) : 0, 27518
Debugger [baseline] (63.62 ms) : 0, 63620
Debugger [candidate] (63.665 ms) : 0, 63665
Remote Config [baseline] (533.446 µs) : 0, 533
Remote Config [candidate] (535.652 µs) : 0, 536
Telemetry [baseline] (7.76 ms) : 0, 7760
Telemetry [candidate] (7.752 ms) : 0, 7752
Flare Poller [baseline] (3.395 ms) : 0, 3395
Flare Poller [candidate] (3.382 ms) : 0, 3382
IAST [baseline] (30.78 ms) : 0, 30780
IAST [candidate] (30.162 ms) : 0, 30162
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.053 s) : 0, 1052894
Total [baseline] (4.36 s) : 0, 4360233
Agent [candidate] (1.068 s) : 0, 1067523
Total [candidate] (11.175 s) : 0, 11174908
section appsec
Agent [baseline] (1.272 s) : 0, 1271619
Total [baseline] (11.138 s) : 0, 11138219
Agent [candidate] (1.269 s) : 0, 1268524
Total [candidate] (11.116 s) : 0, 11116287
section iast
Agent [baseline] (1.247 s) : 0, 1246954
Total [baseline] (11.403 s) : 0, 11403316
Agent [candidate] (1.25 s) : 0, 1249599
Total [candidate] (11.376 s) : 0, 11375668
section profiling
Agent [baseline] (1.194 s) : 0, 1194147
Total [baseline] (11.089 s) : 0, 11089164
Agent [candidate] (1.193 s) : 0, 1192504
Total [candidate] (11.047 s) : 0, 11047221
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.227 ms) : 0, 1227
crashtracking [candidate] (1.255 ms) : 0, 1255
BytebuddyAgent [baseline] (627.525 ms) : 0, 627525
BytebuddyAgent [candidate] (637.362 ms) : 0, 637362
AgentMeter [baseline] (29.575 ms) : 0, 29575
AgentMeter [candidate] (29.779 ms) : 0, 29779
GlobalTracer [baseline] (247.064 ms) : 0, 247064
GlobalTracer [candidate] (250.919 ms) : 0, 250919
AppSec [baseline] (32.097 ms) : 0, 32097
AppSec [candidate] (32.713 ms) : 0, 32713
Debugger [baseline] (59.29 ms) : 0, 59290
Debugger [candidate] (60.292 ms) : 0, 60292
Remote Config [baseline] (609.553 µs) : 0, 610
Remote Config [candidate] (594.475 µs) : 0, 594
Telemetry [baseline] (8.03 ms) : 0, 8030
Telemetry [candidate] (8.128 ms) : 0, 8128
Flare Poller [baseline] (11.405 ms) : 0, 11405
Flare Poller [candidate] (9.845 ms) : 0, 9845
section appsec
crashtracking [baseline] (1.244 ms) : 0, 1244
crashtracking [candidate] (1.235 ms) : 0, 1235
BytebuddyAgent [baseline] (678.032 ms) : 0, 678032
BytebuddyAgent [candidate] (677.181 ms) : 0, 677181
AgentMeter [baseline] (12.163 ms) : 0, 12163
AgentMeter [candidate] (12.18 ms) : 0, 12180
GlobalTracer [baseline] (251.126 ms) : 0, 251126
GlobalTracer [candidate] (250.174 ms) : 0, 250174
AppSec [baseline] (187.77 ms) : 0, 187770
AppSec [candidate] (187.266 ms) : 0, 187266
Debugger [baseline] (67.762 ms) : 0, 67762
Debugger [candidate] (67.156 ms) : 0, 67156
Remote Config [baseline] (577.481 µs) : 0, 577
Remote Config [candidate] (579.947 µs) : 0, 580
Telemetry [baseline] (8.037 ms) : 0, 8037
Telemetry [candidate] (7.962 ms) : 0, 7962
Flare Poller [baseline] (3.53 ms) : 0, 3530
Flare Poller [candidate] (3.475 ms) : 0, 3475
IAST [baseline] (24.599 ms) : 0, 24599
IAST [candidate] (24.63 ms) : 0, 24630
section iast
crashtracking [baseline] (1.239 ms) : 0, 1239
crashtracking [candidate] (1.255 ms) : 0, 1255
BytebuddyAgent [baseline] (819.903 ms) : 0, 819903
BytebuddyAgent [candidate] (820.585 ms) : 0, 820585
AgentMeter [baseline] (11.613 ms) : 0, 11613
AgentMeter [candidate] (11.624 ms) : 0, 11624
GlobalTracer [baseline] (241.833 ms) : 0, 241833
GlobalTracer [candidate] (242.035 ms) : 0, 242035
AppSec [baseline] (27.454 ms) : 0, 27454
AppSec [candidate] (29.077 ms) : 0, 29077
Debugger [baseline] (65.262 ms) : 0, 65262
Debugger [candidate] (67.51 ms) : 0, 67510
Remote Config [baseline] (540.651 µs) : 0, 541
Remote Config [candidate] (546.024 µs) : 0, 546
Telemetry [baseline] (7.854 ms) : 0, 7854
Telemetry [candidate] (8.053 ms) : 0, 8053
Flare Poller [baseline] (3.525 ms) : 0, 3525
Flare Poller [candidate] (3.566 ms) : 0, 3566
IAST [baseline] (31.22 ms) : 0, 31220
IAST [candidate] (28.627 ms) : 0, 28627
section profiling
crashtracking [baseline] (1.22 ms) : 0, 1220
crashtracking [candidate] (1.196 ms) : 0, 1196
BytebuddyAgent [baseline] (696.977 ms) : 0, 696977
BytebuddyAgent [candidate] (695.434 ms) : 0, 695434
AgentMeter [baseline] (9.251 ms) : 0, 9251
AgentMeter [candidate] (9.245 ms) : 0, 9245
GlobalTracer [baseline] (208.577 ms) : 0, 208577
GlobalTracer [candidate] (208.344 ms) : 0, 208344
AppSec [baseline] (33.198 ms) : 0, 33198
AppSec [candidate] (33.151 ms) : 0, 33151
Debugger [baseline] (66.251 ms) : 0, 66251
Debugger [candidate] (66.442 ms) : 0, 66442
Remote Config [baseline] (581.716 µs) : 0, 582
Remote Config [candidate] (587.854 µs) : 0, 588
Telemetry [baseline] (7.879 ms) : 0, 7879
Telemetry [candidate] (7.845 ms) : 0, 7845
Flare Poller [baseline] (3.565 ms) : 0, 3565
Flare Poller [candidate] (3.612 ms) : 0, 3612
ProfilingAgent [baseline] (94.756 ms) : 0, 94756
ProfilingAgent [candidate] (94.828 ms) : 0, 94828
Profiling [baseline] (95.313 ms) : 0, 95313
Profiling [candidate] (95.386 ms) : 0, 95386
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 2 performance regressions! Performance is the same for 15 metrics, 17 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (18.79 ms) : 18597, 18984
. : milestone, 18790,
appsec (19.105 ms) : 18915, 19296
. : milestone, 19105,
code_origins (18.187 ms) : 18003, 18371
. : milestone, 18187,
iast (17.792 ms) : 17618, 17966
. : milestone, 17792,
profiling (18.663 ms) : 18474, 18852
. : milestone, 18663,
tracing (18.833 ms) : 18644, 19022
. : milestone, 18833,
section candidate
no_agent (18.245 ms) : 18061, 18429
. : milestone, 18245,
appsec (19.969 ms) : 19765, 20172
. : milestone, 19969,
code_origins (17.954 ms) : 17776, 18132
. : milestone, 17954,
iast (17.929 ms) : 17757, 18102
. : milestone, 17929,
profiling (19.279 ms) : 19086, 19471
. : milestone, 19279,
tracing (17.812 ms) : 17641, 17984
. : milestone, 17812,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (1.266 ms) : 1253, 1278
. : milestone, 1266,
iast (3.479 ms) : 3427, 3530
. : milestone, 3479,
iast_FULL (6.03 ms) : 5969, 6091
. : milestone, 6030,
iast_GLOBAL (3.73 ms) : 3668, 3793
. : milestone, 3730,
profiling (2.419 ms) : 2391, 2446
. : milestone, 2419,
tracing (1.898 ms) : 1882, 1915
. : milestone, 1898,
section candidate
no_agent (1.251 ms) : 1239, 1264
. : milestone, 1251,
iast (3.403 ms) : 3352, 3455
. : milestone, 3403,
iast_FULL (6.103 ms) : 6041, 6165
. : milestone, 6103,
iast_GLOBAL (3.676 ms) : 3614, 3737
. : milestone, 3676,
profiling (2.173 ms) : 2150, 2196
. : milestone, 2173,
tracing (1.899 ms) : 1883, 1915
. : milestone, 1899,
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~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (1.488 ms) : 1476, 1499
. : milestone, 1488,
appsec (3.765 ms) : 3548, 3983
. : milestone, 3765,
iast (2.278 ms) : 2208, 2347
. : milestone, 2278,
iast_GLOBAL (2.326 ms) : 2256, 2396
. : milestone, 2326,
profiling (2.107 ms) : 2052, 2162
. : milestone, 2107,
tracing (2.088 ms) : 2034, 2141
. : milestone, 2088,
section candidate
no_agent (1.491 ms) : 1479, 1503
. : milestone, 1491,
appsec (3.823 ms) : 3603, 4043
. : milestone, 3823,
iast (2.278 ms) : 2209, 2348
. : milestone, 2278,
iast_GLOBAL (2.327 ms) : 2257, 2397
. : milestone, 2327,
profiling (2.097 ms) : 2042, 2151
. : milestone, 2097,
tracing (2.096 ms) : 2042, 2149
. : milestone, 2096,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~15a0168cf0, baseline=1.62.0-SNAPSHOT~c72f06780f
dateFormat X
axisFormat %s
section baseline
no_agent (15.637 s) : 15637000, 15637000
. : milestone, 15637000,
appsec (14.776 s) : 14776000, 14776000
. : milestone, 14776000,
iast (18.631 s) : 18631000, 18631000
. : milestone, 18631000,
iast_GLOBAL (17.775 s) : 17775000, 17775000
. : milestone, 17775000,
profiling (15.329 s) : 15329000, 15329000
. : milestone, 15329000,
tracing (15.067 s) : 15067000, 15067000
. : milestone, 15067000,
section candidate
no_agent (14.871 s) : 14871000, 14871000
. : milestone, 14871000,
appsec (14.639 s) : 14639000, 14639000
. : milestone, 14639000,
iast (18.252 s) : 18252000, 18252000
. : milestone, 18252000,
iast_GLOBAL (17.908 s) : 17908000, 17908000
. : milestone, 17908000,
profiling (15.019 s) : 15019000, 15019000
. : milestone, 15019000,
tracing (15.09 s) : 15090000, 15090000
. : milestone, 15090000,
|
…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