Uh oh!
There was an error while loading. Please reload this page.
Expose uploaded file content as new WAF address server.request.body.files_content for commons-fileupload - #11137
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 12 commits intoApr 27, 2026
Conversation
Contributor
Hi! 👋 Looks like you updated a Git Submodule.
|
…est.body.files_content Introduces a new AppSec WAF address `server.request.body.files_content` (`List<String>`) that exposes the content of each uploaded file in a multipart/form-data request. Entries correspond positionally to the existing `server.request.body.filenames` address. Content is capped at 4 096 bytes per file (ISO-8859-1) to keep memory usage bounded. Changes: - KnownAddresses: add REQUEST_FILES_CONTENT + forName() case - Events: add requestFilesContent event (ID 31); FILE_WRITTEN bumped to 32 - InstrumentationGateway: register the new BiFunction case - GatewayBridge: add onRequestFilesContent handler + DATA_DEPENDENCIES entry - CommonsFileUploadAppSecModule: after firing filenames, fire content (skipped when the filenames event already blocked the request) - Unit tests: GatewayBridgeSpecification, GatewayBridgeIGRegistrationSpecification, KnownAddressesSpecificationForkedTest - Smoke test: 'block request based on malicious file upload content' verifies end-to-end blocking via a custom WAF rule on the new address Closes APPSEC-61875
jandro996force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
April 16, 2026 14:12
0de9320 to
37e7d09CompareBenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 63 metrics, 8 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1056467
Total [baseline] (8.812 s) : 0, 8811965
Agent [candidate] (1.055 s) : 0, 1055069
Total [candidate] (8.827 s) : 0, 8826835
section iast
Agent [baseline] (1.247 s) : 0, 1247204
Total [baseline] (9.559 s) : 0, 9559402
Agent [candidate] (1.233 s) : 0, 1233198
Total [candidate] (9.523 s) : 0, 9522936
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.223 ms) : 0, 1223
crashtracking [candidate] (1.215 ms) : 0, 1215
BytebuddyAgent [baseline] (634.016 ms) : 0, 634016
BytebuddyAgent [candidate] (632.317 ms) : 0, 632317
AgentMeter [baseline] (29.52 ms) : 0, 29520
AgentMeter [candidate] (29.519 ms) : 0, 29519
GlobalTracer [baseline] (248.596 ms) : 0, 248596
GlobalTracer [candidate] (248.677 ms) : 0, 248677
AppSec [baseline] (32.369 ms) : 0, 32369
AppSec [candidate] (32.377 ms) : 0, 32377
Debugger [baseline] (59.133 ms) : 0, 59133
Debugger [candidate] (58.969 ms) : 0, 58969
Remote Config [baseline] (599.384 µs) : 0, 599
Remote Config [candidate] (594.057 µs) : 0, 594
Telemetry [baseline] (8.009 ms) : 0, 8009
Telemetry [candidate] (7.95 ms) : 0, 7950
Flare Poller [baseline] (6.78 ms) : 0, 6780
Flare Poller [candidate] (7.321 ms) : 0, 7321
section iast
crashtracking [baseline] (1.255 ms) : 0, 1255
crashtracking [candidate] (1.225 ms) : 0, 1225
BytebuddyAgent [baseline] (821.217 ms) : 0, 821217
BytebuddyAgent [candidate] (812.846 ms) : 0, 812846
AgentMeter [baseline] (11.63 ms) : 0, 11630
AgentMeter [candidate] (11.476 ms) : 0, 11476
GlobalTracer [baseline] (241.092 ms) : 0, 241092
GlobalTracer [candidate] (238.704 ms) : 0, 238704
IAST [baseline] (29.532 ms) : 0, 29532
IAST [candidate] (31.475 ms) : 0, 31475
AppSec [baseline] (29.849 ms) : 0, 29849
AppSec [candidate] (26.363 ms) : 0, 26363
Debugger [baseline] (64.421 ms) : 0, 64421
Debugger [candidate] (63.54 ms) : 0, 63540
Remote Config [baseline] (547.252 µs) : 0, 547
Remote Config [candidate] (521.194 µs) : 0, 521
Telemetry [baseline] (7.865 ms) : 0, 7865
Telemetry [candidate] (7.639 ms) : 0, 7639
Flare Poller [baseline] (3.427 ms) : 0, 3427
Flare Poller [candidate] (3.412 ms) : 0, 3412
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1057093
Total [baseline] (11.11 s) : 0, 11110188
Agent [candidate] (1.056 s) : 0, 1055516
Total [candidate] (11.072 s) : 0, 11071883
section appsec
Agent [baseline] (1.27 s) : 0, 1270444
Total [baseline] (11.001 s) : 0, 11000925
Agent [candidate] (1.26 s) : 0, 1260349
Total [candidate] (11.031 s) : 0, 11030600
section iast
Agent [baseline] (1.24 s) : 0, 1240436
Total [baseline] (11.366 s) : 0, 11365621
Agent [candidate] (1.231 s) : 0, 1231433
Total [candidate] (11.295 s) : 0, 11295145
section profiling
Agent [baseline] (1.2 s) : 0, 1199872
Total [baseline] (11.13 s) : 0, 11130262
Agent [candidate] (1.19 s) : 0, 1190445
Total [candidate] (10.976 s) : 0, 10975855
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.226 ms) : 0, 1226
crashtracking [candidate] (1.223 ms) : 0, 1223
BytebuddyAgent [baseline] (633.325 ms) : 0, 633325
BytebuddyAgent [candidate] (631.892 ms) : 0, 631892
AgentMeter [baseline] (29.568 ms) : 0, 29568
AgentMeter [candidate] (29.566 ms) : 0, 29566
GlobalTracer [baseline] (248.768 ms) : 0, 248768
GlobalTracer [candidate] (248.828 ms) : 0, 248828
AppSec [baseline] (32.279 ms) : 0, 32279
AppSec [candidate] (32.348 ms) : 0, 32348
Debugger [baseline] (59.759 ms) : 0, 59759
Debugger [candidate] (59.843 ms) : 0, 59843
Remote Config [baseline] (595.318 µs) : 0, 595
Remote Config [candidate] (587.71 µs) : 0, 588
Telemetry [baseline] (8.022 ms) : 0, 8022
Telemetry [candidate] (7.981 ms) : 0, 7981
Flare Poller [baseline] (7.457 ms) : 0, 7457
Flare Poller [candidate] (7.284 ms) : 0, 7284
section appsec
crashtracking [baseline] (1.223 ms) : 0, 1223
crashtracking [candidate] (1.233 ms) : 0, 1233
BytebuddyAgent [baseline] (676.101 ms) : 0, 676101
BytebuddyAgent [candidate] (673.252 ms) : 0, 673252
AgentMeter [baseline] (12.314 ms) : 0, 12314
AgentMeter [candidate] (12.158 ms) : 0, 12158
GlobalTracer [baseline] (251.804 ms) : 0, 251804
GlobalTracer [candidate] (248.629 ms) : 0, 248629
IAST [baseline] (24.882 ms) : 0, 24882
IAST [candidate] (24.263 ms) : 0, 24263
AppSec [baseline] (188.904 ms) : 0, 188904
AppSec [candidate] (185.578 ms) : 0, 185578
Debugger [baseline] (66.535 ms) : 0, 66535
Debugger [candidate] (66.954 ms) : 0, 66954
Remote Config [baseline] (591.37 µs) : 0, 591
Remote Config [candidate] (575.077 µs) : 0, 575
Telemetry [baseline] (8.049 ms) : 0, 8049
Telemetry [candidate] (7.934 ms) : 0, 7934
Flare Poller [baseline] (3.598 ms) : 0, 3598
Flare Poller [candidate] (3.452 ms) : 0, 3452
section iast
crashtracking [baseline] (1.242 ms) : 0, 1242
crashtracking [candidate] (1.221 ms) : 0, 1221
BytebuddyAgent [baseline] (815.988 ms) : 0, 815988
BytebuddyAgent [candidate] (808.333 ms) : 0, 808333
AgentMeter [baseline] (11.53 ms) : 0, 11530
AgentMeter [candidate] (11.411 ms) : 0, 11411
GlobalTracer [baseline] (240.471 ms) : 0, 240471
GlobalTracer [candidate] (239.081 ms) : 0, 239081
IAST [baseline] (31.87 ms) : 0, 31870
IAST [candidate] (30.017 ms) : 0, 30017
AppSec [baseline] (27.427 ms) : 0, 27427
AppSec [candidate] (27.852 ms) : 0, 27852
Debugger [baseline] (63.947 ms) : 0, 63947
Debugger [candidate] (65.774 ms) : 0, 65774
Remote Config [baseline] (535.041 µs) : 0, 535
Remote Config [candidate] (533.073 µs) : 0, 533
Telemetry [baseline] (7.773 ms) : 0, 7773
Telemetry [candidate] (7.797 ms) : 0, 7797
Flare Poller [baseline] (3.442 ms) : 0, 3442
Flare Poller [candidate] (3.38 ms) : 0, 3380
section profiling
crashtracking [baseline] (1.204 ms) : 0, 1204
crashtracking [candidate] (1.178 ms) : 0, 1178
BytebuddyAgent [baseline] (701.955 ms) : 0, 701955
BytebuddyAgent [candidate] (696.201 ms) : 0, 696201
AgentMeter [baseline] (9.323 ms) : 0, 9323
AgentMeter [candidate] (9.29 ms) : 0, 9290
GlobalTracer [baseline] (209.526 ms) : 0, 209526
GlobalTracer [candidate] (208.035 ms) : 0, 208035
AppSec [baseline] (33.058 ms) : 0, 33058
AppSec [candidate] (32.748 ms) : 0, 32748
Debugger [baseline] (66.205 ms) : 0, 66205
Debugger [candidate] (65.577 ms) : 0, 65577
Remote Config [baseline] (582.869 µs) : 0, 583
Remote Config [candidate] (578.681 µs) : 0, 579
Telemetry [baseline] (7.88 ms) : 0, 7880
Telemetry [candidate] (7.817 ms) : 0, 7817
Flare Poller [baseline] (3.554 ms) : 0, 3554
Flare Poller [candidate] (3.52 ms) : 0, 3520
ProfilingAgent [baseline] (94.438 ms) : 0, 94438
ProfilingAgent [candidate] (94.03 ms) : 0, 94030
Profiling [baseline] (95.025 ms) : 0, 95025
Profiling [candidate] (94.592 ms) : 0, 94592
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 3 performance regressions! Performance is the same for 16 metrics, 16 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (1.238 ms) : 1226, 1250
. : milestone, 1238,
iast (3.487 ms) : 3436, 3539
. : milestone, 3487,
iast_FULL (6.011 ms) : 5950, 6071
. : milestone, 6011,
iast_GLOBAL (3.562 ms) : 3509, 3615
. : milestone, 3562,
profiling (2.198 ms) : 2179, 2218
. : milestone, 2198,
tracing (1.975 ms) : 1958, 1993
. : milestone, 1975,
section candidate
no_agent (1.253 ms) : 1241, 1265
. : milestone, 1253,
iast (3.332 ms) : 3282, 3382
. : milestone, 3332,
iast_FULL (6.123 ms) : 6060, 6186
. : milestone, 6123,
iast_GLOBAL (3.786 ms) : 3716, 3856
. : milestone, 3786,
profiling (2.176 ms) : 2156, 2196
. : milestone, 2176,
tracing (1.913 ms) : 1895, 1931
. : milestone, 1913,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (18.079 ms) : 17896, 18261
. : milestone, 18079,
appsec (18.851 ms) : 18659, 19044
. : milestone, 18851,
code_origins (18.018 ms) : 17841, 18195
. : milestone, 18018,
iast (18.72 ms) : 18533, 18906
. : milestone, 18720,
profiling (19.319 ms) : 19125, 19513
. : milestone, 19319,
tracing (18.469 ms) : 18287, 18651
. : milestone, 18469,
section candidate
no_agent (18.069 ms) : 17886, 18251
. : milestone, 18069,
appsec (19.19 ms) : 18996, 19384
. : milestone, 19190,
code_origins (19.574 ms) : 19382, 19766
. : milestone, 19574,
iast (18.085 ms) : 17903, 18267
. : milestone, 18085,
profiling (19.01 ms) : 18822, 19198
. : milestone, 19010,
tracing (17.999 ms) : 17823, 18175
. : milestone, 17999,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 0 unstable metrics.
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (15.567 s) : 15567000, 15567000
. : milestone, 15567000,
appsec (14.693 s) : 14693000, 14693000
. : milestone, 14693000,
iast (18.669 s) : 18669000, 18669000
. : milestone, 18669000,
iast_GLOBAL (17.807 s) : 17807000, 17807000
. : milestone, 17807000,
profiling (14.771 s) : 14771000, 14771000
. : milestone, 14771000,
tracing (14.854 s) : 14854000, 14854000
. : milestone, 14854000,
section candidate
no_agent (15.443 s) : 15443000, 15443000
. : milestone, 15443000,
appsec (15.014 s) : 15014000, 15014000
. : milestone, 15014000,
iast (18.668 s) : 18668000, 18668000
. : milestone, 18668000,
iast_GLOBAL (18.066 s) : 18066000, 18066000
. : milestone, 18066000,
profiling (15.493 s) : 15493000, 15493000
. : milestone, 15493000,
tracing (14.767 s) : 14767000, 14767000
. : milestone, 14767000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (1.488 ms) : 1476, 1499
. : milestone, 1488,
appsec (3.832 ms) : 3609, 4056
. : milestone, 3832,
iast (2.272 ms) : 2203, 2342
. : milestone, 2272,
iast_GLOBAL (2.33 ms) : 2259, 2401
. : milestone, 2330,
profiling (2.1 ms) : 2045, 2155
. : milestone, 2100,
tracing (2.077 ms) : 2023, 2130
. : milestone, 2077,
section candidate
no_agent (1.486 ms) : 1474, 1497
. : milestone, 1486,
appsec (2.548 ms) : 2493, 2603
. : milestone, 2548,
iast (2.271 ms) : 2201, 2341
. : milestone, 2271,
iast_GLOBAL (2.321 ms) : 2251, 2391
. : milestone, 2321,
profiling (2.101 ms) : 2046, 2156
. : milestone, 2101,
tracing (2.077 ms) : 2024, 2131
. : milestone, 2077,
|
jandro996
marked this pull request as ready for review
April 17, 2026 07:50
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:baf17b2c8e
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
jandro996force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
April 17, 2026 07:57
baf17b2 to
304846bComparejandro996force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
April 17, 2026 12:55
a36ff44 to
22d70a0CompareThe static readContent method in ParseRequestAdvice created a self-reference in the inlined advice bytecode (invokestatic on CommonsFileUploadAppSecModule$ParseRequestAdvice) that muzzle could not resolve in the application classloader, causing the instrumentation to be silently skipped. Moves readContent to a new FileItemContentReader helper class declared via helperClassNames(), which muzzle skips and the HelperInjector injects into the application classloader at runtime.
jandro996force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
April 17, 2026 14:20
4610028 to
2076c7bCompareUh oh!
There was an error while loading. Please reload this page.
Without a bound, uploading N files would pass up to N × 4096 bytes to the WAF in a single call. MAX_FILES_TO_INSPECT = 25 limits total content to at most 100 KB, consistent with the per-file MAX_CONTENT_BYTES cap.
jandro996
marked this pull request as draft
April 22, 2026 12:03
jandro996
marked this pull request as ready for review
April 22, 2026 12:48
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:92ed45d8a8
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…strumentation - Rename CommonsFileUploadAppSecModule to CommonsFileUploadAppSecInstrumentation - Extract readContents() loop to FileItemContentReader for testability - Add unit tests for readContents() including MAX_FILES_TO_INSPECT cap - Parametrize readContent size boundary tests with where: block - Fix incorrect positional-alignment claim in KnownAddresses Javadoc - Fix FileItemContentReader Javadoc to describe purpose not deployment
jandro996
commented
Apr 23, 2026
MemberAuthor
Not sure if make sense to make it configurable, what do you think? |
jandro996
commented
Apr 24, 2026
MemberAuthor
@codex review |
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
smola
approved these changes
Apr 27, 2026
Contributor
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
github-merge-queueBot
removed this pull request from the merge queue due to failed status checks
Apr 27, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
gh-worker-dd-mergequeue-cf854dBot
deleted the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
April 27, 2026 08:21
jandro996 added a commit
that referenced
this pull request
May 4, 2026
…monsFileUpload pattern Fetch both callbacks upfront before the collection loop, derive inspectContent from the captured contentCb reference, and add early exit when neither callback is registered. Eliminates a redundant getCallback() call and matches the pattern used in CommonsFileUploadAppSecInstrumentation (PR #11137).
gh-worker-dd-mergequeue-cf854dBot
pushed a commit
that referenced
this pull request
May 21, 2026
…ara (#11267) feat(appsec/tomcat): add GlassFish/Payara multipart filename instrumentation GlassFish 5 / Payara 5 does not have Request.parseParts() — instead Request.getParts() delegates entirely to org.apache.catalina.fileupload.Multipart.getParts(). That class uses its own ArrayList<Part> field (INVOKEVIRTUAL, not INVOKEINTERFACE) and calls a private initParts() instead of the Tomcat parseParts() that ParsePartsInstrumentation looks for. As a result, ParsePartsInstrumentation is a complete no-op on Payara: the method matcher finds nothing, and the bytecode visitor intercepts no Collection.add() calls. File names never reach the WAF even though the request is parsed without error. GlassFishMultipartInstrumentation targets org.apache.catalina.fileupload.Multipart.getParts() — a public method that exists only in GlassFish/Payara's web-core.jar and is therefore automatically skipped by ByteBuddy on standard Tomcat. After the method returns it iterates the Collection<Part> result and uses the existing ParameterCollector reflection helpers (getSubmittedFileName()) to extract file names, then fires requestFilesFilenames (and optionally requestFilesContent) callbacks exactly as ParsePartsInstrumentation does. fix(appsec): add muzzleDirective and requestFilesContent to GlassFish/Payara multipart instrumentation - Add `muzzleDirective() { return "glassfish"; }` to prevent the instrumentation from being included in Tomcat muzzle tests, which would violate the `assertInverse` constraint of the `from703` block and fail CI - Consolidate double `getCallbackProvider()` call into a single variable - Add glassfish muzzle block to build.gradle for CI validation against glassfish-embedded-all artifacts - Remove glassfish-embedded-all from testImplementation (its bundled Guava conflicts with the test bootstrap classpath setup) refactor(appsec): align GlassFish multipart advice with canonical CommonsFileUpload pattern Fetch both callbacks upfront before the collection loop, derive inspectContent from the captured contentCb reference, and add early exit when neither callback is registered. Eliminates a redundant getCallback() call and matches the pattern used in CommonsFileUploadAppSecInstrumentation (PR #11137). fix(appsec): bypass Java module-system IllegalAccessException in GlassFish multipart instrumentation On Java 11+ with GlassFish/Payara, reflective Method.invoke() from the injected ParameterCollector helper (unnamed module) to PartItem (GlassFish named module) fails with IllegalAccessException — setAccessible(true) is also blocked by the module system. Replace reflection-based approach with a direct cast to javax.servlet.http.Part inside the @Advice.OnMethodExit body. Because ByteBuddy inlines the advice into the target class (Multipart), it runs in the same classloader/module context as PartItem, so virtual dispatch through the Part interface works without any reflective access. Changes: - Rewrite GetPartsAdvice to cast each part to javax.servlet.http.Part and call getSubmittedFileName()/getInputStream()/getContentType() directly on the interface - Add compileOnly javax.servlet:javax.servlet-api:3.1.0 so getSubmittedFileName() (added in Servlet 3.1) is available at compile time; tomcat-catalina:7.0.4 ships only Servlet 3.0 - Add setAccessible(true) to ParameterCollector.resolveAndCache() (defensive; used by ParsePartsInstrumentation on Tomcat where reflection still works) - Remove helperClassNames() override — no helper injection needed for GlassFish path fix(appsec): protect each GlassFish multipart part iteration with per-item try/catch An uncaught exception from getSubmittedFileName() on part N would short-circuit the loop silently, leaving parts N+1..M unprocessed and the WAF with partial data. suppress=Throwable.class on @Advice.OnMethodExit only swallows the exception after the loop exits — it does not protect individual iterations. Wrap the entire per-part body in try/catch(Exception ignored) so a broken PartItem skips that part without affecting the remaining ones. revert: remove unnecessary setAccessible(true) from ParameterCollector These calls were added during an attempt to fix the GlassFish IllegalAccessException via reflection, but that approach was superseded by inlining the advice directly into Multipart (which avoids reflection entirely). ParameterCollector is only used by ParsePartsInstrumentation on standard Tomcat, where reflection on ApplicationPart works without setAccessible(true) — as tests confirmed before this change. style: replace FQN datadog.trace.api.Config with regular import style: skip filenames collection when filenamesCb is null Align with CommonsFileUploadAppSecInstrumentation: only populate the filenames list when filenamesCallback is actually registered. Previously the list was created and filled regardless, then silently dropped at dispatch time. No correctness impact, but avoids unnecessary ArrayList allocation and string copies when only the content callback is registered. feat(appsec): add blocking fallback for GlassFish/Payara multipart instrumentation TomcatServerInstrumentation is muzzled out for Payara's response type, so BlockResponseFunction is never registered. Add GlassFishBlockingHelper that commits the blocking response directly via Servlet API, extracting the HttpServletResponse from Multipart.request (private field) via reflection. The setAccessible call works because the advice is inlined into Multipart.getParts() — the same module as the field owner. Verified against system-tests APPSEC_BLOCKING with spring-boot-payara: - Test_Blocking_request_body_filenames: passes (was missing_feature) - Test_Blocking_request_body_files_content: passes (was missing_feature) style(appsec): address PR review feedback on GlassFish multipart instrumentation - Use try-with-resources for OutputStream in GlassFishBlockingHelper - Cache Config limit values as static finals in GlassFishBlockingHelper to avoid per-request Config.get() calls (consistent with ParameterCollector) - Remove Config import from GlassFishMultipartInstrumentation (now reads limits from GlassFishBlockingHelper static fields) - Add comment to skipVersions explaining the jakarta namespace reason - Add assertInverse to glassfish muzzle block for defensive CI coverage fix: restrict GlassFish muzzle range to [4.0, 6.1.0) to exclude jakarta.* versions Merge branch 'master' into alejandro.gonzalez/APPSEC-61873-payara Merge branch 'master' into alejandro.gonzalez/APPSEC-61873-payara fix(appsec): make GlassFishBlockingHelper fields public and avoid BlockingException for Payara blocking ByteBuddy inlines advice into Multipart.getParts() (package org.apache.catalina.fileupload), which is a different package than GlassFishBlockingHelper. Package-private static fields caused IllegalAccessError at runtime crashing the Payara container. BlockingException thrown from getParts() propagated through Payara's Grizzly pipeline and caused an ungraceful shutdown. Replace with parts = Collections.emptyList() so the response is committed (via commitBlocking fallback) and the controller skips processing the parts. refactor(appsec): extract tryBlock() helper and add unit tests for GlassFishBlockingHelper Extract the repeated brf/fallback blocking pattern from the GlassFish multipart advice into GlassFishBlockingHelper.tryBlock(). This eliminates the duplication between the filenames and content blocking paths, and fixes the ordering issue where blocked=true was assigned after effectivelyBlocked() — if effectivelyBlocked() threw, blocked would stay false and the content callback would fire after filenames already blocked. tryBlock() uses a two-phase try/catch: the outer block handles commit failures (returning false), while the inner block wraps effectivelyBlocked() so a thrown exception does not suppress the true return value when the response was already committed. Add GlassFishBlockingHelperTest covering all branches of commitBlocking() and tryBlock(). refactor: move GlassFish multipart processing into GlassFishBlockingHelper Extract the part-iteration loop and IG callback dispatch from the advice into a new GlassFishBlockingHelper.processPartsAndBlock() static method. The advice now contains only the inlined reflection block (which must stay inlined due to Java module-system constraints) and a single call to the helper. Unit tests added for processPartsAndBlock(): form-field skip, empty filename, normal filename, content reading, maxFiles limit, getInputStream failure fallback, sequential callback ordering (filenames blocks → content not fired), content callback blocks → returns true, non-Part object skip. style(appsec): address manuel review comments on GlassFish instrumentation Use @advice.FieldValue("request") instead of getDeclaredField+setAccessible to access Multipart.request — ByteBuddy inlines direct field access, no reflection needed for the field itself. getResponse() still uses reflection since the return type is a Payara-internal class. Add explanatory comments to all catch(Exception ignored) blocks in both the advice and GlassFishBlockingHelper. Type @advice.FieldValue to connector.Request to eliminate all reflection Typing the field to org.apache.catalina.connector.Request allows calling getResponse() directly, removing the Method reflection block entirely. Merge branch 'master' into alejandro.gonzalez/APPSEC-61873-payara Revert @advice.FieldValue type to Object to fix muzzle AssertPass org.apache.catalina.connector.Request is not exported by glassfish-embedded-all, so typing the field parameter to that class adds a bytecode reference that muzzle cannot resolve, causing the AssertPass check for GlassFish 4.0 to fail. Keep @advice.FieldValue (avoids setAccessible across module boundaries) but type as Object. getResponse() stays reflective for the same reason. Eliminate reflection in GlassFishMultipartInstrumentation Type @advice.FieldValue("request") as org.apache.catalina.connector.Request instead of Object. Pass catRequest directly to GlassFishBlockingHelper, which casts it to the org.apache.catalina.Request Catalina interface (available in GlassFish/Payara via HttpRequest) to reach HttpServletResponse without any Method.invoke() reflection. The org.apache.catalina.Request.getResponse() method returns org.apache.catalina.Response with a stable JVM descriptor shared between Tomcat and GlassFish, so no muzzle descriptor mismatch occurs. This is not possible calling connector.Request.getResponse() directly because Tomcat returns connector.Response while GlassFish returns org.apache.catalina.Response. Add glassfish-embedded-all:4.0 as compileOnly dependency so that org.apache.catalina.Request and org.apache.catalina.Response — GlassFish-specific interfaces removed from Tomcat 7.x public API — are available at compile time. Also add missing comment to the empty catch for effectivelyBlocked(). Merge branch 'master' into alejandro.gonzalez/APPSEC-61873-payara ci: retrigger CI Type @advice.FieldValue as org.apache.catalina.Request interface Using the Catalina Request interface (instead of connector.Request) as the @advice.FieldValue parameter type lets muzzle verify that the instrumented field type (connector.Request) still implements that interface in every supported GlassFish version, catching any future breakage at build time. The interface's getResponse() descriptor is stable across Tomcat and GlassFish, which also removes the need for the instanceof cast in the helper. Add glassfish-embedded-all:4.0 as testCompileOnly so the test sources can resolve the interface type. Merge branch 'master' into alejandro.gonzalez/APPSEC-61873-payara Merge branch 'master' into alejandro.gonzalez/APPSEC-61873-payara Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
This was referenced Jun 23, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
server.request.body.files_contentaddress andrequestFilesContent()event in the gateway API, wired throughGatewayBridgeandInstrumentationGatewayServletFileUpload.parseRequest()instrumentation (commons-fileupload) to read up to 4096 bytes of each uploaded file's content and fire the new WAF callback; blocks with aBlockingExceptiononRequestBlockingAction; content event is skipped when the filenames event has already blocked the requestAdditional Info
request.getParts(), Jetty, Liberty) will follow in successive PRsContributor 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 issueJira ticket: APPSEC-61875
Note:Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels 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.