Skip to content

Commit 4f53294

Browse files
committed
Add server.request.body.filenames support for Jersey and RESTEasy
1 parent 8569434 commit 4f53294

14 files changed

Lines changed: 662 additions & 70 deletions

File tree

dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/test/groovy/GrizzlyTest.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ class GrizzlyTest extends HttpServerTest<HttpServer> {
4949
true
5050
}
5151

52+
@Override
53+
boolean testBodyFilenames() {
54+
true
55+
}
56+
5257
@Override
5358
boolean testBodyJson() {
5459
true

dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ class Jersey2JettyTest extends HttpServerTest<JettyServer> {
5454
true
5555
}
5656

57+
@Override
58+
boolean testBodyFilenames() {
59+
true
60+
}
61+
5762
@Override
5863
boolean testBodyJson() {
5964
true

dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/Jersey3JettyTest.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ class Jersey3JettyTest extends HttpServerTest<JettyServer> {
5353
true
5454
}
5555

56+
@Override
57+
boolean testBodyFilenames() {
58+
true
59+
}
60+
5661
@Override
5762
boolean testBodyJson() {
5863
true

dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/build.gradle

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,16 @@ muzzle {
3131

3232
apply from: "$rootDir/gradle/java.gradle"
3333

34+
configurations.configureEach {
35+
resolutionStrategy.deactivateDependencyLocking()
36+
}
37+
3438
dependencies {
3539
compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-common', version: '2.0'
3640
compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '2.0'
3741
compileOnly group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '2.0'
42+
43+
testImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '2.18'
3844
}
3945

4046
// tested in grizzly-http-2.3.20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package datadog.trace.instrumentation.jersey2;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
import java.util.Map;
6+
import javax.ws.rs.core.MediaType;
7+
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
8+
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
9+
import org.glassfish.jersey.message.internal.MediaTypes;
10+
11+
public final class MultiPartHelper {
12+
13+
private MultiPartHelper() {}
14+
15+
public static void collectBodyPart(
16+
FormDataBodyPart bodyPart, Map<String, List<String>> bodyMap, List<String> filenames) {
17+
if (bodyMap != null
18+
&& MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())) {
19+
// BodyPartEntity allows re-reading the part without consuming the stream
20+
bodyMap.computeIfAbsent(bodyPart.getName(), k -> new ArrayList<>()).add(bodyPart.getValue());
21+
}
22+
if (filenames != null) {
23+
String filename = filenameFromBodyPart(bodyPart);
24+
if (filename != null) {
25+
filenames.add(filename);
26+
}
27+
}
28+
}
29+
30+
public static String filenameFromBodyPart(FormDataBodyPart bodyPart) {
31+
FormDataContentDisposition cd = bodyPart.getFormDataContentDisposition();
32+
if (cd == null) return null;
33+
String filename = cd.getFileName();
34+
return (filename == null || filename.isEmpty()) ? null : filename;
35+
}
36+
}

dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartReaderServerSideInstrumentation.java

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.function.BiFunction;
26-
import javax.ws.rs.core.MediaType;
2726
import net.bytebuddy.asm.Advice;
2827
import org.glassfish.jersey.media.multipart.BodyPart;
2928
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
3029
import org.glassfish.jersey.media.multipart.MultiPart;
31-
import org.glassfish.jersey.message.internal.MediaTypes;
3230

3331
@AutoService(InstrumenterModule.class)
3432
public class MultiPartReaderServerSideInstrumentation extends InstrumenterModule.AppSec
@@ -48,6 +46,11 @@ public String instrumentedType() {
4846
return "org.glassfish.jersey.media.multipart.internal.MultiPartReaderServerSide";
4947
}
5048

49+
@Override
50+
public String[] helperClassNames() {
51+
return new String[] {packageName + ".MultiPartHelper"};
52+
}
53+
5154
@Override
5255
public void methodAdvice(MethodTransformer transformer) {
5356
transformer.applyAdvice(
@@ -72,42 +75,47 @@ static void after(
7275
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
7376
BiFunction<RequestContext, Object, Flow<Void>> callback =
7477
cbp.getCallback(EVENTS.requestBodyProcessed());
75-
if (callback == null) {
78+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
79+
cbp.getCallback(EVENTS.requestFilesFilenames());
80+
if (callback == null && filenamesCallback == null) {
7681
return;
7782
}
7883

79-
Map<String, List<String>> map = new HashMap<>();
84+
Map<String, List<String>> map = callback != null ? new HashMap<>() : null;
85+
List<String> filenames = filenamesCallback != null ? new ArrayList<>() : null;
8086
for (BodyPart bodyPart : ret.getBodyParts()) {
8187
if (!(bodyPart instanceof FormDataBodyPart)) {
8288
continue;
8389
}
84-
FormDataBodyPart dataBodyPart = (FormDataBodyPart) bodyPart;
85-
if (!MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) {
86-
continue;
87-
}
88-
// if the type of dataBodyPart.getEntity() is BodyPartEntity, it is safe to read the part
89-
// more than once. So we're not depriving the application of the data by consuming it here
90-
String v = dataBodyPart.getValue();
90+
MultiPartHelper.collectBodyPart((FormDataBodyPart) bodyPart, map, filenames);
91+
}
9192

92-
String name = dataBodyPart.getName();
93-
List<String> values = map.get(name);
94-
if (values == null) {
95-
values = new ArrayList<>();
96-
map.put(name, values);
93+
if (map != null) {
94+
Flow<Void> flow = callback.apply(reqCtx, map);
95+
Flow.Action action = flow.getAction();
96+
if (action instanceof Flow.Action.RequestBlockingAction) {
97+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
98+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
99+
if (blockResponseFunction != null) {
100+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
101+
t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)");
102+
reqCtx.getTraceSegment().effectivelyBlocked();
103+
}
97104
}
98-
99-
values.add(v);
100105
}
101106

102-
Flow<Void> flow = callback.apply(reqCtx, map);
103-
Flow.Action action = flow.getAction();
104-
if (action instanceof Flow.Action.RequestBlockingAction) {
105-
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
106-
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
107-
if (blockResponseFunction != null) {
108-
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
109-
t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)");
110-
reqCtx.getTraceSegment().effectivelyBlocked();
107+
if (filenames != null && !filenames.isEmpty()) {
108+
Flow<Void> filenamesFlow = filenamesCallback.apply(reqCtx, filenames);
109+
Flow.Action filenamesAction = filenamesFlow.getAction();
110+
if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) {
111+
Flow.Action.RequestBlockingAction rba =
112+
(Flow.Action.RequestBlockingAction) filenamesAction;
113+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
114+
if (blockResponseFunction != null) {
115+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
116+
t = new BlockingException("Blocked request (multipart file upload)");
117+
reqCtx.getTraceSegment().effectivelyBlocked();
118+
}
111119
}
112120
}
113121
}
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import datadog.trace.instrumentation.jersey2.MultiPartHelper
2+
import org.glassfish.jersey.media.multipart.FormDataBodyPart
3+
import org.glassfish.jersey.media.multipart.FormDataContentDisposition
4+
import spock.lang.Specification
5+
6+
import javax.ws.rs.core.MediaType
7+
8+
class MultiPartHelperTest extends Specification {
9+
10+
// filenameFromBodyPart
11+
12+
def "returns null when content disposition is null"() {
13+
given:
14+
def bodyPart = Mock(FormDataBodyPart)
15+
bodyPart.getFormDataContentDisposition() >> null
16+
17+
expect:
18+
MultiPartHelper.filenameFromBodyPart(bodyPart) == null
19+
}
20+
21+
def "returns null when filename is null or empty"() {
22+
given:
23+
def cd = Mock(FormDataContentDisposition)
24+
cd.getFileName() >> rawFilename
25+
def bodyPart = Mock(FormDataBodyPart)
26+
bodyPart.getFormDataContentDisposition() >> cd
27+
28+
expect:
29+
MultiPartHelper.filenameFromBodyPart(bodyPart) == null
30+
31+
where:
32+
rawFilename << [null, '']
33+
}
34+
35+
def "extracts filename"() {
36+
given:
37+
def cd = Mock(FormDataContentDisposition)
38+
cd.getFileName() >> filename
39+
def bodyPart = Mock(FormDataBodyPart)
40+
bodyPart.getFormDataContentDisposition() >> cd
41+
42+
expect:
43+
MultiPartHelper.filenameFromBodyPart(bodyPart) == filename
44+
45+
where:
46+
filename << ['report.php', 'upload.txt', 'shell;evil.php', 'file"name.php']
47+
}
48+
49+
// collectBodyPart — body map
50+
51+
def "text/plain part is added to body map"() {
52+
given:
53+
def bodyPart = Mock(FormDataBodyPart)
54+
bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
55+
bodyPart.getName() >> 'field'
56+
bodyPart.getValue() >> 'value'
57+
bodyPart.getFormDataContentDisposition() >> null
58+
def map = [:]
59+
60+
when:
61+
MultiPartHelper.collectBodyPart(bodyPart, map, null)
62+
63+
then:
64+
map == [field: ['value']]
65+
}
66+
67+
def "non-text/plain part is not added to body map"() {
68+
given:
69+
def bodyPart = Mock(FormDataBodyPart)
70+
bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE
71+
bodyPart.getFormDataContentDisposition() >> null
72+
def map = [:]
73+
74+
when:
75+
MultiPartHelper.collectBodyPart(bodyPart, map, null)
76+
77+
then:
78+
map.isEmpty()
79+
}
80+
81+
def "null body map is skipped without error"() {
82+
given:
83+
def bodyPart = Mock(FormDataBodyPart)
84+
bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
85+
bodyPart.getFormDataContentDisposition() >> null
86+
87+
expect:
88+
MultiPartHelper.collectBodyPart(bodyPart, null, null)
89+
}
90+
91+
def "multiple values for same field are accumulated"() {
92+
given:
93+
def bodyPart = Mock(FormDataBodyPart)
94+
bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
95+
bodyPart.getName() >> 'tag'
96+
bodyPart.getValue() >>> ['a', 'b']
97+
bodyPart.getFormDataContentDisposition() >> null
98+
def map = [:]
99+
100+
when:
101+
MultiPartHelper.collectBodyPart(bodyPart, map, null)
102+
MultiPartHelper.collectBodyPart(bodyPart, map, null)
103+
104+
then:
105+
map == [tag: ['a', 'b']]
106+
}
107+
108+
// collectBodyPart — filenames
109+
110+
def "filename is added to list when present"() {
111+
given:
112+
def cd = Mock(FormDataContentDisposition)
113+
cd.getFileName() >> 'report.php'
114+
def bodyPart = Mock(FormDataBodyPart)
115+
bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE
116+
bodyPart.getFormDataContentDisposition() >> cd
117+
def filenames = []
118+
119+
when:
120+
MultiPartHelper.collectBodyPart(bodyPart, null, filenames)
121+
122+
then:
123+
filenames == ['report.php']
124+
}
125+
126+
def "null filenames list is skipped without error"() {
127+
given:
128+
def cd = Mock(FormDataContentDisposition)
129+
cd.getFileName() >> 'report.php'
130+
def bodyPart = Mock(FormDataBodyPart)
131+
bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
132+
bodyPart.getName() >> 'f'
133+
bodyPart.getValue() >> 'v'
134+
bodyPart.getFormDataContentDisposition() >> cd
135+
136+
expect:
137+
MultiPartHelper.collectBodyPart(bodyPart, [:], null)
138+
}
139+
}

dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/build.gradle

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,20 @@ muzzle {
2424

2525
apply from: "$rootDir/gradle/java.gradle"
2626

27+
testJvmConstraints {
28+
minJavaVersion = JavaVersion.VERSION_11
29+
}
30+
31+
configurations.configureEach {
32+
resolutionStrategy.deactivateDependencyLocking()
33+
}
34+
2735
dependencies {
2836
compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-common', version: '3.0.0'
2937
compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '3.0.0'
3038
compileOnly group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '3.0.0'
39+
40+
testImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '3.1.2'
3141
}
3242

3343
// tested in GrizzlyTest/GrizzlyAsyncTest
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package datadog.trace.instrumentation.jersey3;
2+
3+
import jakarta.ws.rs.core.MediaType;
4+
import java.util.ArrayList;
5+
import java.util.List;
6+
import java.util.Map;
7+
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
8+
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
9+
import org.glassfish.jersey.message.internal.MediaTypes;
10+
11+
public final class MultiPartHelper {
12+
13+
private MultiPartHelper() {}
14+
15+
public static void collectBodyPart(
16+
FormDataBodyPart bodyPart, Map<String, List<String>> bodyMap, List<String> filenames) {
17+
if (bodyMap != null
18+
&& MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())) {
19+
// BodyPartEntity allows re-reading the part without consuming the stream
20+
bodyMap.computeIfAbsent(bodyPart.getName(), k -> new ArrayList<>()).add(bodyPart.getValue());
21+
}
22+
if (filenames != null) {
23+
String filename = filenameFromBodyPart(bodyPart);
24+
if (filename != null) {
25+
filenames.add(filename);
26+
}
27+
}
28+
}
29+
30+
public static String filenameFromBodyPart(FormDataBodyPart bodyPart) {
31+
FormDataContentDisposition cd = bodyPart.getFormDataContentDisposition();
32+
if (cd == null) return null;
33+
String filename = cd.getFileName();
34+
return (filename == null || filename.isEmpty()) ? null : filename;
35+
}
36+
}

0 commit comments

Comments
 (0)