Skip to content

Commit 871f5b6

Browse files
jandro996devflow.devflow-routing-intake
jandro996
and
devflow.devflow-routing-intake
authored
Add server.request.body.files_content AppSec address for Akka HTTP (#11725)
feat(appsec): add server.request.body.files_content support for Akka HTTP - Extend handleMultipartStrictFormData (strictUnmarshaller) and handleStrictFormData (formFieldMultiMap) in UnmarshallerHelpers to accumulate file content via MultipartContentDecoder and dispatch EVENTS.requestFilesContent() callback - Content dispatch is sequential after filenames (fires only if no prior block), consistent with other frameworks (Tomcat, Netty, Jersey) - Uses ByteString.take(MAX_CONTENT_BYTES).toArray() to avoid full allocation; MAX_* constants read from Config - Add testBodyFilesContent() overrides to both akka-http-10.0 and akka-http-10.6 test modules fix(appsec): skip body decoding in handleStrictFormData when body callback absent When only files_content is subscribed (not requestBodyProcessed), the previous code still called getData().decodeString() on every file body part to populate conv — decoding the full file into a String regardless of MAX_CONTENT_BYTES. Gate decodeString and handleArbitraryPostData on bodyCb != null to avoid unnecessary full-file allocation. Merge branch 'master' into filecontent-akka fix: disable testBodyFilesContent in AkkaHttp102ServerInstrumentationBindSyncTest Sync server binding does not support body processing; override returns false to match the pattern already used for testBodyMultipart, testBodyFilenames, etc. fix: disable testBodyFilesContent in AkkaHttp102ServerInstrumentationBindSyncTest (10.6) Same fix as 10.0: sync binding does not process body, override returns false to match testBodyMultipart, testBodyFilenames, etc. Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 26ef439 commit 871f5b6

5 files changed

Lines changed: 161 additions & 44 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
@@ -79,6 +79,11 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
7979
true
8080
}
8181

82+
@Override
83+
boolean testBodyFilesContent() {
84+
true
85+
}
86+
8287
@Override
8388
boolean testBodyJson() {
8489
true
@@ -233,6 +238,11 @@ abstract class AkkaHttpServerInstrumentationSyncTest extends AkkaHttpServerInstr
233238
false
234239
}
235240

241+
@Override
242+
boolean testBodyFilesContent() {
243+
false
244+
}
245+
236246
@Override
237247
boolean testBodyJson() {
238248
false
@@ -298,6 +308,11 @@ class AkkaHttpServerInstrumentationBindAndHandleTest extends AkkaHttpServerInstr
298308
akkaHttpVersion != '10.0.10'
299309
}
300310

311+
@Override
312+
boolean testBodyFilesContent() {
313+
akkaHttpVersion != '10.0.10'
314+
}
315+
301316
@Override
302317
boolean testBodyUrlencoded() {
303318
akkaHttpVersion != '10.0.10'
@@ -329,6 +344,11 @@ class AkkaHttpServerInstrumentationBindAndHandleAsyncWithRouteAsyncHandlerTest e
329344
akkaHttpVersion != '10.0.10'
330345
}
331346

347+
@Override
348+
boolean testBodyFilesContent() {
349+
akkaHttpVersion != '10.0.10'
350+
}
351+
332352
@Override
333353
boolean testBodyUrlencoded() {
334354
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
@@ -41,6 +41,11 @@ class AkkaHttp102ServerInstrumentationBindSyncTest extends AkkaHttpServerInstrum
4141
false
4242
}
4343

