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 59 metrics, 12 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.053 s) : 0, 1052822
Total [baseline] (8.813 s) : 0, 8813274
Agent [candidate] (1.065 s) : 0, 1064919
Total [candidate] (8.859 s) : 0, 8858935
section iast
Agent [baseline] (1.224 s) : 0, 1223771
Total [baseline] (9.562 s) : 0, 9562248
Agent [candidate] (1.221 s) : 0, 1221352
Total [candidate] (9.562 s) : 0, 9561935
gantt
title insecure-bank - break down per module: candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.244 ms) : 0, 1244
crashtracking [candidate] (1.211 ms) : 0, 1211
BytebuddyAgent [baseline] (632.049 ms) : 0, 632049
BytebuddyAgent [candidate] (640.592 ms) : 0, 640592
AgentMeter [baseline] (29.289 ms) : 0, 29289
AgentMeter [candidate] (29.411 ms) : 0, 29411
GlobalTracer [baseline] (247.26 ms) : 0, 247260
GlobalTracer [candidate] (249.64 ms) : 0, 249640
AppSec [baseline] (31.897 ms) : 0, 31897
AppSec [candidate] (32.199 ms) : 0, 32199
Debugger [baseline] (58.984 ms) : 0, 58984
Debugger [candidate] (59.441 ms) : 0, 59441
Remote Config [baseline] (602.256 µs) : 0, 602
Remote Config [candidate] (592.978 µs) : 0, 593
Telemetry [baseline] (8.012 ms) : 0, 8012
Telemetry [candidate] (8.083 ms) : 0, 8083
Flare Poller [baseline] (7.401 ms) : 0, 7401
Flare Poller [candidate] (7.367 ms) : 0, 7367
section iast
crashtracking [baseline] (1.217 ms) : 0, 1217
crashtracking [candidate] (1.185 ms) : 0, 1185
BytebuddyAgent [baseline] (801.989 ms) : 0, 801989
BytebuddyAgent [candidate] (799.865 ms) : 0, 799865
AgentMeter [baseline] (11.397 ms) : 0, 11397
AgentMeter [candidate] (11.366 ms) : 0, 11366
GlobalTracer [baseline] (238.684 ms) : 0, 238684
GlobalTracer [candidate] (238.528 ms) : 0, 238528
IAST [baseline] (25.89 ms) : 0, 25890
IAST [candidate] (25.887 ms) : 0, 25887
AppSec [baseline] (31.188 ms) : 0, 31188
AppSec [candidate] (32.294 ms) : 0, 32294
Debugger [baseline] (62.26 ms) : 0, 62260
Debugger [candidate] (56.778 ms) : 0, 56778
Remote Config [baseline] (1.73 ms) : 0, 1730
Remote Config [candidate] (522.634 µs) : 0, 523
Telemetry [baseline] (9.916 ms) : 0, 9916
Telemetry [candidate] (14.526 ms) : 0, 14526
Flare Poller [baseline] (3.394 ms) : 0, 3394
Flare Poller [candidate] (3.987 ms) : 0, 3987
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1056792
Total [baseline] (10.987 s) : 0, 10987179
Agent [candidate] (1.058 s) : 0, 1058267
Total [candidate] (11.013 s) : 0, 11013164
section appsec
Agent [baseline] (1.245 s) : 0, 1244883
Total [baseline] (11.102 s) : 0, 11102091
Agent [candidate] (1.244 s) : 0, 1244320
Total [candidate] (11.167 s) : 0, 11166856
section iast
Agent [baseline] (1.223 s) : 0, 1223368
Total [baseline] (11.315 s) : 0, 11314806
Agent [candidate] (1.222 s) : 0, 1221730
Total [candidate] (11.303 s) : 0, 11302602
section profiling
Agent [baseline] (1.184 s) : 0, 1184004
Total [baseline] (11.034 s) : 0, 11033715
Agent [candidate] (1.185 s) : 0, 1184607
Total [candidate] (11.047 s) : 0, 11047148
gantt
title petclinic - break down per module: candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.218 ms) : 0, 1218
crashtracking [candidate] (1.189 ms) : 0, 1189
BytebuddyAgent [baseline] (632.562 ms) : 0, 632562
BytebuddyAgent [candidate] (635.681 ms) : 0, 635681
AgentMeter [baseline] (29.385 ms) : 0, 29385
AgentMeter [candidate] (29.312 ms) : 0, 29312
GlobalTracer [baseline] (248.304 ms) : 0, 248304
GlobalTracer [candidate] (248.069 ms) : 0, 248069
AppSec [baseline] (31.857 ms) : 0, 31857
AppSec [candidate] (31.952 ms) : 0, 31952
Debugger [baseline] (59.817 ms) : 0, 59817
Debugger [candidate] (59.867 ms) : 0, 59867
Remote Config [baseline] (601.817 µs) : 0, 602
Remote Config [candidate] (592.25 µs) : 0, 592
Telemetry [baseline] (8.065 ms) : 0, 8065
Telemetry [candidate] (8.031 ms) : 0, 8031
Flare Poller [baseline] (8.904 ms) : 0, 8904
Flare Poller [candidate] (7.392 ms) : 0, 7392
section appsec
crashtracking [baseline] (1.212 ms) : 0, 1212
crashtracking [candidate] (1.185 ms) : 0, 1185
BytebuddyAgent [baseline] (660.647 ms) : 0, 660647
BytebuddyAgent [candidate] (660.382 ms) : 0, 660382
AgentMeter [baseline] (12.02 ms) : 0, 12020
AgentMeter [candidate] (12.024 ms) : 0, 12024
GlobalTracer [baseline] (248.37 ms) : 0, 248370
GlobalTracer [candidate] (248.138 ms) : 0, 248138
IAST [baseline] (24.543 ms) : 0, 24543
IAST [candidate] (24.502 ms) : 0, 24502
AppSec [baseline] (183.52 ms) : 0, 183520
AppSec [candidate] (183.595 ms) : 0, 183595
Debugger [baseline] (65.651 ms) : 0, 65651
Debugger [candidate] (64.879 ms) : 0, 64879
Remote Config [baseline] (588.661 µs) : 0, 589
Remote Config [candidate] (592.044 µs) : 0, 592
Telemetry [baseline] (8.526 ms) : 0, 8526
Telemetry [candidate] (9.338 ms) : 0, 9338
Flare Poller [baseline] (3.549 ms) : 0, 3549
Flare Poller [candidate] (3.535 ms) : 0, 3535
section iast
crashtracking [baseline] (1.219 ms) : 0, 1219
crashtracking [candidate] (1.188 ms) : 0, 1188
BytebuddyAgent [baseline] (801.38 ms) : 0, 801380
BytebuddyAgent [candidate] (800.375 ms) : 0, 800375
AgentMeter [baseline] (11.419 ms) : 0, 11419
AgentMeter [candidate] (11.356 ms) : 0, 11356
GlobalTracer [baseline] (238.793 ms) : 0, 238793
GlobalTracer [candidate] (237.88 ms) : 0, 237880
IAST [baseline] (25.774 ms) : 0, 25774
IAST [candidate] (25.835 ms) : 0, 25835
AppSec [baseline] (29.42 ms) : 0, 29420
AppSec [candidate] (32.415 ms) : 0, 32415
Debugger [baseline] (63.144 ms) : 0, 63144
Debugger [candidate] (57.343 ms) : 0, 57343
Remote Config [baseline] (535.37 µs) : 0, 535
Remote Config [candidate] (521.928 µs) : 0, 522
Telemetry [baseline] (12.003 ms) : 0, 12003
Telemetry [candidate] (14.835 ms) : 0, 14835
Flare Poller [baseline] (3.572 ms) : 0, 3572
Flare Poller [candidate] (3.83 ms) : 0, 3830
section profiling
crashtracking [baseline] (1.18 ms) : 0, 1180
crashtracking [candidate] (1.181 ms) : 0, 1181
BytebuddyAgent [baseline] (691.091 ms) : 0, 691091
BytebuddyAgent [candidate] (693.031 ms) : 0, 693031
AgentMeter [baseline] (9.164 ms) : 0, 9164
AgentMeter [candidate] (9.141 ms) : 0, 9141
GlobalTracer [baseline] (206.783 ms) : 0, 206783
GlobalTracer [candidate] (206.769 ms) : 0, 206769
AppSec [baseline] (32.731 ms) : 0, 32731
AppSec [candidate] (32.492 ms) : 0, 32492
Debugger [baseline] (65.858 ms) : 0, 65858
Debugger [candidate] (65.439 ms) : 0, 65439
Remote Config [baseline] (580.378 µs) : 0, 580
Remote Config [candidate] (564.277 µs) : 0, 564
Telemetry [baseline] (7.955 ms) : 0, 7955
Telemetry [candidate] (7.931 ms) : 0, 7931
Flare Poller [baseline] (3.605 ms) : 0, 3605
Flare Poller [candidate] (3.602 ms) : 0, 3602
ProfilingAgent [baseline] (94.015 ms) : 0, 94015
ProfilingAgent [candidate] (93.466 ms) : 0, 93466
Profiling [baseline] (94.589 ms) : 0, 94589
Profiling [candidate] (94.026 ms) : 0, 94026
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 1 performance regressions! Performance is the same for 16 metrics, 17 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section baseline
no_agent (18.067 ms) : 17888, 18246
. : milestone, 18067,
appsec (18.987 ms) : 18793, 19182
. : milestone, 18987,
code_origins (17.892 ms) : 17719, 18066
. : milestone, 17892,
iast (18.014 ms) : 17838, 18189
. : milestone, 18014,
profiling (18.949 ms) : 18758, 19141
. : milestone, 18949,
tracing (17.849 ms) : 17674, 18025
. : milestone, 17849,
section candidate
no_agent (19.625 ms) : 19422, 19828
. : milestone, 19625,
appsec (18.605 ms) : 18420, 18791
. : milestone, 18605,
code_origins (17.779 ms) : 17603, 17956
. : milestone, 17779,
iast (17.801 ms) : 17624, 17977
. : milestone, 17801,
profiling (19.092 ms) : 18896, 19287
. : milestone, 19092,
tracing (18.001 ms) : 17822, 18180
. : milestone, 18001,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section baseline
no_agent (1.279 ms) : 1266, 1292
. : milestone, 1279,
iast (3.23 ms) : 3192, 3267
. : milestone, 3230,
iast_FULL (5.939 ms) : 5880, 5999
. : milestone, 5939,
iast_GLOBAL (3.838 ms) : 3776, 3900
. : milestone, 3838,
profiling (2.213 ms) : 2194, 2233
. : milestone, 2213,
tracing (1.877 ms) : 1861, 1894
. : milestone, 1877,
section candidate
no_agent (1.246 ms) : 1234, 1258
. : milestone, 1246,
iast (3.292 ms) : 3244, 3341
. : milestone, 3292,
iast_FULL (5.929 ms) : 5870, 5988
. : milestone, 5929,
iast_GLOBAL (3.623 ms) : 3560, 3686
. : milestone, 3623,
profiling (2.389 ms) : 2363, 2415
. : milestone, 2389,
tracing (1.855 ms) : 1841, 1870
. : milestone, 1855,
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 biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section baseline
no_agent (14.797 s) : 14797000, 14797000
. : milestone, 14797000,
appsec (14.858 s) : 14858000, 14858000
. : milestone, 14858000,
iast (18.952 s) : 18952000, 18952000
. : milestone, 18952000,
iast_GLOBAL (18.086 s) : 18086000, 18086000
. : milestone, 18086000,
profiling (15.186 s) : 15186000, 15186000
. : milestone, 15186000,
tracing (14.932 s) : 14932000, 14932000
. : milestone, 14932000,
section candidate
no_agent (15.326 s) : 15326000, 15326000
. : milestone, 15326000,
appsec (15.034 s) : 15034000, 15034000
. : milestone, 15034000,
iast (18.276 s) : 18276000, 18276000
. : milestone, 18276000,
iast_GLOBAL (17.911 s) : 17911000, 17911000
. : milestone, 17911000,
profiling (14.998 s) : 14998000, 14998000
. : milestone, 14998000,
tracing (14.894 s) : 14894000, 14894000
. : milestone, 14894000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~d8a92f8c6d, baseline=1.61.0-SNAPSHOT~4fa94c4f4f
dateFormat X
axisFormat %s
section baseline
no_agent (1.485 ms) : 1473, 1496
. : milestone, 1485,
appsec (3.845 ms) : 3623, 4067
. : milestone, 3845,
iast (2.273 ms) : 2204, 2342
. : milestone, 2273,
iast_GLOBAL (2.32 ms) : 2250, 2390
. : milestone, 2320,
profiling (2.098 ms) : 2043, 2153
. : milestone, 2098,
tracing (2.087 ms) : 2033, 2140
. : milestone, 2087,
section candidate
no_agent (1.495 ms) : 1484, 1507
. : milestone, 1495,
appsec (3.82 ms) : 3599, 4041
. : milestone, 3820,
iast (2.268 ms) : 2199, 2336
. : milestone, 2268,
iast_GLOBAL (2.313 ms) : 2244, 2382
. : milestone, 2313,
profiling (2.092 ms) : 2037, 2146
. : milestone, 2092,
tracing (2.089 ms) : 2035, 2142
. : milestone, 2089,
|
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