Add server.request.body.filenames support for Jetty#10988
Draft
Add server.request.body.filenames support for Jetty#10988
Conversation
jandro996
added
comp: asm waf
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 60 metrics, 11 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.06 s) : 0, 1059674
Total [baseline] (11.112 s) : 0, 11111853
Agent [candidate] (1.055 s) : 0, 1054616
Total [candidate] (11.138 s) : 0, 11138348
section appsec
Agent [baseline] (1.251 s) : 0, 1250500
Total [baseline] (11.189 s) : 0, 11188819
Agent [candidate] (1.251 s) : 0, 1250887
Total [candidate] (11.215 s) : 0, 11215127
section iast
Agent [baseline] (1.235 s) : 0, 1235408
Total [baseline] (11.331 s) : 0, 11330846
Agent [candidate] (1.224 s) : 0, 1224330
Total [candidate] (11.258 s) : 0, 11258474
section profiling
Agent [baseline] (1.185 s) : 0, 1185337
Total [baseline] (10.942 s) : 0, 10942030
Agent [candidate] (1.191 s) : 0, 1190696
Total [candidate] (11.101 s) : 0, 11101240
gantt
title petclinic - break down per module: candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.224 ms) : 0, 1224
crashtracking [candidate] (1.224 ms) : 0, 1224
BytebuddyAgent [baseline] (633.731 ms) : 0, 633731
BytebuddyAgent [candidate] (631.516 ms) : 0, 631516
AgentMeter [baseline] (29.383 ms) : 0, 29383
AgentMeter [candidate] (29.29 ms) : 0, 29290
GlobalTracer [baseline] (248.905 ms) : 0, 248905
GlobalTracer [candidate] (248.755 ms) : 0, 248755
AppSec [baseline] (31.958 ms) : 0, 31958
AppSec [candidate] (31.793 ms) : 0, 31793
Debugger [baseline] (59.894 ms) : 0, 59894
Debugger [candidate] (59.763 ms) : 0, 59763
Remote Config [baseline] (602.596 µs) : 0, 603
Remote Config [candidate] (592.19 µs) : 0, 592
Telemetry [baseline] (8.097 ms) : 0, 8097
Telemetry [candidate] (8.05 ms) : 0, 8050
Flare Poller [baseline] (9.823 ms) : 0, 9823
Flare Poller [candidate] (7.531 ms) : 0, 7531
section appsec
crashtracking [baseline] (1.243 ms) : 0, 1243
crashtracking [candidate] (1.22 ms) : 0, 1220
BytebuddyAgent [baseline] (662.71 ms) : 0, 662710
BytebuddyAgent [candidate] (662.679 ms) : 0, 662679
AgentMeter [baseline] (12.043 ms) : 0, 12043
AgentMeter [candidate] (12.055 ms) : 0, 12055
GlobalTracer [baseline] (249.648 ms) : 0, 249648
GlobalTracer [candidate] (249.34 ms) : 0, 249340
AppSec [baseline] (184.679 ms) : 0, 184679
AppSec [candidate] (185.484 ms) : 0, 185484
Debugger [baseline] (66.42 ms) : 0, 66420
Debugger [candidate] (66.416 ms) : 0, 66416
Remote Config [baseline] (606.089 µs) : 0, 606
Remote Config [candidate] (610.264 µs) : 0, 610
Telemetry [baseline] (8.648 ms) : 0, 8648
Telemetry [candidate] (8.586 ms) : 0, 8586
Flare Poller [baseline] (3.556 ms) : 0, 3556
Flare Poller [candidate] (3.585 ms) : 0, 3585
IAST [baseline] (24.659 ms) : 0, 24659
IAST [candidate] (24.642 ms) : 0, 24642
section iast
crashtracking [baseline] (1.263 ms) : 0, 1263
crashtracking [candidate] (1.216 ms) : 0, 1216
BytebuddyAgent [baseline] (810.062 ms) : 0, 810062
BytebuddyAgent [candidate] (800.704 ms) : 0, 800704
AgentMeter [baseline] (11.471 ms) : 0, 11471
AgentMeter [candidate] (11.399 ms) : 0, 11399
GlobalTracer [baseline] (240.712 ms) : 0, 240712
GlobalTracer [candidate] (239.697 ms) : 0, 239697
AppSec [baseline] (31.848 ms) : 0, 31848
AppSec [candidate] (33.416 ms) : 0, 33416
Debugger [baseline] (60.701 ms) : 0, 60701
Debugger [candidate] (58.959 ms) : 0, 58959
Remote Config [baseline] (2.321 ms) : 0, 2321
Remote Config [candidate] (1.132 ms) : 0, 1132
Telemetry [baseline] (11.201 ms) : 0, 11201
Telemetry [candidate] (12.217 ms) : 0, 12217
Flare Poller [baseline] (3.512 ms) : 0, 3512
Flare Poller [candidate] (3.601 ms) : 0, 3601
IAST [baseline] (25.946 ms) : 0, 25946
IAST [candidate] (25.869 ms) : 0, 25869
section profiling
ProfilingAgent [baseline] (94.052 ms) : 0, 94052
ProfilingAgent [candidate] (94.92 ms) : 0, 94920
crashtracking [baseline] (1.195 ms) : 0, 1195
crashtracking [candidate] (1.178 ms) : 0, 1178
BytebuddyAgent [baseline] (692.317 ms) : 0, 692317
BytebuddyAgent [candidate] (694.114 ms) : 0, 694114
AgentMeter [baseline] (9.176 ms) : 0, 9176
AgentMeter [candidate] (9.207 ms) : 0, 9207
GlobalTracer [baseline] (207.112 ms) : 0, 207112
GlobalTracer [candidate] (208.76 ms) : 0, 208760
AppSec [baseline] (32.508 ms) : 0, 32508
AppSec [candidate] (32.704 ms) : 0, 32704
Debugger [baseline] (65.734 ms) : 0, 65734
Debugger [candidate] (66.23 ms) : 0, 66230
Remote Config [baseline] (580.685 µs) : 0, 581
Remote Config [candidate] (575.762 µs) : 0, 576
Telemetry [baseline] (7.887 ms) : 0, 7887
Telemetry [candidate] (7.959 ms) : 0, 7959
Flare Poller [baseline] (3.562 ms) : 0, 3562
Flare Poller [candidate] (3.578 ms) : 0, 3578
Profiling [baseline] (94.611 ms) : 0, 94611
Profiling [candidate] (95.494 ms) : 0, 95494
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1055893
Total [baseline] (8.859 s) : 0, 8858673
Agent [candidate] (1.061 s) : 0, 1061066
Total [candidate] (8.868 s) : 0, 8867711
section iast
Agent [baseline] (1.222 s) : 0, 1222418
Total [baseline] (9.568 s) : 0, 9567899
Agent [candidate] (1.225 s) : 0, 1225332
Total [candidate] (9.556 s) : 0, 9555880
gantt
title insecure-bank - break down per module: candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.231 ms) : 0, 1231
crashtracking [candidate] (1.222 ms) : 0, 1222
BytebuddyAgent [baseline] (632.513 ms) : 0, 632513
BytebuddyAgent [candidate] (636.988 ms) : 0, 636988
AgentMeter [baseline] (29.399 ms) : 0, 29399
AgentMeter [candidate] (29.477 ms) : 0, 29477
GlobalTracer [baseline] (248.615 ms) : 0, 248615
GlobalTracer [candidate] (248.447 ms) : 0, 248447
AppSec [baseline] (31.915 ms) : 0, 31915
AppSec [candidate] (31.957 ms) : 0, 31957
Debugger [baseline] (59.138 ms) : 0, 59138
Debugger [candidate] (59.208 ms) : 0, 59208
Remote Config [baseline] (608.398 µs) : 0, 608
Remote Config [candidate] (608.626 µs) : 0, 609
Telemetry [baseline] (8.083 ms) : 0, 8083
Telemetry [candidate] (8.056 ms) : 0, 8056
Flare Poller [baseline] (8.221 ms) : 0, 8221
Flare Poller [candidate] (8.957 ms) : 0, 8957
section iast
crashtracking [baseline] (1.242 ms) : 0, 1242
crashtracking [candidate] (1.232 ms) : 0, 1232
BytebuddyAgent [baseline] (800.0 ms) : 0, 800000
BytebuddyAgent [candidate] (802.446 ms) : 0, 802446
AgentMeter [baseline] (11.316 ms) : 0, 11316
AgentMeter [candidate] (11.408 ms) : 0, 11408
GlobalTracer [baseline] (239.236 ms) : 0, 239236
GlobalTracer [candidate] (239.64 ms) : 0, 239640
IAST [baseline] (25.766 ms) : 0, 25766
IAST [candidate] (25.787 ms) : 0, 25787
AppSec [baseline] (31.252 ms) : 0, 31252
AppSec [candidate] (30.466 ms) : 0, 30466
Debugger [baseline] (60.846 ms) : 0, 60846
Debugger [candidate] (61.161 ms) : 0, 61161
Remote Config [baseline] (540.061 µs) : 0, 540
Remote Config [candidate] (1.708 ms) : 0, 1708
Telemetry [baseline] (12.461 ms) : 0, 12461
Telemetry [candidate] (11.75 ms) : 0, 11750
Flare Poller [baseline] (3.401 ms) : 0, 3401
Flare Poller [candidate] (3.433 ms) : 0, 3433
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 18 metrics, 17 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section baseline
no_agent (1.234 ms) : 1222, 1246
. : milestone, 1234,
iast (3.313 ms) : 3270, 3356
. : milestone, 3313,
iast_FULL (6.015 ms) : 5955, 6075
. : milestone, 6015,
iast_GLOBAL (3.762 ms) : 3700, 3825
. : milestone, 3762,
profiling (2.455 ms) : 2430, 2480
. : milestone, 2455,
tracing (1.877 ms) : 1862, 1892
. : milestone, 1877,
section candidate
no_agent (1.246 ms) : 1234, 1258
. : milestone, 1246,
iast (3.302 ms) : 3258, 3345
. : milestone, 3302,
iast_FULL (5.96 ms) : 5900, 6019
. : milestone, 5960,
iast_GLOBAL (3.675 ms) : 3612, 3737
. : milestone, 3675,
profiling (2.455 ms) : 2428, 2482
. : milestone, 2455,
tracing (1.836 ms) : 1822, 1851
. : milestone, 1836,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section baseline
no_agent (18.264 ms) : 18079, 18449
. : milestone, 18264,
appsec (18.596 ms) : 18407, 18784
. : milestone, 18596,
code_origins (17.689 ms) : 17513, 17865
. : milestone, 17689,
iast (18.298 ms) : 18115, 18482
. : milestone, 18298,
profiling (18.537 ms) : 18351, 18723
. : milestone, 18537,
tracing (17.849 ms) : 17677, 18021
. : milestone, 17849,
section candidate
no_agent (19.464 ms) : 19265, 19663
. : milestone, 19464,
appsec (18.67 ms) : 18485, 18855
. : milestone, 18670,
code_origins (17.728 ms) : 17551, 17905
. : milestone, 17728,
iast (18.129 ms) : 17950, 18309
. : milestone, 18129,
profiling (18.568 ms) : 18379, 18757
. : milestone, 18568,
tracing (17.753 ms) : 17577, 17929
. : milestone, 17753,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 2 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section baseline
no_agent (15.027 s) : 15027000, 15027000
. : milestone, 15027000,
appsec (14.991 s) : 14991000, 14991000
. : milestone, 14991000,
iast (18.309 s) : 18309000, 18309000
. : milestone, 18309000,
iast_GLOBAL (18.033 s) : 18033000, 18033000
. : milestone, 18033000,
profiling (15.079 s) : 15079000, 15079000
. : milestone, 15079000,
tracing (14.906 s) : 14906000, 14906000
. : milestone, 14906000,
section candidate
no_agent (15.024 s) : 15024000, 15024000
. : milestone, 15024000,
appsec (14.814 s) : 14814000, 14814000
. : milestone, 14814000,
iast (18.453 s) : 18453000, 18453000
. : milestone, 18453000,
iast_GLOBAL (18.022 s) : 18022000, 18022000
. : milestone, 18022000,
profiling (14.813 s) : 14813000, 14813000
. : milestone, 14813000,
tracing (15.046 s) : 15046000, 15046000
. : milestone, 15046000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~4d8ca56959, baseline=1.61.0-SNAPSHOT~aa7c70f2e7
dateFormat X
axisFormat %s
section baseline
no_agent (1.502 ms) : 1490, 1513
. : milestone, 1502,
appsec (3.87 ms) : 3645, 4095
. : milestone, 3870,
iast (2.3 ms) : 2230, 2370
. : milestone, 2300,
iast_GLOBAL (2.338 ms) : 2268, 2407
. : milestone, 2338,
profiling (2.547 ms) : 2383, 2711
. : milestone, 2547,
tracing (2.12 ms) : 2065, 2174
. : milestone, 2120,
section candidate
no_agent (1.494 ms) : 1482, 1506
. : milestone, 1494,
appsec (3.878 ms) : 3655, 4101
. : milestone, 3878,
iast (2.294 ms) : 2225, 2364
. : milestone, 2294,
iast_GLOBAL (2.346 ms) : 2276, 2417
. : milestone, 2346,
profiling (2.124 ms) : 2069, 2180
. : milestone, 2124,
tracing (2.111 ms) : 2057, 2166
. : milestone, 2111,
|
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61873-3
branch
from
e3d4073 to
e2d5ed0
Compare
Add GetFilenamesAdvice to all three Jetty AppSec modules to collect uploaded file names from multipart requests and fire the requestFilesFilenames() IG callback: - jetty-appsec-8.1.3: intercepts getParts() return value; includes Content-Disposition header fallback for Servlet 3.0 (Jetty 9.0) where getSubmittedFileName() is not available - jetty-appsec-9.2: intercepts no-arg getParts() for Servlet 3.1+ - jetty-appsec-9.3: same, applies to Jetty 9.3, 10, 11 Enable testBodyFilenames() in Jetty 9.x, 10 and 11 server tests.
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61873-3
branch
from
f2998c3 to
629f074
Compare
| } | ||
| } | ||
| // Fallback: parse filename from Content-Disposition header (Servlet 3.0) | ||
| if (name == null) { |
There was a problem hiding this comment.
Shouldn't this be outside of the main parts loop?
Member
Author
There was a problem hiding this comment.
Fixed. Restructured into two separate loops chosen once before iteration: if getSubmittedFileName != null (Servlet 3.1+) iterate using that method; otherwise iterate parsing the Content-Disposition header (Servlet 3.0 fallback). No per-part branching inside the loop.
| transformer.applyAdvice( | ||
| named("extractContentParameters").and(takesArguments(0)).or(named("getParts")), | ||
| getClass().getName() + "$ExtractContentParametersAdvice"); | ||
| transformer.applyAdvice(named("getParts"), getClass().getName() + "$GetFilenamesAdvice"); |
Member
Author
There was a problem hiding this comment.
Fixed. GetFilenamesAdvice now has a call-depth guard (CallDepthThreadLocalMap with Collection.class) to avoid double-firing when getParts() internally calls getParts(MultiMap)
![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: c732823549
ℹ️ 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".
...va/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java
Outdated
Show resolved
Hide resolved
...va/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java
Outdated
Show resolved
Hide resolved
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61873-3
branch
2 times, most recently
from
ecf65c5 to
eae08aa
Compare
…MultiMap) path - jetty-appsec-9.3: add call-depth guard (Collection.class) to GetFilenamesAdvice to prevent double callback invocation when getParts() calls getParts(MultiMap) internally - jetty-appsec-9.2: extend GetFilenamesAdvice matcher to all getParts overloads (not just no-arg) to cover getParameter*()/getParameterMap() code paths, guarded with same call-depth mechanism to avoid double-firing
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61873-3
branch
2 times, most recently
from
3ab9ff7 to
77ec572
Compare
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61873-3
branch
from
2e72584 to
d37e03e
Compare
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61873-3
branch
from
d37e03e to
d8a92f8
Compare
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
2 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
GetFilenamesAdviceto all three Jetty AppSec instrumentation modules to collect uploaded file names from multipart requests and fire therequestFilesFilenames()IG callback:jetty-appsec-8.1.3: interceptsgetParts()return value; includes Content-Disposition header fallback for Servlet 3.0 (Jetty 9.0) wheregetSubmittedFileName()is not availablejetty-appsec-9.2: intercepts no-arggetParts()for Servlet 3.1+jetty-appsec-9.3: same pattern, applies to Jetty 9.3, 10, 11testBodyFilenames()in Jetty 9.x, 10, and 11 server teststestBodyFilenames() = falseinJettyAsyncHandlerTest— async re-dispatch changes how Jetty processes multipart parts, the tag is not set in that variantMotivation
Additional Notes
Depends on #10973 (merged).
Part of Jira ticket: APPSEC-61873 —
server.request.body.filenamesimplementation across server frameworks.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