Skip to content

Commit 9aedc5b

Browse files
jandro996devflow.devflow-routing-intake
jandro996
and
devflow.devflow-routing-intake
authored
Add server.request.body.filenames AppSec address for Akka HTTP (#11173)
Add server.request.body.filenames support for Akka HTTP Address review comments: always fire both body and filenames callbacks - Fire both bodyCallback and filenamesCallback regardless of whether one triggers blocking, matching the canonical Tomcat/Liberty pattern. Previously, executeCallback threw BlockingException immediately, preventing filenamesCallback from running if body was blocked. - Avoid creating the filenames ArrayList when filenamesCallback is null. - Extract tryBlock() helper to deduplicate blocking logic shared between handleMultipartStrictFormData, handleStrictFormData, and executeCallback. Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent da5e7c3 commit 9aedc5b

7 files changed

Lines changed: 164 additions & 21 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: 104 additions & 21 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;
@@ -124,18 +125,10 @@ private static void executeCallback(
124125
Flow<Void> flow = callback.apply(reqCtx, conv);
125126
Flow.Action action = flow.getAction();
126127
if (action instanceof Flow.Action.RequestBlockingAction) {
127-
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
128-
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
129-
if (blockResponseFunction != null) {
130-
boolean success =
131-
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
132-
if (success) {
133-
if (blockResponseFunction instanceof AkkaBlockResponseFunction) {
134-
AkkaBlockResponseFunction abrf = (AkkaBlockResponseFunction) blockResponseFunction;
135-
abrf.setUnmarshallBlock(true);
136-
}
137-
throw new BlockingException("Blocked request (for " + details + ")");
138-
}
128+
BlockingException e =
129+
tryBlock(reqCtx, (Flow.Action.RequestBlockingAction) action, "for " + details);
130+
if (e != null) {
131+
throw e;
139132
}
140133
}
141134
}
@@ -199,17 +192,28 @@ private static void handleMultipartStrictFormData(
199192
}
200193

201194
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
202-
BiFunction<RequestContext, Object, Flow<Void>> callback =
195+
BiFunction<RequestContext, Object, Flow<Void>> bodyCallback =
203196
cbp.getCallback(EVENTS.requestBodyProcessed());
204-
if (callback == null) {
197+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
198+
cbp.getCallback(EVENTS.requestFilesFilenames());
199+
if (bodyCallback == null && filenamesCallback == null) {
205200
return;
206201
}
207202

208-
// conversion to map string -> list of string
209203
java.lang.Iterable<akka.http.javadsl.model.Multipart.FormData.BodyPart.Strict> strictParts =
210204
st.getStrictParts();
211205
Map<String, List<String>> conv = new HashMap<>();
206+
List<String> filenames = filenamesCallback != null ? new ArrayList<>() : null;
212207
for (akka.http.javadsl.model.Multipart.FormData.BodyPart.Strict part : strictParts) {
208+
Optional<String> filenameOpt = part.getFilename();
209+
if (filenames != null && filenameOpt.isPresent() && !filenameOpt.get().isEmpty()) {
210+
filenames.add(filenameOpt.get());
211+
}
212+
213+
if (bodyCallback == null) {
214+
continue;
215+
}
216+
213217
akka.http.javadsl.model.HttpEntity.Strict entity = part.getEntity();
214218
if (!(entity instanceof HttpEntity.Strict)) {
215219
continue;
@@ -232,8 +236,33 @@ private static void handleMultipartStrictFormData(
232236
curStrings.add(s);
233237
}
234238

235-
// callback execution
236-
executeCallback(reqCtx, callback, conv, "multipartFormDataUnmarshaller");
239+
BlockingException pendingBlock = null;
240+
if (bodyCallback != null) {
241+
Flow<Void> flow = bodyCallback.apply(reqCtx, conv);
242+
Flow.Action action = flow.getAction();
243+
if (action instanceof Flow.Action.RequestBlockingAction) {
244+
pendingBlock =
245+
tryBlock(
246+
reqCtx,
247+
(Flow.Action.RequestBlockingAction) action,
248+
"multipartFormDataUnmarshaller");
249+
}
250+
}
251+
252+
if (filenamesCallback != null && filenames != null && !filenames.isEmpty()) {
253+
Flow<Void> flow = filenamesCallback.apply(reqCtx, filenames);
254+
if (pendingBlock == null) {
255+
Flow.Action action = flow.getAction();
256+
if (action instanceof Flow.Action.RequestBlockingAction) {
257+
pendingBlock =
258+
tryBlock(reqCtx, (Flow.Action.RequestBlockingAction) action, "multipart file upload");
259+
}
260+
}
261+
}
262+
263+
if (pendingBlock != null) {
264+
throw pendingBlock;
265+
}
237266
}
238267

239268
public static Unmarshaller<HttpEntity, String> transformStringUnmarshaller(
@@ -387,8 +416,13 @@ public static Unmarshaller<HttpEntity, StrictForm> transformStrictFormUnmarshall
387416
}
388417

389418
private static void handleStrictFormData(StrictForm sf) {
419+
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
420+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCb =
421+
cbp.getCallback(EVENTS.requestFilesFilenames());
422+
390423
Iterator<Tuple2<String, StrictForm.Field>> iterator = sf.fields().iterator();
391424
Map<String, List<String>> conv = new HashMap<>();
425+
List<String> filenames = filenamesCb != null ? new ArrayList<>() : null;
392426
while (iterator.hasNext()) {
393427
Tuple2<String, StrictForm.Field> next = iterator.next();
394428
String fieldName = next._1();
@@ -413,9 +447,15 @@ private static void handleStrictFormData(StrictForm sf) {
413447
strings.add((String) strictFieldValue);
414448
} else if (strictFieldValue
415449
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();
450+
akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict bodyPart =
451+
(akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict) strictFieldValue;
452+
if (filenames != null) {
453+
Optional<String> filenameOpt = bodyPart.getFilename();
454+
if (filenameOpt.isPresent() && !filenameOpt.get().isEmpty()) {
455+
filenames.add(filenameOpt.get());
456+
}
457+
}
458+
HttpEntity.Strict sentity = bodyPart.entity();
419459
String s =
420460
sentity
421461
.getData()
@@ -425,7 +465,34 @@ private static void handleStrictFormData(StrictForm sf) {
425465
}
426466
}
427467

428-
handleArbitraryPostData(conv, "HttpEntity -> StrictForm unmarshaller");
468+
BlockingException pendingBlock = null;
469+
try {
470+
handleArbitraryPostData(conv, "HttpEntity -> StrictForm unmarshaller");
471+
} catch (BlockingException e) {
472+
pendingBlock = e;
473+
}
474+
475+
if (filenamesCb != null && filenames != null && !filenames.isEmpty()) {
476+
AgentSpan span = activeSpan();
477+
RequestContext reqCtx;
478+
if (span != null
479+
&& (reqCtx = span.getRequestContext()) != null
480+
&& reqCtx.getData(RequestContextSlot.APPSEC) != null) {
481+
Flow<Void> flow = filenamesCb.apply(reqCtx, filenames);
482+
if (pendingBlock == null) {
483+
Flow.Action action = flow.getAction();
484+
if (action instanceof Flow.Action.RequestBlockingAction) {
485+
pendingBlock =
486+
tryBlock(
487+
reqCtx, (Flow.Action.RequestBlockingAction) action, "multipart file upload");
488+
}
489+
}
490+
}
491+
}
492+
493+
if (pendingBlock != null) {
494+
throw pendingBlock;
495+
}
429496
}
430497

431498
private static Object tryConvertingScalaContainers(Object obj, int depth) {
@@ -473,6 +540,22 @@ private static void handleArbitraryPostData(Object o, String source) {
473540
executeCallback(reqCtx, callback, o, source);
474541
}
475542

543+
private static BlockingException tryBlock(
544+
RequestContext reqCtx, Flow.Action.RequestBlockingAction rba, String details) {
545+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
546+
if (brf == null) {
547+
return null;
548+
}
549+
boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
550+
if (!success) {
551+
return null;
552+
}
553+
if (brf instanceof AkkaBlockResponseFunction) {
554+
((AkkaBlockResponseFunction) brf).setUnmarshallBlock(true);
555+
}
556+
return new BlockingException("Blocked request (" + details + ")");
557+
}
558+
476559
private static void handleException(Exception e, String logMessage) {
477560
if (e instanceof BlockingException) {
478561
throw (BlockingException) e;

‎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/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/test/boot/SpringBootBasedTest.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
109109
true
110110
}
111111

112+
@Override
113+
boolean testBodyFilenames() {
114+
true
115+
}
116+
112117
@Override
113118
boolean testBodyJson() {
114119
true

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
119119
true
120120
}
121121

122+
@Override
123+
boolean testBodyFilenames() {
124+
true
125+
}
126+
122127
@Override
123128
boolean testBodyJson() {
124129
true

0 commit comments

Comments
 (0)