Skip to content

Commit 045ea63

Browse files
committed
fix(appsec): use per-part charset for files_content in commons-fileupload
1 parent e2d98c0 commit 045ea63

2 files changed

Lines changed: 92 additions & 2 deletions

File tree

dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
import java.io.IOException;
44
import java.io.InputStream;
5+
import java.nio.charset.Charset;
56
import java.nio.charset.StandardCharsets;
67
import java.util.ArrayList;
78
import java.util.List;
9+
import java.util.Locale;
810
import org.apache.commons.fileupload.FileItem;
911

1012
/** Reads uploaded file content for WAF inspection. */
@@ -35,11 +37,33 @@ public static String readContent(FileItem fileItem) {
3537
&& (n = is.read(buf, total, MAX_CONTENT_BYTES - total)) != -1) {
3638
total += n;
3739
}
38-
return new String(buf, 0, total, StandardCharsets.ISO_8859_1);
40+
return decodeBytes(buf, total, fileItem.getContentType());
3941
} catch (IOException ignored) {
4042
return "";
4143
}
4244
}
4345

46+
static String decodeBytes(byte[] buf, int length, String contentType) {
47+
Charset charset = extractCharset(contentType);
48+
if (charset == null) charset = StandardCharsets.UTF_8;
49+
try {
50+
return new String(buf, 0, length, charset);
51+
} catch (Exception e) {
52+
return new String(buf, 0, length, StandardCharsets.ISO_8859_1);
53+
}
54+
}
55+
56+
static Charset extractCharset(String contentType) {
57+
if (contentType == null) return null;
58+
int idx = contentType.toLowerCase(Locale.ROOT).indexOf("charset=");
59+
if (idx < 0) return null;
60+
String name = contentType.substring(idx + 8).split("[;, ]")[0].trim();
61+
try {
62+
return Charset.forName(name);
63+
} catch (IllegalArgumentException e) {
64+
return null;
65+
}
66+
}
67+
4468
private FileItemContentReader() {}
4569
}

dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/FileItemContentReaderTest.groovy

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,33 @@ class FileItemContentReaderTest extends Specification {
3939
FileItemContentReader.readContent(item) == ''
4040
}
4141

42+
void 'readContent decodes UTF-8 when Content-Type specifies charset=UTF-8'() {
43+
given:
44+
def text = 'héllo wörld'
45+
def item = fileItemFromBytes(text.getBytes('UTF-8'), 'file.txt', 'text/plain; charset=UTF-8')
46+
47+
expect:
48+
FileItemContentReader.readContent(item) == text
49+
}
50+
51+
void 'readContent falls back to UTF-8 when Content-Type has no charset'() {
52+
given:
53+
def text = 'hello world'
54+
def item = fileItemFromBytes(text.getBytes('UTF-8'), 'file.txt', 'text/plain')
55+
56+
expect:
57+
FileItemContentReader.readContent(item) == text
58+
}
59+
60+
void 'readContent falls back to UTF-8 when Content-Type is null'() {
61+
given:
62+
def text = 'hello world'
63+
def item = fileItemFromBytes(text.getBytes('UTF-8'), 'file.txt', null)
64+
65+
expect:
66+
FileItemContentReader.readContent(item) == text
67+
}
68+
4269
void 'readContents returns content for each non-form file with a name'() {
4370
given:
4471
def items = [fileItem('content-a', 'file-a.txt'), fileItem('content-b', 'file-b.txt'),]
@@ -96,15 +123,54 @@ class FileItemContentReaderTest extends Specification {
96123
FileItemContentReader.readContents([]) == []
97124
}
98125

126+
void 'extractCharset returns null for null contentType'() {
127+
expect:
128+
FileItemContentReader.extractCharset(null) == null
129+
}
130+
131+
void 'extractCharset returns null for contentType without charset'() {
132+
expect:
133+
FileItemContentReader.extractCharset('text/plain') == null
134+
FileItemContentReader.extractCharset('image/jpeg') == null
135+
FileItemContentReader.extractCharset('application/octet-stream') == null
136+
}
137+
138+
void 'extractCharset returns null for invalid charset name'() {
139+
expect:
140+
FileItemContentReader.extractCharset('text/plain; charset=NOTACHARSET') == null
141+
}
142+
143+
void 'extractCharset extracts charset case-insensitively'() {
144+
expect:
145+
FileItemContentReader.extractCharset('text/plain; CHARSET=UTF-8').name() == 'UTF-8'
146+
FileItemContentReader.extractCharset('text/plain; Charset=UTF-8').name() == 'UTF-8'
147+
FileItemContentReader.extractCharset('text/plain; charset=utf-8').name() == 'UTF-8'
148+
}
149+
150+
void 'extractCharset extracts charset from standard Content-Type'() {
151+
expect:
152+
FileItemContentReader.extractCharset('text/plain; charset=UTF-8').name() == 'UTF-8'
153+
FileItemContentReader.extractCharset('text/xml; charset=ISO-8859-1').name() == 'ISO-8859-1'
154+
}
155+
99156
private FileItem fileItem(String content) {
100157
fileItem(content, 'file.txt')
101158
}
102159

103160
private FileItem fileItem(String content, String name) {
161+
fileItem(content, name, null)
162+
}
163+
164+
private FileItem fileItem(String content, String name, String contentType) {
165+
fileItemFromBytes((content ?: '').getBytes('ISO-8859-1'), name, contentType)
166+
}
167+
168+
private FileItem fileItemFromBytes(byte[] bytes, String name, String contentType) {
104169
FileItem item = Stub(FileItem)
105170
item.isFormField() >> false
106171
item.getName() >> name
107-
item.getInputStream() >> new ByteArrayInputStream((content ?: '').getBytes('ISO-8859-1'))
172+
item.getContentType() >> contentType
173+
item.getInputStream() >> new ByteArrayInputStream(bytes)
108174
return item
109175
}
110176
}

0 commit comments

Comments
 (0)