Add server.request.body.filenames support for Jetty#10988
Open
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 63 metrics, 8 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.058 s) : 0, 1057837
Total [baseline] (11.154 s) : 0, 11153535
Agent [candidate] (1.057 s) : 0, 1057473
Total [candidate] (11.088 s) : 0, 11088136
section appsec
Agent [baseline] (1.257 s) : 0, 1257407
Total [baseline] (11.119 s) : 0, 11119147
Agent [candidate] (1.259 s) : 0, 1258511
Total [candidate] (11.082 s) : 0, 11081537
section iast
Agent [baseline] (1.233 s) : 0, 1232906
Total [baseline] (11.322 s) : 0, 11322317
Agent [candidate] (1.228 s) : 0, 1228438
Total [candidate] (11.322 s) : 0, 11321693
section profiling
Agent [baseline] (1.185 s) : 0, 1184585
Total [baseline] (11.057 s) : 0, 11056842
Agent [candidate] (1.188 s) : 0, 1187531
Total [candidate] (11.073 s) : 0, 11073499
gantt
title petclinic - break down per module: candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.234 ms) : 0, 1234
crashtracking [candidate] (1.222 ms) : 0, 1222
BytebuddyAgent [baseline] (633.432 ms) : 0, 633432
BytebuddyAgent [candidate] (632.044 ms) : 0, 632044
AgentMeter [baseline] (29.511 ms) : 0, 29511
AgentMeter [candidate] (29.722 ms) : 0, 29722
GlobalTracer [baseline] (249.049 ms) : 0, 249049
GlobalTracer [candidate] (249.599 ms) : 0, 249599
AppSec [baseline] (32.422 ms) : 0, 32422
AppSec [candidate] (32.544 ms) : 0, 32544
Debugger [baseline] (59.784 ms) : 0, 59784
Debugger [candidate] (60.17 ms) : 0, 60170
Remote Config [baseline] (588.946 µs) : 0, 589
Remote Config [candidate] (602.295 µs) : 0, 602
Telemetry [baseline] (8.024 ms) : 0, 8024
Telemetry [candidate] (8.068 ms) : 0, 8068
Flare Poller [baseline] (7.576 ms) : 0, 7576
Flare Poller [candidate] (7.388 ms) : 0, 7388
section appsec
crashtracking [baseline] (1.239 ms) : 0, 1239
crashtracking [candidate] (1.221 ms) : 0, 1221
BytebuddyAgent [baseline] (667.328 ms) : 0, 667328
BytebuddyAgent [candidate] (667.236 ms) : 0, 667236
AgentMeter [baseline] (12.258 ms) : 0, 12258
AgentMeter [candidate] (12.292 ms) : 0, 12292
GlobalTracer [baseline] (250.534 ms) : 0, 250534
GlobalTracer [candidate] (250.662 ms) : 0, 250662
IAST [baseline] (24.654 ms) : 0, 24654
IAST [candidate] (24.771 ms) : 0, 24771
AppSec [baseline] (185.935 ms) : 0, 185935
AppSec [candidate] (186.416 ms) : 0, 186416
Debugger [baseline] (66.348 ms) : 0, 66348
Debugger [candidate] (66.619 ms) : 0, 66619
Remote Config [baseline] (607.161 µs) : 0, 607
Remote Config [candidate] (623.887 µs) : 0, 624
Telemetry [baseline] (8.493 ms) : 0, 8493
Telemetry [candidate] (8.568 ms) : 0, 8568
Flare Poller [baseline] (3.565 ms) : 0, 3565
Flare Poller [candidate] (3.612 ms) : 0, 3612
section iast
crashtracking [baseline] (1.239 ms) : 0, 1239
crashtracking [candidate] (1.227 ms) : 0, 1227
BytebuddyAgent [baseline] (807.295 ms) : 0, 807295
BytebuddyAgent [candidate] (803.643 ms) : 0, 803643
AgentMeter [baseline] (11.682 ms) : 0, 11682
AgentMeter [candidate] (11.577 ms) : 0, 11577
GlobalTracer [baseline] (241.178 ms) : 0, 241178
GlobalTracer [candidate] (239.706 ms) : 0, 239706
IAST [baseline] (26.655 ms) : 0, 26655
IAST [candidate] (25.975 ms) : 0, 25975
AppSec [baseline] (29.468 ms) : 0, 29468
AppSec [candidate] (33.98 ms) : 0, 33980
Debugger [baseline] (65.795 ms) : 0, 65795
Debugger [candidate] (62.85 ms) : 0, 62850
Remote Config [baseline] (529.711 µs) : 0, 530
Remote Config [candidate] (537.904 µs) : 0, 538
Telemetry [baseline] (9.319 ms) : 0, 9319
Telemetry [candidate] (9.233 ms) : 0, 9233
Flare Poller [baseline] (3.567 ms) : 0, 3567
Flare Poller [candidate] (3.581 ms) : 0, 3581
section profiling
crashtracking [baseline] (1.172 ms) : 0, 1172
crashtracking [candidate] (1.182 ms) : 0, 1182
BytebuddyAgent [baseline] (691.127 ms) : 0, 691127
BytebuddyAgent [candidate] (693.682 ms) : 0, 693682
AgentMeter [baseline] (9.239 ms) : 0, 9239
AgentMeter [candidate] (8.984 ms) : 0, 8984
GlobalTracer [baseline] (207.179 ms) : 0, 207179
GlobalTracer [candidate] (207.428 ms) : 0, 207428
AppSec [baseline] (32.789 ms) : 0, 32789
AppSec [candidate] (32.849 ms) : 0, 32849
Debugger [baseline] (65.924 ms) : 0, 65924
Debugger [candidate] (65.944 ms) : 0, 65944
Remote Config [baseline] (587.481 µs) : 0, 587
Remote Config [candidate] (574.259 µs) : 0, 574
Telemetry [baseline] (7.838 ms) : 0, 7838
Telemetry [candidate] (7.827 ms) : 0, 7827
Flare Poller [baseline] (3.589 ms) : 0, 3589
Flare Poller [candidate] (3.555 ms) : 0, 3555
ProfilingAgent [baseline] (93.863 ms) : 0, 93863
ProfilingAgent [candidate] (94.108 ms) : 0, 94108
Profiling [baseline] (94.425 ms) : 0, 94425
Profiling [candidate] (94.671 ms) : 0, 94671
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.068 s) : 0, 1068373
Total [baseline] (8.906 s) : 0, 8906185
Agent [candidate] (1.058 s) : 0, 1058219
Total [candidate] (8.849 s) : 0, 8848892
section iast
Agent [baseline] (1.233 s) : 0, 1232890
Total [baseline] (9.597 s) : 0, 9596656
Agent [candidate] (1.236 s) : 0, 1235865
Total [candidate] (9.568 s) : 0, 9567920
gantt
title insecure-bank - break down per module: candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.259 ms) : 0, 1259
crashtracking [candidate] (1.242 ms) : 0, 1242
BytebuddyAgent [baseline] (638.984 ms) : 0, 638984
BytebuddyAgent [candidate] (634.688 ms) : 0, 634688
AgentMeter [baseline] (29.807 ms) : 0, 29807
AgentMeter [candidate] (29.458 ms) : 0, 29458
GlobalTracer [baseline] (250.549 ms) : 0, 250549
GlobalTracer [candidate] (249.127 ms) : 0, 249127
AppSec [baseline] (32.704 ms) : 0, 32704
AppSec [candidate] (32.265 ms) : 0, 32265
Debugger [baseline] (59.922 ms) : 0, 59922
Debugger [candidate] (59.334 ms) : 0, 59334
Remote Config [baseline] (596.205 µs) : 0, 596
Remote Config [candidate] (587.947 µs) : 0, 588
Telemetry [baseline] (8.1 ms) : 0, 8100
Telemetry [candidate] (9.435 ms) : 0, 9435
Flare Poller [baseline] (9.915 ms) : 0, 9915
Flare Poller [candidate] (5.967 ms) : 0, 5967
section iast
crashtracking [baseline] (1.245 ms) : 0, 1245
crashtracking [candidate] (1.235 ms) : 0, 1235
BytebuddyAgent [baseline] (807.13 ms) : 0, 807130
BytebuddyAgent [candidate] (809.695 ms) : 0, 809695
AgentMeter [baseline] (11.62 ms) : 0, 11620
AgentMeter [candidate] (11.686 ms) : 0, 11686
GlobalTracer [baseline] (240.471 ms) : 0, 240471
GlobalTracer [candidate] (240.639 ms) : 0, 240639
IAST [baseline] (25.955 ms) : 0, 25955
IAST [candidate] (26.037 ms) : 0, 26037
AppSec [baseline] (29.216 ms) : 0, 29216
AppSec [candidate] (31.391 ms) : 0, 31391
Debugger [baseline] (67.436 ms) : 0, 67436
Debugger [candidate] (65.31 ms) : 0, 65310
Remote Config [baseline] (539.71 µs) : 0, 540
Remote Config [candidate] (558.64 µs) : 0, 559
Telemetry [baseline] (9.41 ms) : 0, 9410
Telemetry [candidate] (9.309 ms) : 0, 9309
Flare Poller [baseline] (3.631 ms) : 0, 3631
Flare Poller [candidate] (3.571 ms) : 0, 3571
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 2 performance regressions! Performance is the same for 16 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section baseline
no_agent (19.215 ms) : 19022, 19408
. : milestone, 19215,
appsec (18.615 ms) : 18432, 18798
. : milestone, 18615,
code_origins (17.731 ms) : 17553, 17909
. : milestone, 17731,
iast (18.802 ms) : 18613, 18990
. : milestone, 18802,
profiling (18.157 ms) : 17976, 18338
. : milestone, 18157,
tracing (17.929 ms) : 17749, 18108
. : milestone, 17929,
section candidate
no_agent (18.118 ms) : 17935, 18301
. : milestone, 18118,
appsec (19.005 ms) : 18815, 19196
. : milestone, 19005,
code_origins (17.832 ms) : 17655, 18008
. : milestone, 17832,
iast (17.817 ms) : 17642, 17992
. : milestone, 17817,
profiling (18.304 ms) : 18124, 18484
. : milestone, 18304,
tracing (17.988 ms) : 17809, 18167
. : milestone, 17988,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section baseline
no_agent (1.232 ms) : 1221, 1243
. : milestone, 1232,
iast (3.393 ms) : 3344, 3443
. : milestone, 3393,
iast_FULL (5.812 ms) : 5754, 5870
. : milestone, 5812,
iast_GLOBAL (3.54 ms) : 3487, 3592
. : milestone, 3540,
profiling (2.038 ms) : 2020, 2056
. : milestone, 2038,
tracing (1.907 ms) : 1891, 1923
. : milestone, 1907,
section candidate
no_agent (1.225 ms) : 1213, 1237
. : milestone, 1225,
iast (3.347 ms) : 3298, 3395
. : milestone, 3347,
iast_FULL (5.92 ms) : 5860, 5980
. : milestone, 5920,
iast_GLOBAL (3.699 ms) : 3639, 3760
. : milestone, 3699,
profiling (2.266 ms) : 2243, 2289
. : milestone, 2266,
tracing (1.957 ms) : 1940, 1973
. : milestone, 1957,
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.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section baseline
no_agent (1.491 ms) : 1480, 1503
. : milestone, 1491,
appsec (3.777 ms) : 3559, 3996
. : milestone, 3777,
iast (2.278 ms) : 2209, 2347
. : milestone, 2278,
iast_GLOBAL (2.312 ms) : 2242, 2381
. : milestone, 2312,
profiling (2.097 ms) : 2042, 2151
. : milestone, 2097,
tracing (2.082 ms) : 2028, 2135
. : milestone, 2082,
section candidate
no_agent (1.495 ms) : 1484, 1507
. : milestone, 1495,
appsec (3.773 ms) : 3556, 3991
. : milestone, 3773,
iast (2.28 ms) : 2211, 2350
. : milestone, 2280,
iast_GLOBAL (2.321 ms) : 2251, 2391
. : milestone, 2321,
profiling (2.104 ms) : 2049, 2159
. : milestone, 2104,
tracing (2.095 ms) : 2041, 2149
. : milestone, 2095,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~1799f8b303, baseline=1.62.0-SNAPSHOT~081af53226
dateFormat X
axisFormat %s
section baseline
no_agent (15.585 s) : 15585000, 15585000
. : milestone, 15585000,
appsec (14.419 s) : 14419000, 14419000
. : milestone, 14419000,
iast (18.22 s) : 18220000, 18220000
. : milestone, 18220000,
iast_GLOBAL (18.249 s) : 18249000, 18249000
. : milestone, 18249000,
profiling (15.848 s) : 15848000, 15848000
. : milestone, 15848000,
tracing (15.053 s) : 15053000, 15053000
. : milestone, 15053000,
section candidate
no_agent (15.303 s) : 15303000, 15303000
. : milestone, 15303000,
appsec (14.651 s) : 14651000, 14651000
. : milestone, 14651000,
iast (18.299 s) : 18299000, 18299000
. : milestone, 18299000,
iast_GLOBAL (18.209 s) : 18209000, 18209000
. : milestone, 18209000,
profiling (14.797 s) : 14797000, 14797000
. : milestone, 14797000,
tracing (14.839 s) : 14839000, 14839000
. : milestone, 14839000,
|
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".
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
- Add BODY_MULTIPART_REPEATED case to TestServlet3 (javax) so Jetty 9.x/10.x test modules can exercise the repeated getParts() scenario - Enable testBodyFilenamesCalledOnce() for Jetty 9.0, 9.0.4, 9.3, 9.4.21, and 10.0
…vice path - New BODY_MULTIPART_COMBINED endpoint: calls getParameterMap() first (triggers GetFilenamesFromMultiPartAdvice via extractContentParameters -> getParts(MultiMap)), then getParts() explicitly (GetFilenamesAdvice must not double-fire since _contentParameters is already set) - New test 'file upload filenames called once via parameter map' verifies the callback fires exactly once across both advice paths - Enabled in Jetty 9.0, 9.0.4, 9.3, 9.4.21, 10.0 and 11.0
…eplaces _contentParameters as the getParts() cache In Jetty 9.3, getParts(MultiMap) sets _contentParameters, so the map==null guard prevents re-firing on repeated getParts() calls. In Jetty 9.4+, getParts() delegates to getParts(null) and caches the result in _multiParts instead, leaving _contentParameters null on every call. Add _multiParts==null as an additional guard (optional=true handles Jetty 9.3 where the field does not exist).
…ith exit-advice Replace the brittle GetPartsMethodVisitor ASM bytecode injection (intercepting MultiMap.add() calls inside getParts()) with a clean ByteBuddy exit-advice approach: - Remove ParameterCollector, GetPartsAdvice, GetPartsVisitorWrapper, RequestClassVisitor, and GetPartsMethodVisitor; drop HasTypeAdvice from the instrumentation module. - Add PartHelper with extractFormFields() (reads InputStream per part) and extractFilenames() (parses Content-Disposition manually, since Part.getSubmittedFileName() is Servlet 3.1+ and not available in Jetty 8.x). - Add GetFilenamesAdvice (@RequiresRequestContext / @ActiveRequestContext) on getParts():Collection that fires requestBodyProcessed() with form fields and requestFilesFilenames() with upload filenames. Uses CallDepthThreadLocalMap to guard against reentrant calls. - Jetty8LatestDepForkedTest: set testBodyFilenamesCalledOnce() and testBodyFilenamesCalledOnceCombined() to false because Jetty 8.x has no _multiParts / _contentParameters field guards to suppress duplicate firings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e43466fc5b
ℹ️ 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".
…utStream - jetty-appsec-9.4: extend muzzle to also cover Jetty 10.0.0–10.0.9, which were previously a gap. In those versions _multiParts is typed MultiPartFormInputStream (not MultiParts); the primary Reference spec matches 9.4.10+ and 10.0.10+, and an OrReference alternative matches 10.0.0–10.0.9. The GetFilenamesAdvice already uses typing=DYNAMIC so no advice changes are needed. - jetty-appsec-8.1.3 PartHelper: wrap part.getInputStream() in try-with-resources to avoid leaking file descriptors on file-backed multipart form fields.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f2e2b3998
ℹ️ 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".
Splitting the header on ';' naively truncated filenames that contain semicolons inside a quoted value, e.g. filename="shell;evil.php" would produce "shell" instead of the full name. Replace the split() loop with a quote-aware state-machine parser that skips semicolons inside quoted strings and handles backslash-escaped characters. Add test cases for semicolons in filenames, escaped quotes, and filename appearing before other parameters.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ce4bf1084
ℹ️ 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".
…c-11.0 Replace the JAKARTA_PART_REFERENCE classpath check with a _dispatcherType field descriptor check on Request.class bytecode, mirroring the approach already used by jetty-appsec-9.4. The classpath check passes on any Jetty 9.4/10 app that has jakarta.servlet-api as a dependency, causing double-instrumentation of extractContentParameters. The bytecode check is authoritative: in Jetty 11+ Request.class carries _dispatcherType as Ljakarta/servlet/DispatcherType;, while 9.4/10 carry the javax descriptor.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74948a75d6
ℹ️ 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".
…uploads In Jetty 8.x, getPart(String name) calls _multiPartInputStream.getPart(String) directly without delegating to getParts(). Applications that retrieve only one file via getPart() without ever calling getParts() would have their filename event missed. Add GetPartAdvice to cover this path. The charset fix (AI comment 2) was investigated and is not applicable: HTML5 form submissions always use UTF-8 and browsers never include charset= on individual part Content-Type headers, so the existing hardcoded UTF-8 is correct.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1508fef49c
ℹ️ 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".
…ilename tests in async suite 1. Add _multiPartInputStream == null guard to GetFilenamesAdvice.before() so that repeated getParts() calls on the same request (which Jetty caches) do not re-fire requestFilesFilenames/requestBodyProcessed WAF callbacks. The field is null before the first multipart parse and non-null on all subsequent cached calls, matching the pattern used in the 9.4/11.0 advice (_multiParts guard). 2. JettyAsyncHandlerTest already disabled testBodyFilenames() but neglected to disable testBodyFilenamesCalledOnce() and testBodyFilenamesCalledOnceCombined(), which are now enabled in the Jetty11Test parent. Override both to false in the async handler suite to prevent spurious test failures.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bece33644c
ℹ️ 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".
…elper filenameFromPart() was returning null for both 'no filename parameter' and 'filename=""', causing extractFormFields() to buffer the full body of file inputs submitted with no file chosen (filename=""). An empty <input type=file> is still a file part, not a form field. Return "" instead of null so that callers using != null correctly skip those parts without reading their content. Update tests to assert "" for empty-filename cases and add regression tests for extractFormFields/extractFilenames with empty-filename parts. Note: the second AI comment about getPart(String) double-firing was not implemented. The bytecode shows the internal call is to MultiPartInputStream.getParts() (not Request.getParts()), so GetFilenamesAdvice (which instruments Request.getParts()) is never triggered during a getPart() call. There is no double-firing.
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
Instruments Jetty's multipart request handling to fire the
requestFilesFilenamesAppSec gateway event, enabling WAF rules that act on uploaded file names.Jetty parses multipart bodies through two code paths depending on how the application accesses the request:
getParameterMap()→extractContentParameters()→ internalgetParts(MultiMap)getParts()/getPart(String)call from user codeBoth paths are instrumented where they exist. Guards on internal state fields (
_contentParameters,_multiParts,_multiPartInputStream) prevent double-firing when the same method is called more than once per request.jetty-appsec-7.0getParts()does not exist in Jetty 7.xjetty-appsec-8.1.3[8.1.3, 9.2.0.RC0)GetPartsMethodVisitor) with clean exit advice ongetParts()andgetPart(String). In Jetty 8.x,getPart(String)calls_multiPartInputStream.getPart()directly without going throughgetParts(), so it requires its own advice to catch single-part uploads. AddedPartHelperto parseContent-Dispositionheaders manually —Part.getSubmittedFileName()is Servlet 3.1+ and unavailable in Jetty 8.x (Servlet 3.0). The_multiPartInputStream == nullguard inGetFilenamesAdviceprevents WAF callbacks from re-firing on repeatedgetParts()calls (the field is set on first parse and non-null on cached calls).jetty-appsec-9.2[9.2, 9.3)requestBodyProcessed; no filename support needed at this rangejetty-appsec-9.3[9.3, 9.4.10)[9.3, 12)to[9.3, 9.4.10). AddedGetFilenamesAdvice+GetFilenamesFromMultiPartAdvice. Double-fire guard uses_multiPartInputStream(present only in this range; replaced by_multiPartsin 9.4.10). AddedMultipartHelperusingPart.getSubmittedFileName()(Servlet 3.1).jetty-appsec-9.4[9.4.10, 11.0)javax.servletversions from 9.4.10 onward:[9.4.10, 10.0)where_multiPartsisMultiParts, and[10.0.0, 11.0)(Jetty 10.0.0–10.0.9 used_multiParts: MultiPartFormInputStream; 10.0.10+ switched toMultiParts). An OR-muzzle reference covers both field types. The_dispatcherType: Ljavax/servlet/DispatcherType;bytecode field discriminates against Jetty 11+ (which usesLjakarta/servlet/DispatcherType;). Previously handled byjetty-appsec-9.3via reflection.jetty-appsec-11.0[11.0, 12.0)jakarta.servlet.http.Part). Identical structure to 9.4 but compiled againstjakarta. The muzzle discriminator uses_dispatcherType: Ljakarta/servlet/DispatcherType;inRequest.classbytecode — checking forjakarta.servlet.http.Parton the classpath was unreliable when bothjavaxandjakartawere present, causing double instrumentation. Previously handled byjetty-appsec-9.3via reflection.Motivation
Part of APPSEC-61873 —
server.request.body.filenamesimplementation across server frameworks.Additional Notes
Depends on #10973 (merged)
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