Skip to content

Commit a1a267d

Browse files
committed
fix(appsec): skip file content I/O in Tomcat when requestFilesContent callback is unregistered
Add an inspectContent boolean to ParameterCollectorImpl, set from the presence of the requestFilesContent callback in ParsePartsInstrumentation before(). When false, addPart() still collects filenames but skips the getInputStream() read, matching the lazy-init approach already used by the Netty and commons-fileupload integrations.
1 parent c8f1092 commit a1a267d

3 files changed

Lines changed: 40 additions & 16 deletions

File tree

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,15 @@ class ParameterCollectorImpl implements ParameterCollector {
6161
private static final int MAX_CONTENT_BYTES = 4096;
6262
private static final int MAX_FILES_TO_INSPECT = 25;
6363

64+
private final boolean inspectContent;
6465
private Map<String, List<String>> map;
6566
private List<String> filenames;
6667
private List<String> contents;
6768

69+
public ParameterCollectorImpl(boolean inspectContent) {
70+
this.inspectContent = inspectContent;
71+
}
72+
6873
public boolean isEmpty() {
6974
return map == null;
7075
}
@@ -115,11 +120,13 @@ public void addPart(Object part) {
115120
}
116121
filenames.add(filename);
117122
}
118-
if (contents == null) {
119-
contents = new ArrayList<>();
120-
}
121-
if (contents.size() < MAX_FILES_TO_INSPECT) {
122-
contents.add(readContent(part));
123+
if (inspectContent) {
124+
if (contents == null) {
125+
contents = new ArrayList<>();
126+
}
127+
if (contents.size() < MAX_FILES_TO_INSPECT) {
128+
contents.add(readContent(part));
129+
}
123130
}
124131
} catch (Throwable ignored) {
125132
}

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,12 @@ static void before(
8686
RequestContext requestContext = agentSpan.getRequestContext();
8787
if (requestContext != null && requestContext.getData(RequestContextSlot.APPSEC) != null) {
8888
reqCtx = requestContext;
89-
collector = new ParameterCollector.ParameterCollectorImpl();
89+
boolean inspectContent =
90+
AgentTracer.get()
91+
.getCallbackProvider(RequestContextSlot.APPSEC)
92+
.getCallback(EVENTS.requestFilesContent())
93+
!= null;
94+
collector = new ParameterCollector.ParameterCollectorImpl(inspectContent);
9095
return;
9196
}
9297
}

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/groovy/datadog/trace/instrumentation/tomcat7/ParameterCollectorImplTest.groovy

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ class ParameterCollectorImplTest extends Specification {
66

77
void 'getContents returns empty list when no parts added'() {
88
expect:
9-
new ParameterCollector.ParameterCollectorImpl().getContents().isEmpty()
9+
new ParameterCollector.ParameterCollectorImpl(true).getContents().isEmpty()
1010
}
1111

1212
void 'addPart skips content when filename is null'() {
1313
given:
14-
def collector = new ParameterCollector.ParameterCollectorImpl()
14+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
1515

1616
when:
1717
collector.addPart(new TestPart(null, 'some body'))
@@ -23,7 +23,7 @@ class ParameterCollectorImplTest extends Specification {
2323

2424
void 'addPart reads content but skips filename when filename is empty string'() {
2525
given:
26-
def collector = new ParameterCollector.ParameterCollectorImpl()
26+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
2727

2828
when:
2929
collector.addPart(new TestPart('', 'some body'))
@@ -35,7 +35,7 @@ class ParameterCollectorImplTest extends Specification {
3535

3636
void 'addPart reads content for file part with filename'() {
3737
given:
38-
def collector = new ParameterCollector.ParameterCollectorImpl()
38+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
3939

4040
when:
4141
collector.addPart(new TestPart('file.txt', 'hello world'))
@@ -47,7 +47,7 @@ class ParameterCollectorImplTest extends Specification {
4747

4848
void 'addPart reads content for multiple files'() {
4949
given:
50-
def collector = new ParameterCollector.ParameterCollectorImpl()
50+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
5151

5252
when:
5353
collector.addPart(new TestPart('a.txt', 'content-a'))
@@ -60,7 +60,7 @@ class ParameterCollectorImplTest extends Specification {
6060

6161
void 'addPart truncates content at 4096 bytes'() {
6262
given:
63-
def collector = new ParameterCollector.ParameterCollectorImpl()
63+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
6464
def longContent = 'X' * 5000
6565

6666
when:
@@ -72,7 +72,7 @@ class ParameterCollectorImplTest extends Specification {
7272

7373
void 'addPart reads exactly 4096 bytes when content is 4096 bytes'() {
7474
given:
75-
def collector = new ParameterCollector.ParameterCollectorImpl()
75+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
7676
def content = 'Y' * 4096
7777

7878
when:
@@ -84,7 +84,7 @@ class ParameterCollectorImplTest extends Specification {
8484

8585
void 'addPart stops collecting content after 25 files but still collects filenames'() {
8686
given:
87-
def collector = new ParameterCollector.ParameterCollectorImpl()
87+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
8888

8989
when:
9090
(1..26).each { i -> collector.addPart(new TestPart("file${i}.txt", "content${i}")) }
@@ -96,7 +96,7 @@ class ParameterCollectorImplTest extends Specification {
9696

9797
void 'addPart adds empty string when getInputStream throws'() {
9898
given:
99-
def collector = new ParameterCollector.ParameterCollectorImpl()
99+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
100100

101101
when:
102102
collector.addPart(new FailingPart('bad.txt'))
@@ -108,7 +108,7 @@ class ParameterCollectorImplTest extends Specification {
108108

109109
void 'addPart preserves ISO-8859-1 bytes'() {
110110
given:
111-
def collector = new ParameterCollector.ParameterCollectorImpl()
111+
def collector = new ParameterCollector.ParameterCollectorImpl(true)
112112
def bytes = (0..255).collect { (byte) it } as byte[]
113113
def expected = new String(bytes, 'ISO-8859-1')
114114

@@ -119,6 +119,18 @@ class ParameterCollectorImplTest extends Specification {
119119
collector.getContents()[0] == expected
120120
}
121121

122+
void 'addPart skips content when inspectContent is false but still collects filename'() {
123+
given:
124+
def collector = new ParameterCollector.ParameterCollectorImpl(false)
125+
126+
when:
127+
collector.addPart(new TestPart('file.txt', 'hello world'))
128+
129+
then:
130+
collector.getContents().isEmpty()
131+
collector.getFilenames() == ['file.txt']
132+
}
133+
122134
void 'ParameterCollectorNoop getContents returns empty list'() {
123135
expect:
124136
ParameterCollector.ParameterCollectorNoop.INSTANCE.getContents().isEmpty()

0 commit comments

Comments
 (0)