Skip to content

Commit 3ee4e5e

Browse files
jandro996jandro996
committed
Add server.request.body.filenames support for Akka HTTP, Jersey, and Grizzly
- Akka HTTP 10.0/10.6: extract filenames in handleMultipartStrictFormData() via getFilename() on BodyPart.Strict and fire requestFilesFilenames callback. Also covers the formFieldMultiMap path via handleStrictFormData(). - Jersey 2.0/3.0 (MultiPartReaderServerSideInstrumentation): extract filenames from FormDataBodyPart.getContentDisposition().getFileName() and fire requestFilesFilenames after requestBodyProcessed. Grizzly uses this path. - Decouple requestBodyProcessed and requestFilesFilenames callbacks in Jersey and Akka handleMultipartStrictFormData: fetch both upfront, return early only if both are null. Previously an early return on requestBodyProcessed==null silently skipped filename detection, breaking deployments with filename-only WAF rules (same fix already applied to Undertow). - Merge the two iterations over strictParts in handleMultipartStrictFormData into a single loop. - Add null guard for getContentDisposition() in Jersey before calling getFileName(). - Enable testBodyFilenames() in Akka, Grizzly, Spring Boot, and Jersey3Jetty test suites.
1 parent 081af53 commit 3ee4e5e

11 files changed

Lines changed: 212 additions & 28 deletions

File tree

‎dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/groovy/AkkaHttpServerInstrumentationTest.groovy‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
7474
true
7575
}
7676

77+
@Override
78+
boolean testBodyFilenames() {
79+
true
80+
}
81+
7782
@Override
7883
boolean testBodyJson() {
7984
true
@@ -223,6 +228,11 @@ abstract class AkkaHttpServerInstrumentationSyncTest extends AkkaHttpServerInstr
223228
false
224229
}
225230

231+
@Override
232+
boolean testBodyFilenames() {
233+
false
234+
}
235+
226236
@Override
227237
boolean testBodyJson() {
228238
false
@@ -283,6 +293,11 @@ class AkkaHttpServerInstrumentationBindAndHandleTest extends AkkaHttpServerInstr
283293
akkaHttpVersion != '10.0.10'
284294
}
285295

296+
@Override
297+
boolean testBodyFilenames() {
298+
akkaHttpVersion != '10.0.10'
299+
}
300+
286301
@Override
287302
boolean testBodyUrlencoded() {
288303
akkaHttpVersion != '10.0.10'
@@ -309,6 +324,11 @@ class AkkaHttpServerInstrumentationBindAndHandleAsyncWithRouteAsyncHandlerTest e
309324
akkaHttpVersion != '10.0.10'
310325
}
311326

327+
@Override
328+
boolean testBodyFilenames() {
329+
akkaHttpVersion != '10.0.10'
330+
}
331+
312332
@Override
313333
boolean testBodyUrlencoded() {
314334
akkaHttpVersion != '10.0.10'

‎dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/latestDepTest/groovy/AkkaHttp102ServerInstrumentationTests.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ class AkkaHttp102ServerInstrumentationBindSyncTest extends AkkaHttpServerInstrum
3636
false
3737
}
3838

