Skip to content

Commit 1799f8b

Browse files
committed
fix(appsec): distinguish empty filename from absent filename in PartHelper
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.
1 parent bece336 commit 1799f8b

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

  • dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ static String filenameFromPart(Part part) {
122122
value = cd.substring(valueStart, i).trim();
123123
}
124124
if (isFilename) {
125-
return value.isEmpty() ? null : value;
125+
// Return empty string (not null) so callers can distinguish "filename present but empty"
126+
// from "no filename parameter". extractFormFields() uses != null to skip file parts,
127+
// so empty string correctly prevents buffering a file-upload body with filename="".
128+
return value;
126129
}
127130
}
128131
return null;

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/PartHelperTest.groovy

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,36 @@ class PartHelperTest extends Specification {
9191
PartHelper.filenameFromPart(p) == 'photo.jpg'
9292
}
9393

94-
def "filenameFromPart returns null for empty quoted filename"() {
94+
def "filenameFromPart returns empty string for empty quoted filename"() {
9595
given:
9696
Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename=""' }
9797

9898
expect:
99-
PartHelper.filenameFromPart(p) == null
99+
PartHelper.filenameFromPart(p) == ''
100100
}
101101

102-
def "filenameFromPart returns null for empty unquoted filename"() {
102+
def "filenameFromPart returns empty string for empty unquoted filename"() {
103103
given:
104104
Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename=' }
105105

106106
expect:
107-
PartHelper.filenameFromPart(p) == null
107+
PartHelper.filenameFromPart(p) == ''
108+
}
109+
110+
def "extractFormFields skips part with empty filename"() {
111+
given:
112+
def parts = [emptyFilenamePart('field')]
113+
114+
expect:
115+
PartHelper.extractFormFields(parts) == [:]
116+
}
117+
118+
def "extractFilenames skips empty filename"() {
119+
given:
120+
def parts = [emptyFilenamePart('field')]
121+
122+
expect:
123+
PartHelper.extractFilenames(parts) == []
108124
}
109125

110126
def "filenameFromPart preserves semicolons inside a quoted filename"() {
@@ -192,4 +208,11 @@ class PartHelperTest extends Specification {
192208
p.getHeader('Content-Disposition') >> "form-data; name=\"file\"; filename=\"${filename}\""
193209
return p
194210
}
211+
212+
/** Creates a stub Part that has filename="" — a file input submitted with no file chosen. */
213+
private Part emptyFilenamePart(String name) {
214+
Part p = Stub(Part)
215+
p.getHeader('Content-Disposition') >> "form-data; name=\"${name}\"; filename=\"\""
216+
return p
217+
}
195218
}

0 commit comments

Comments
 (0)