44+
@Override
45+
boolean testBodyFilesContent() {
46+
false
47+
}
48+
4449
@Override
4550
boolean testBodyJson() {
4651
false

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

Lines changed: 111 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
import akka.japi.JavaPartialFunction;
1515
import akka.stream.Materializer;
1616
import datadog.appsec.api.blocking.BlockingException;
17+
import datadog.trace.api.Config;
1718
import datadog.trace.api.gateway.BlockResponseFunction;
1819
import datadog.trace.api.gateway.CallbackProvider;
1920
import datadog.trace.api.gateway.Flow;
2021
import datadog.trace.api.gateway.RequestContext;
2122
import datadog.trace.api.gateway.RequestContextSlot;
23+
import datadog.trace.api.http.MultipartContentDecoder;
2224
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
2325
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
2426
import java.lang.reflect.Field;
@@ -44,6 +46,8 @@
4446
public class UnmarshallerHelpers {
4547

4648
public static final int MAX_CONVERSION_DEPTH = 10;
49+
static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes();
50+
static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount();
4751
private static final Logger log = LoggerFactory.getLogger(UnmarshallerHelpers.class);
4852

4953
private static final MediaType APPLICATION_X_WWW_FORM_URLENCODED;
@@ -196,21 +200,29 @@ private static void handleMultipartStrictFormData(
196200
cbp.getCallback(EVENTS.requestBodyProcessed());
197201
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
198202
cbp.getCallback(EVENTS.requestFilesFilenames());
199-
if (bodyCallback == null && filenamesCallback == null) {
203+
BiFunction<RequestContext, List<String>, Flow<Void>> contentCallback =
204+
cbp.getCallback(EVENTS.requestFilesContent());
205+
if (bodyCallback == null && filenamesCallback == null && contentCallback == null) {
200206
return;
201207
}
202208

203209
java.lang.Iterable<akka.http.javadsl.model.Multipart.FormData.BodyPart.Strict> strictParts =
204210
st.getStrictParts();
205211
Map<String, List<String>> conv = new HashMap<>();
206212
List<String> filenames = filenamesCallback != null ? new ArrayList<>() : null;
213+
List<String> filesContent = contentCallback != null ? new ArrayList<>() : null;
207214
for (akka.http.javadsl.model.Multipart.FormData.BodyPart.Strict part : strictParts) {
208215
Optional<String> filenameOpt = part.getFilename();
209216
if (filenames != null && filenameOpt.isPresent() && !filenameOpt.get().isEmpty()) {
210217
filenames.add(filenameOpt.get());
211218
}
212219

213-
if (bodyCallback == null) {
220+
boolean needsEntity =
221+
bodyCallback != null
222+
|| (filesContent != null
223+
&& filenameOpt.isPresent()
224+
&& filesContent.size() < MAX_FILES_TO_INSPECT);
225+
if (!needsEntity) {
214226
continue;
215227
}
216228

@@ -221,19 +233,30 @@ private static void handleMultipartStrictFormData(
221233

222234
HttpEntity.Strict sentity = (HttpEntity.Strict) entity;
223235

224-
String name = part.getName();
225-
List<String> curStrings = conv.get(name);
226-
if (curStrings == null) {
227-
curStrings = new ArrayList<>();
228-
conv.put(name, curStrings);
236+
if (bodyCallback != null) {
237+
String name = part.getName();
238+
List<String> curStrings = conv.get(name);
239+
if (curStrings == null) {
240+
curStrings = new ArrayList<>();
241+
conv.put(name, curStrings);
242+
}
243+
244+
String s =
245+
sentity
246+
.getData()
247+
.decodeString(
248+
Unmarshaller$.MODULE$.bestUnmarshallingCharsetFor(sentity).nioCharset());
249+
curStrings.add(s);
229250
}
230251

231-
String s =
232-
sentity
233-
.getData()
234-
.decodeString(
235-
Unmarshaller$.MODULE$.bestUnmarshallingCharsetFor(sentity).nioCharset());
236-
curStrings.add(s);
252+
if (filesContent != null
253+
&& filenameOpt.isPresent()
254+
&& filesContent.size() < MAX_FILES_TO_INSPECT) {
255+
byte[] bytes = sentity.getData().take(MAX_CONTENT_BYTES).toArray();
256+
filesContent.add(
257+
MultipartContentDecoder.decodeBytes(
258+
bytes, bytes.length, entity.getContentType().toString()));
259+
}
237260
}
238261

239262
BlockingException pendingBlock = null;
@@ -260,6 +283,18 @@ private static void handleMultipartStrictFormData(
260283
}
261284
}
262285

286+
if (pendingBlock == null && filesContent != null && !filesContent.isEmpty()) {
287+
Flow<Void> flow = contentCallback.apply(reqCtx, filesContent);
288+
Flow.Action action = flow.getAction();
289+
if (action instanceof Flow.Action.RequestBlockingAction) {
290+
pendingBlock =
291+
tryBlock(
292+
reqCtx,
293+
(Flow.Action.RequestBlockingAction) action,
294+
"multipart file upload content");
295+
}
296+
}
297+
263298
if (pendingBlock != null) {
264299
throw pendingBlock;
265300
}
@@ -417,12 +452,20 @@ public static Unmarshaller<HttpEntity, StrictForm> transformStrictFormUnmarshall
417452

418453
private static void handleStrictFormData(StrictForm sf) {
419454
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
455+
BiFunction<RequestContext, Object, Flow<Void>> bodyCb =
456+
cbp.getCallback(EVENTS.requestBodyProcessed());
420457
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCb =
421458
cbp.getCallback(EVENTS.requestFilesFilenames());
459+
BiFunction<RequestContext, List<String>, Flow<Void>> contentCb =
460+
cbp.getCallback(EVENTS.requestFilesContent());
461+
if (bodyCb == null && filenamesCb == null && contentCb == null) {
462+
return;
463+
}
422464

423465
Iterator<Tuple2<String, StrictForm.Field>> iterator = sf.fields().iterator();
424466
Map<String, List<String>> conv = new HashMap<>();
425467
List<String> filenames = filenamesCb != null ? new ArrayList<>() : null;
468+
List<String> filesContent = contentCb != null ? new ArrayList<>() : null;
426469
while (iterator.hasNext()) {
427470
Tuple2<String, StrictForm.Field> next = iterator.next();
428471
String fieldName = next._1();
@@ -449,47 +492,71 @@ private static void handleStrictFormData(StrictForm sf) {
449492
instanceof akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict) {
450493
akka.http.scaladsl.model.Multipart$FormData$BodyPart$Strict bodyPart =
451494
(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-
}
495+
Optional<String> filenameOpt = bodyPart.getFilename();
496+
if (filenames != null && filenameOpt.isPresent() && !filenameOpt.get().isEmpty()) {
497+
filenames.add(filenameOpt.get());
457498
}
458499
HttpEntity.Strict sentity = bodyPart.entity();
459-
String s =
460-
sentity
461-
.getData()
462-
.decodeString(
463-
Unmarshaller$.MODULE$.bestUnmarshallingCharsetFor(sentity).nioCharset());
464-
strings.add(s);
500+
if (filesContent != null
501+
&& filenameOpt.isPresent()
502+
&& filesContent.size() < MAX_FILES_TO_INSPECT) {
503+
byte[] bytes = sentity.getData().take(MAX_CONTENT_BYTES).toArray();
504+
filesContent.add(
505+
MultipartContentDecoder.decodeBytes(
506+
bytes, bytes.length, sentity.contentType().toString()));
507+
}
508+
if (bodyCb != null) {
509+
String s =
510+
sentity
511+
.getData()
512+
.decodeString(
513+
Unmarshaller$.MODULE$.bestUnmarshallingCharsetFor(sentity).nioCharset());
514+
strings.add(s);
515+
}
465516
}
466517
}
467518

468519
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-
}
520+
if (bodyCb != null) {
521+
try {
522+
handleArbitraryPostData(conv, "HttpEntity -> StrictForm unmarshaller");
523+
} catch (BlockingException e) {
524+
pendingBlock = e;
525+
}
526+
}
527+
528+
AgentSpan span = activeSpan();
529+
RequestContext reqCtx = null;
530+
if (span != null) {
531+
RequestContext ctx = span.getRequestContext();
532+
if (ctx != null && ctx.getData(RequestContextSlot.APPSEC) != null) {
533+
reqCtx = ctx;
534+
}
535+
}
536+
537+
if (reqCtx != null && filenamesCb != null && filenames != null && !filenames.isEmpty()) {
538+
Flow<Void> flow = filenamesCb.apply(reqCtx, filenames);
539+
if (pendingBlock == null) {
540+
Flow.Action action = flow.getAction();
541+
if (action instanceof Flow.Action.RequestBlockingAction) {
542+
pendingBlock =
543+
tryBlock(reqCtx, (Flow.Action.RequestBlockingAction) action, "multipart file upload");
489544
}
490545
}
491546
}
492547

548+
if (pendingBlock == null && reqCtx != null && contentCb != null && !filesContent.isEmpty()) {
549+
Flow<Void> flow = contentCb.apply(reqCtx, filesContent);
550+
Flow.Action action = flow.getAction();
551+
if (action instanceof Flow.Action.RequestBlockingAction) {
552+
pendingBlock =
553+
tryBlock(
554+
reqCtx,
555+
(Flow.Action.RequestBlockingAction) action,
556+
"multipart file upload content");
557+
}
558+
}
559+
493560
if (pendingBlock != null) {
494561
throw pendingBlock;
495562
}

‎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
@@ -41,6 +41,11 @@ class AkkaHttp102ServerInstrumentationBindSyncTest extends AkkaHttpServerInstrum
4141
false
4242
}
4343

44+
@Override
45+
boolean testBodyFilesContent() {
46+
false
47+
}
48+
4449
@Override
4550
boolean testBodyJson() {
4651
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
@@ -80,6 +80,11 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
8080
true
8181
}
8282

83+
@Override
84+
boolean testBodyFilesContent() {
85+
true
86+
}
87+
8388
@Override
8489
boolean testBodyJson() {
8590
true
@@ -233,6 +238,11 @@ abstract class AkkaHttpServerInstrumentationSyncTest extends AkkaHttpServerInstr
233238
false
234239
}
235240

241+
@Override
242+
boolean testBodyFilesContent() {
243+
false
244+
}
245+
236246
@Override
237247
boolean testBodyJson() {
238248
false
@@ -294,6 +304,11 @@ class AkkaHttpServerInstrumentationBindAndHandleTest extends AkkaHttpServerInstr
294304
true
295305
}
296306

307+
@Override
308+
boolean testBodyFilesContent() {
309+
true
310+
}
311+
297312
@Override
298313
boolean testBodyUrlencoded() {
299314
true
@@ -325,6 +340,11 @@ class AkkaHttpServerInstrumentationBindAndHandleAsyncWithRouteAsyncHandlerTest e
325340
true
326341
}
327342

343+
@Override
344+
boolean testBodyFilesContent() {
345+
true
346+
}
347+
328348
@Override
329349
boolean testBodyUrlencoded() {
330350
true

0 commit comments

Comments
 (0)