39+
@Override
40+
boolean testBodyFilenames() {
41+
false
42+
}
43+
3944
@Override
4045
boolean testBodyJson() {
4146
false

‎dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java‎

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.HashMap;
2727
import java.util.List;
2828
import java.util.Map;
29+
import java.util.Optional;
2930
import java.util.WeakHashMap;
3031
import java.util.function.BiFunction;
3132
import org.slf4j.Logger;
@@ -199,17 +200,24 @@ private static void handleMultipartStrictFormData(
199200
}
200201

201202
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
202-
BiFunction<RequestContext, Object, Flow<Void>> callback =
203+
BiFunction<RequestContext, Object, Flow<Void>> bodyCallback =
203204
cbp.getCallback(EVENTS.requestBodyProcessed());
204-
if (callback == null) {
205+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
206+
cbp.getCallback(EVENTS.requestFilesFilenames());
207+
if (bodyCallback == null && filenamesCallback == null) {
205208
return;
206209
}
207210

208-
// conversion to map string -> list of string
209211
java.lang.Iterable<akka.http.javadsl.model.Multipart.FormData.BodyPart.Strict> strictParts =
210212
st.getStrictParts();
211213
Map<String, List<String>> conv = new HashMap<>();
214+
List<String> filenames = new ArrayList<>();
212215
for (akka.http.javadsl.model.Multipart.FormData.BodyPart.Strict part : strictParts) {
216+
Optional<String> filenameOpt = part.getFilename();
217+
if (filenameOpt.isPresent() && !filenameOpt.get().isEmpty()) {
218+
filenames.add(filenameOpt.get());
219+
}
220+
213221
akka.http.javadsl.model.HttpEntity.Strict entity = part.getEntity();
214222
if (!(entity instanceof HttpEntity.Strict)) {
215223
continue;
@@ -232,8 +240,27 @@ private static void handleMultipartStrictFormData(
232240
curStrings.add(s);
233241
}
234242

235-
// callback execution
236-
executeCallback(reqCtx, callback, conv, "multipartFormDataUnmarshaller");
243+
if (bodyCallback != null) {
244+
executeCallback(reqCtx, bodyCallback, conv, "multipartFormDataUnmarshaller");
245+
}
246+
247+
if (filenamesCallback != null && !filenames.isEmpty()) {
248+
Flow<Void> filenamesFlow = filenamesCallback.apply(reqCtx, filenames);
249+
Flow.Action filenamesAction = filenamesFlow.getAction();
250+
if (filenamesAction instanceof Flow.Action.RequestBlockingAction) {
251+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) filenamesAction;
252+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
253+
if (brf != null) {
254+
boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
255+
if (success) {
256+
if (brf instanceof AkkaBlockResponseFunction) {
257+
((AkkaBlockResponseFunction) brf).setUnmarshallBlock(true);
258+
}
259+
throw new BlockingException("Blocked request (multipart file upload)");
260+
}
261+
}
262+
}
263+
}
237264
}
238265

239266
public static Unmarshaller<HttpEntity, String> transformStringUnmarshaller(
@@ -389,6 +416,7 @@ public static Unmarshaller<HttpEntity, StrictForm> transformStrictFormUnmarshall
389416
private static void handleStrictFormData(StrictForm sf) {
390417
Iterator<Tuple2<String, StrictForm.Field>> iterator = sf.fields().iterator();
391418
Map<String, List<String>> conv = new HashMap<>();
419+
List<String> filenames = new ArrayList<>();
392420
while (iterator.hasNext()) {
393421
Tuple2<String, StrictForm.Field> next = iterator.next();
394422
String fieldName = next._1();
@@ -413,9 +441,13 @@ private static void handleStrictFormData(StrictForm sf) {
413441
strings.add((String) strictFieldValue);
414442
} else if (strictFieldValue
415443
instanceof akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict) {
416-
HttpEntity.Strict sentity =
417-
((akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict) strictFieldValue)
418-
.entity();
444+
akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict bodyPart =
445+
(akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict) strictFieldValue;
446+
Optional<String> filenameOpt = bodyPart.getFilename();
447+
if (filenameOpt.isPresent() && !filenameOpt.get().isEmpty()) {
448+
filenames.add(filenameOpt.get());
449+
}
450+
HttpEntity.Strict sentity = bodyPart.entity();
419451
String s =
420452
sentity
421453
.getData()
@@ -426,6 +458,36 @@ private static void handleStrictFormData(StrictForm sf) {
426458
}
427459

428460
handleArbitraryPostData(conv, "HttpEntity -> StrictForm unmarshaller");
461+
462+
if (!filenames.isEmpty()) {
463+
AgentSpan span = activeSpan();
464+
RequestContext reqCtx;
465+
if (span != null
466+
&& (reqCtx = span.getRequestContext()) != null
467+
&& reqCtx.getData(RequestContextSlot.APPSEC) != null) {
468+
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
469+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCb =
470+
cbp.getCallback(EVENTS.requestFilesFilenames());
471+
if (filenamesCb != null) {
472+
Flow<Void> filenamesFlow = filenamesCb.apply(reqCtx, filenames);
473+
Flow.Action filenamesAction = filenamesFlow.getAction();
474+
if (filenamesAction instanceof Flow.Action.RequestBlockingAction) {
475+
Flow.Action.RequestBlockingAction rba =
476+
(Flow.Action.RequestBlockingAction) filenamesAction;
477+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
478+
if (brf != null) {
479+
boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
480+
if (success) {
481+
if (brf instanceof AkkaBlockResponseFunction) {
482+
((AkkaBlockResponseFunction) brf).setUnmarshallBlock(true);
483+
}
484+
throw new BlockingException("Blocked request (multipart file upload)");
485+
}
486+
}
487+
}
488+
}
489+
}
490+
}
429491
}
430492

431493
private static Object tryConvertingScalaContainers(Object obj, int depth) {

‎dd-java-agent/instrumentation/akka/akka-http/akka-http-10.6/src/test/groovy/AkkaHttp102ServerInstrumentationTests.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ class AkkaHttp102ServerInstrumentationBindSyncTest extends AkkaHttpServerInstrum
3636
false
3737
}
3838

39+
@Override
40+
boolean testBodyFilenames() {
41+
false
42+
}
43+
3944
@Override
4045
boolean testBodyJson() {
4146
false

‎dd-java-agent/instrumentation/akka/akka-http/akka-http-10.6/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
7575
true
7676
}
7777

78+
@Override
79+
boolean testBodyFilenames() {
80+
true
81+
}
82+
7883
@Override
7984
boolean testBodyJson() {
8085
true
@@ -223,6 +228,11 @@ abstract class AkkaHttpServerInstrumentationSyncTest extends AkkaHttpServerInstr
223228
false
224229
}
225230

231+
@Override
232+
boolean testBodyFilenames() {
233+
false
234+
}
235+
226236
@Override
227237
boolean testBodyJson() {
228238
false
@@ -279,6 +289,11 @@ class AkkaHttpServerInstrumentationBindAndHandleTest extends AkkaHttpServerInstr
279289
true
280290
}
281291

292+
@Override
293+
boolean testBodyFilenames() {
294+
true
295+
}
296+
282297
@Override
283298
boolean testBodyUrlencoded() {
284299
true
@@ -305,6 +320,11 @@ class AkkaHttpServerInstrumentationBindAndHandleAsyncWithRouteAsyncHandlerTest e
305320
true
306321
}
307322

323+
@Override
324+
boolean testBodyFilenames() {
325+
true
326+
}
327+
308328
@Override
309329
boolean testBodyUrlencoded() {
310330
true

‎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/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/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartReaderServerSideInstrumentation.java‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import javax.ws.rs.core.MediaType;
2727
import net.bytebuddy.asm.Advice;
2828
import org.glassfish.jersey.media.multipart.BodyPart;
29+
import org.glassfish.jersey.media.multipart.ContentDisposition;
2930
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
3031
import org.glassfish.jersey.media.multipart.MultiPart;
3132
import org.glassfish.jersey.message.internal.MediaTypes;
@@ -70,18 +71,26 @@ static void after(
7071
}
7172

7273
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
73-
BiFunction<RequestContext, Object, Flow<Void>> callback =
74+
BiFunction<RequestContext, Object, Flow<Void>> bodyCallback =
7475
cbp.getCallback(EVENTS.requestBodyProcessed());
75-
if (callback == null) {
76+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCb =
77+
cbp.getCallback(EVENTS.requestFilesFilenames());
78+
if (bodyCallback == null && filenamesCb == null) {
7679
return;
7780
}
7881

7982
Map<String, List<String>> map = new HashMap<>();
83+
List<String> filenames = new ArrayList<>();
8084
for (BodyPart bodyPart : ret.getBodyParts()) {
8185
if (!(bodyPart instanceof FormDataBodyPart)) {
8286
continue;
8387
}
8488
FormDataBodyPart dataBodyPart = (FormDataBodyPart) bodyPart;
89+
ContentDisposition cd = dataBodyPart.getContentDisposition();
90+
String filename = cd != null ? cd.getFileName() : null;
91+
if (filename != null && !filename.isEmpty()) {
92+
filenames.add(filename);
93+
}
8594
if (!MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) {
8695
continue;
8796
}
@@ -99,14 +108,31 @@ static void after(
99108
values.add(v);
100109
}
101110

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)");
111+
if (bodyCallback != null) {
112+
Flow<Void> flow = bodyCallback.apply(reqCtx, map);
113+
Flow.Action action = flow.getAction();
114+
if (action instanceof Flow.Action.RequestBlockingAction) {
115+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
116+
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
117+
if (blockResponseFunction != null) {
118+
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
119+
t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)");
120+
reqCtx.getTraceSegment().effectivelyBlocked();
121+
}
122+
}
123+
}
124+
125+
if (filenamesCb != null && !filenames.isEmpty()) {
126+
Flow<Void> filenamesFlow = filenamesCb.apply(reqCtx, filenames);
127+
Flow.Action filenamesAction = filenamesFlow.getAction();
128+
if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) {
129+
Flow.Action.RequestBlockingAction rba =
130+
(Flow.Action.RequestBlockingAction) filenamesAction;
131+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
132+
if (brf != null) {
133+
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
134+
}
135+
t = new BlockingException("Blocked request (multipart file upload)");
110136
reqCtx.getTraceSegment().effectivelyBlocked();
111137
}
112138
}

0 commit comments

Comments
 (0)