Skip to content

Commit 6ff7c4d

Browse files
jandro996jandro996
committed
Add server.request.body.filenames support for Jersey and RESTEasy
1 parent 8569434 commit 6ff7c4d

14 files changed

Lines changed: 460 additions & 62 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

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package datadog.trace.instrumentation.jersey2;
2+
3+
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
4+
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
5+
6+
public final class MultiPartHelper {
7+
8+
private MultiPartHelper() {}
9+
10+
public static String filenameFromBodyPart(FormDataBodyPart bodyPart) {
11+
FormDataContentDisposition cd = bodyPart.getFormDataContentDisposition();
12+
if (cd == null) return null;
13+
String filename = cd.getFileName();
14+
return (filename == null || filename.isEmpty()) ? null : filename;
15+
}
16+
}

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

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ public String instrumentedType() {
4848
return "org.glassfish.jersey.media.multipart.internal.MultiPartReaderServerSide";
4949
}
5050

51+
@Override
52+
public String[] helperClassNames() {
53+
return new String[] {packageName + ".MultiPartHelper"};
54+
}
55+
5156
@Override
5257
public void methodAdvice(MethodTransformer transformer) {
5358
transformer.applyAdvice(
@@ -72,42 +77,66 @@ static void after(
7277
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
7378
BiFunction<RequestContext, Object, Flow<Void>> callback =
7479
cbp.getCallback(EVENTS.requestBodyProcessed());
75-
if (callback == null) {
80+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
81+
cbp.getCallback(EVENTS.requestFilesFilenames());
82+
if (callback == null && filenamesCallback == null) {
7683
return;
7784
}
7885

79-
Map<String, List<String>> map = new HashMap<>();
86+
Map<String, List<String>> map = callback != null ? new HashMap<>() : null;
87+
List<String> filenames = filenamesCallback != null ? new ArrayList<>() : null;
8088
for (BodyPart bodyPart : ret.getBodyParts()) {
8189
if (!(bodyPart instanceof FormDataBodyPart)) {
8290
continue;
8391
}
8492
FormDataBodyPart dataBodyPart = (FormDataBodyPart) bodyPart;
85-
if (!MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) {
86-
continue;
93+
if (map != null
94+
&& MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) {
95+
// if the type of dataBodyPart.getEntity() is BodyPartEntity, it is safe to read the part
96+
// more than once. So we're not depriving the application of the data by consuming it here
97+
String v = dataBodyPart.getValue();
98+
String name = dataBodyPart.getName();
99+
List<String> values = map.get(name);
100+
if (values == null) {
101+
values = new ArrayList<>();
102+
map.put(name, values);
103+
}
104+
values.add(v);
87105
}
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();
91-
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);
106+
if (filenames != null) {
107+
String filename = MultiPartHelper.filenameFromBodyPart(dataBodyPart);
108+
if (filename != null) {
109+
filenames.add(filename);
110+
}
97111
}
112+
}
98113

99-
values.add(v);
114+
if (map != null) {
115+
Flow<Void> flow = callback.apply(reqCtx, map);
116+
Flow.Action action = flow.getAction();
117+
if (action instanceof Flow.Action.RequestBlockingAction) {
118+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
119+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
120+
if (blockResponseFunction != null) {
121+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
122+
t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)");
123+
reqCtx.getTraceSegment().effectivelyBlocked();
124+
}
125+
}
100126
}
101127

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();
128+
if (filenames != null && !filenames.isEmpty()) {
129+
Flow<Void> filenamesFlow = filenamesCallback.apply(reqCtx, filenames);
130+
Flow.Action filenamesAction = filenamesFlow.getAction();
131+
if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) {
132+
Flow.Action.RequestBlockingAction rba =
133+
(Flow.Action.RequestBlockingAction) filenamesAction;
134+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
135+
if (blockResponseFunction != null) {
136+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
137+
t = new BlockingException("Blocked request (multipart file upload)");
138+
reqCtx.getTraceSegment().effectivelyBlocked();
139+
}
111140
}
112141
}
113142
}

‎dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/test/groovy/MultiPartHelperTest.groovy‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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+
class MultiPartHelperTest extends Specification {
7+
8+
def "returns null when content disposition is null"() {
9+
given:
10+
def bodyPart = Mock(FormDataBodyPart)
11+
bodyPart.getFormDataContentDisposition() >> null
12+
13+
expect:
14+
MultiPartHelper.filenameFromBodyPart(bodyPart) == null
15+
}
16+
17+
def "returns null when filename is null"() {
18+
given:
19+
def cd = Mock(FormDataContentDisposition)
20+
cd.getFileName() >> null
21+
def bodyPart = Mock(FormDataBodyPart)
22+
bodyPart.getFormDataContentDisposition() >> cd
23+
24+
expect:
25+
MultiPartHelper.filenameFromBodyPart(bodyPart) == null
26+
}
27+
28+
def "returns null when filename is empty"() {
29+
given:
30+
def cd = Mock(FormDataContentDisposition)
31+
cd.getFileName() >> ''
32+
def bodyPart = Mock(FormDataBodyPart)
33+
bodyPart.getFormDataContentDisposition() >> cd
34+
35+
expect:
36+
MultiPartHelper.filenameFromBodyPart(bodyPart) == null
37+
}
38+
39+
def "extracts filename"() {
40+
given:
41+
def cd = Mock(FormDataContentDisposition)
42+
cd.getFileName() >> filename
43+
def bodyPart = Mock(FormDataBodyPart)
44+
bodyPart.getFormDataContentDisposition() >> cd
45+
46+
expect:
47+
MultiPartHelper.filenameFromBodyPart(bodyPart) == filename
48+
49+
where:
50+
filename << ['report.php', 'upload.txt', 'shell;evil.php', 'file"name.php']
51+
}
52+
}

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

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

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

27+
configurations.configureEach {
28+
resolutionStrategy.deactivateDependencyLocking()
29+
}
30+
2731
dependencies {
2832
compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-common', version: '3.0.0'
2933
compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '3.0.0'
3034
compileOnly group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '3.0.0'
35+
36+
testImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '3.1.2'
3137
}
3238

3339
// tested in GrizzlyTest/GrizzlyAsyncTest

‎dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartHelper.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package datadog.trace.instrumentation.jersey3;
2+
3+
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
4+
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
5+
6+
public final class MultiPartHelper {
7+
8+
private MultiPartHelper() {}
9+
10+
public static String filenameFromBodyPart(FormDataBodyPart bodyPart) {
11+
FormDataContentDisposition cd = bodyPart.getFormDataContentDisposition();
12+
if (cd == null) return null;
13+
String filename = cd.getFileName();
14+
return (filename == null || filename.isEmpty()) ? null : filename;
15+
}
16+
}

‎dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartReaderServerSideInstrumentation.java‎

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ public String instrumentedType() {
4848
return "org.glassfish.jersey.media.multipart.internal.MultiPartReaderServerSide";
4949
}
5050

51+
@Override
52+
public String[] helperClassNames() {
53+
return new String[] {packageName + ".MultiPartHelper"};
54+
}
55+
5156
@Override
5257
public void methodAdvice(MethodTransformer transformer) {
5358
transformer.applyAdvice(
@@ -72,42 +77,66 @@ static void after(
7277
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
7378
BiFunction<RequestContext, Object, Flow<Void>> callback =
7479
cbp.getCallback(EVENTS.requestBodyProcessed());
75-
if (callback == null) {
80+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
81+
cbp.getCallback(EVENTS.requestFilesFilenames());
82+
if (callback == null && filenamesCallback == null) {
7683
return;
7784
}
7885

79-
Map<String, List<String>> map = new HashMap<>();
86+
Map<String, List<String>> map = callback != null ? new HashMap<>() : null;
87+
List<String> filenames = filenamesCallback != null ? new ArrayList<>() : null;
8088
for (BodyPart bodyPart : ret.getBodyParts()) {
8189
if (!(bodyPart instanceof FormDataBodyPart)) {
8290
continue;
8391
}
8492
FormDataBodyPart dataBodyPart = (FormDataBodyPart) bodyPart;
85-
if (!MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) {
86-
continue;
93+
if (map != null
94+
&& MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) {
95+
// if the type of dataBodyPart.getEntity() is BodyPartEntity, it is safe to read the part
96+
// more than once. So we're not depriving the application of the data by consuming it here
97+
String v = dataBodyPart.getValue();
98+
String name = dataBodyPart.getName();
99+
List<String> values = map.get(name);
100+
if (values == null) {
101+
values = new ArrayList<>();
102+
map.put(name, values);
103+
}
104+
values.add(v);
87105
}
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();
91-
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);
106+
if (filenames != null) {
107+
String filename = MultiPartHelper.filenameFromBodyPart(dataBodyPart);
108+
if (filename != null) {
109+
filenames.add(filename);
110+
}
97111
}
112+
}
98113

99-
values.add(v);
114+
if (map != null) {
115+
Flow<Void> flow = callback.apply(reqCtx, map);
116+
Flow.Action action = flow.getAction();
117+
if (action instanceof Flow.Action.RequestBlockingAction) {
118+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
119+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
120+
if (blockResponseFunction != null) {
121+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
122+
t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)");
123+
reqCtx.getTraceSegment().effectivelyBlocked();
124+
}
125+
}
100126
}
101127

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();
128+
if (filenames != null && !filenames.isEmpty()) {
129+
Flow<Void> filenamesFlow = filenamesCallback.apply(reqCtx, filenames);
130+
Flow.Action filenamesAction = filenamesFlow.getAction();
131+
if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) {
132+
Flow.Action.RequestBlockingAction rba =
133+
(Flow.Action.RequestBlockingAction) filenamesAction;
134+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
135+
if (blockResponseFunction != null) {
136+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
137+
t = new BlockingException("Blocked request (multipart file upload)");
138+
reqCtx.getTraceSegment().effectivelyBlocked();
139+
}
111140
}
112141
}
113142
}

0 commit comments

Comments
 (0)