From 0e487847661cfe717ae4e2eaf7563f11ad33fb65 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 24 Jun 2026 13:11:32 +0200 Subject: [PATCH 1/4] feat(appsec): add server.request.body.files_content for Undertow and Play Implements the files_content WAF address for Undertow 2.0/2.2+ and Play 2.5/2.6. - Undertow: new FormDataContentHelper reads file content via getPath() with reflection fallback for 2.2+ in-memory uploads (getFileItem().getInputStream()) - Play 2.5/2.6: extends BodyParserHelpers with handleMultipartFilesContent(), gated on pendingBlock == null so filenames blocking suppresses content inspection - Content callback fires sequentially after filenames, never if already blocked - Charset-aware decoding via MultipartContentDecoder throughout - MAX_CONTENT_BYTES / MAX_FILES_TO_INSPECT kept in helper classes (not advice) to avoid muzzle failures --- .../play25/appsec/BodyParserHelpers.java | 130 ++++++++++++++ .../play25/server/PlayServerTest.groovy | 5 + .../play26/appsec/BodyParserHelpers.java | 130 ++++++++++++++ .../play26/server/PlayAsyncServerTest.groovy | 5 + .../play26/server/PlayServerTest.groovy | 5 + .../undertow/FormDataContentHelper.java | 99 +++++++++++ ...MultiPartUploadHandlerInstrumentation.java | 25 ++- .../test/groovy/UndertowServletTest.groovy | 5 + .../undertow/FormDataContentHelperTest.java | 162 ++++++++++++++++++ .../test/groovy/UndertowServletTest.groovy | 5 + 10 files changed, 569 insertions(+), 2 deletions(-) create mode 100644 dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java create mode 100644 dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/java/datadog/trace/instrumentation/undertow/FormDataContentHelperTest.java diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java index 16f0309e4bc..0df0e62b9c1 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java @@ -12,13 +12,19 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TextNode; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.http.MultipartContentDecoder; +import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.io.File; +import java.io.FileInputStream; +import java.io.InputStream; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.HashMap; @@ -54,6 +60,15 @@ public class BodyParserHelpers { // cause muzzle to disable the instrumentation for Play 2.7. private static final Method MULTIPART_FILES_METHOD; + // Cached via reflection: FilePart.ref() returns the generic file reference (TemporaryFile in + // Play 2.5/2.6), and TemporaryFile.file() returns java.io.File. Using reflection avoids + // embedding bytecode references to TemporaryFile that could break muzzle. + private static final Method FILE_PART_REF; + private static final Method TEMP_FILE_FILE; + + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + static { Method m = null; try { @@ -61,6 +76,19 @@ public class BodyParserHelpers { } catch (Exception ignored) { } MULTIPART_FILES_METHOD = m; + + Method ref = null; + Method file = null; + try { + ref = MultipartFormData.FilePart.class.getMethod("ref"); + } catch (Exception ignored) { + } + try { + file = Class.forName("play.api.libs.Files$TemporaryFile").getMethod("file"); + } catch (Exception ignored) { + } + FILE_PART_REF = ref; + TEMP_FILE_FILE = file; } private static JFunction1< @@ -148,6 +176,22 @@ private static MultipartFormData handleMultipartFormData(MultipartFormData log.debug("Error handling multipartFormData filenames", e); } + if (pendingBlock == null) { + try { + if (MULTIPART_FILES_METHOD != null) { + Object files = MULTIPART_FILES_METHOD.invoke(data); + if (files instanceof scala.collection.Iterable) { + handleMultipartFilesContent( + new ScalaIteratorAdapter(((scala.collection.Iterable) files).iterator())); + } + } + } catch (BlockingException be) { + pendingBlock = be; + } catch (Exception e) { + log.debug("Error handling multipartFormData files content", e); + } + } + if (pendingBlock != null) throw pendingBlock; return data; } @@ -176,6 +220,7 @@ private static void handleMultipartFilenames(java.util.Iterator iterator) { executeFilenamesCallback(reqCtx, callback, filenames); } + @VisibleForTesting static List collectFilenames(java.util.Iterator iterator) { List filenames = new ArrayList<>(); while (iterator.hasNext()) { @@ -206,6 +251,91 @@ private static void executeFilenamesCallback( } } + private static void handleMultipartFilesContent(java.util.Iterator iterator) { + AgentSpan span = activeSpan(); + if (span == null) { + return; + } + RequestContext reqCtx = span.getRequestContext(); + if (reqCtx == null || reqCtx.getData(RequestContextSlot.APPSEC) == null) { + return; + } + + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> callback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (callback == null) { + return; + } + + List contents = collectFilesContent(iterator); + if (contents.isEmpty()) { + return; + } + + executeFilesContentCallback(reqCtx, callback, contents); + } + + @VisibleForTesting + static List collectFilesContent(java.util.Iterator iterator) { + List contents = new ArrayList<>(MAX_FILES_TO_INSPECT); + while (iterator.hasNext() && contents.size() < MAX_FILES_TO_INSPECT) { + MultipartFormData.FilePart part = (MultipartFormData.FilePart) iterator.next(); + // null filename → no Content-Disposition filename attribute → form field → skip + // empty filename → file upload with no name → content still inspected + if (part.filename() == null) { + continue; + } + contents.add(readFilePartContent(part)); + } + return contents; + } + + @VisibleForTesting + static String readFilePartContent(MultipartFormData.FilePart part) { + if (FILE_PART_REF == null || TEMP_FILE_FILE == null) { + return ""; + } + try { + Object ref = FILE_PART_REF.invoke(part); + if (ref == null) { + return ""; + } + Object fileObj = TEMP_FILE_FILE.invoke(ref); + if (!(fileObj instanceof File)) { + return ""; + } + String contentType = null; + scala.Option ct = (scala.Option) part.contentType(); + if (ct != null && ct.isDefined()) { + contentType = (String) ct.get(); + } + try (InputStream is = new FileInputStream((File) fileObj)) { + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } + } catch (Exception ignored) { + return ""; + } + } + + private static void executeFilesContentCallback( + RequestContext reqCtx, + BiFunction, Flow> callback, + List contents) { + Flow flow = callback.apply(reqCtx, contents); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (success) { + throw new BlockingException("Blocked request (multipart file upload content)"); + } + } + } + } + public static Function1 getHandleJsonF() { return HANDLE_JSON; } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy index 76e4dfbc7f2..fd10be014f3 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy @@ -88,6 +88,11 @@ class PlayServerTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + @Override boolean testBlocking() { true diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java index c5004dc3bbb..d0ee2e49470 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java @@ -12,13 +12,19 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TextNode; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.http.MultipartContentDecoder; +import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.io.File; +import java.io.FileInputStream; +import java.io.InputStream; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; @@ -64,6 +70,15 @@ public class BodyParserHelpers { // cause muzzle to disable the instrumentation for Play 2.7. private static final Method MULTIPART_FILES_METHOD; + // Cached via reflection: FilePart.ref() returns the generic file reference (TemporaryFile in + // Play 2.5/2.6), and TemporaryFile.file() returns java.io.File. Using reflection avoids + // embedding bytecode references to TemporaryFile that could break muzzle. + private static final Method FILE_PART_REF; + private static final Method TEMP_FILE_FILE; + + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + static { Method m = null; try { @@ -71,6 +86,19 @@ public class BodyParserHelpers { } catch (Exception ignored) { } MULTIPART_FILES_METHOD = m; + + Method ref = null; + Method file = null; + try { + ref = MultipartFormData.FilePart.class.getMethod("ref"); + } catch (Exception ignored) { + } + try { + file = Class.forName("play.api.libs.Files$TemporaryFile").getMethod("file"); + } catch (Exception ignored) { + } + FILE_PART_REF = ref; + TEMP_FILE_FILE = file; } private static JFunction1< @@ -159,6 +187,22 @@ private static MultipartFormData handleMultipartFormData(MultipartFormData log.debug("Error handling multipartFormData filenames", e); } + if (pendingBlock == null) { + try { + if (MULTIPART_FILES_METHOD != null) { + Object files = MULTIPART_FILES_METHOD.invoke(data); + if (files instanceof scala.collection.Iterable) { + handleMultipartFilesContent( + new ScalaIteratorAdapter(((scala.collection.Iterable) files).iterator())); + } + } + } catch (BlockingException be) { + pendingBlock = be; + } catch (Exception e) { + log.debug("Error handling multipartFormData files content", e); + } + } + if (pendingBlock != null) throw pendingBlock; return data; } @@ -187,6 +231,7 @@ private static void handleMultipartFilenames(java.util.Iterator iterator) { executeFilenamesCallback(reqCtx, callback, filenames); } + @VisibleForTesting static List collectFilenames(java.util.Iterator iterator) { List filenames = new ArrayList<>(); while (iterator.hasNext()) { @@ -217,6 +262,91 @@ private static void executeFilenamesCallback( } } + private static void handleMultipartFilesContent(java.util.Iterator iterator) { + AgentSpan span = activeSpan(); + if (span == null) { + return; + } + RequestContext reqCtx = span.getRequestContext(); + if (reqCtx == null || reqCtx.getData(RequestContextSlot.APPSEC) == null) { + return; + } + + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> callback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (callback == null) { + return; + } + + List contents = collectFilesContent(iterator); + if (contents.isEmpty()) { + return; + } + + executeFilesContentCallback(reqCtx, callback, contents); + } + + @VisibleForTesting + static List collectFilesContent(java.util.Iterator iterator) { + List contents = new ArrayList<>(MAX_FILES_TO_INSPECT); + while (iterator.hasNext() && contents.size() < MAX_FILES_TO_INSPECT) { + MultipartFormData.FilePart part = (MultipartFormData.FilePart) iterator.next(); + // null filename → no Content-Disposition filename attribute → form field → skip + // empty filename → file upload with no name → content still inspected + if (part.filename() == null) { + continue; + } + contents.add(readFilePartContent(part)); + } + return contents; + } + + @VisibleForTesting + static String readFilePartContent(MultipartFormData.FilePart part) { + if (FILE_PART_REF == null || TEMP_FILE_FILE == null) { + return ""; + } + try { + Object ref = FILE_PART_REF.invoke(part); + if (ref == null) { + return ""; + } + Object fileObj = TEMP_FILE_FILE.invoke(ref); + if (!(fileObj instanceof File)) { + return ""; + } + String contentType = null; + scala.Option ct = (scala.Option) part.contentType(); + if (ct != null && ct.isDefined()) { + contentType = (String) ct.get(); + } + try (InputStream is = new FileInputStream((File) fileObj)) { + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } + } catch (Exception ignored) { + return ""; + } + } + + private static void executeFilesContentCallback( + RequestContext reqCtx, + BiFunction, Flow> callback, + List contents) { + Flow flow = callback.apply(reqCtx, contents); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (success) { + throw new BlockingException("Blocked request (multipart file upload content)"); + } + } + } + } + public static Function1 getHandleJsonF() { return HANDLE_JSON; } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayAsyncServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayAsyncServerTest.groovy index 339c17ff797..fdf35a116f9 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayAsyncServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayAsyncServerTest.groovy @@ -13,6 +13,11 @@ class PlayAsyncServerTest extends AbstractPlayServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + @Shared def executor diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy index b48ff0ad201..0ac729a38ac 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy @@ -12,6 +12,11 @@ class PlayServerTest extends AbstractPlayServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + def 'test instrumentation gateway xml request body'() { setup: def request = request( diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java new file mode 100644 index 00000000000..9901a9b67b9 --- /dev/null +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java @@ -0,0 +1,99 @@ +package datadog.trace.instrumentation.undertow; + +import datadog.trace.api.Config; +import datadog.trace.api.http.MultipartContentDecoder; +import datadog.trace.api.internal.VisibleForTesting; +import io.undertow.server.handlers.form.FormData; +import io.undertow.util.HeaderMap; +import io.undertow.util.Headers; +import java.io.InputStream; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class FormDataContentHelper { + + private static final Logger log = LoggerFactory.getLogger(FormDataContentHelper.class); + + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + + // Undertow 2.2+ added getFileItem() to FormValue for in-memory uploads. + // In 2.0 all uploads are always on disk so getPath() suffices. + private static final Method GET_FILE_ITEM; + private static final Method FILE_ITEM_GET_INPUT_STREAM; + + static { + Method gfi = null; + Method gis = null; + try { + gfi = FormData.FormValue.class.getMethod("getFileItem"); + gis = gfi.getReturnType().getMethod("getInputStream"); + } catch (Exception ignored) { + } + GET_FILE_ITEM = gfi; + FILE_ITEM_GET_INPUT_STREAM = gis; + } + + public static List collectContents(FormData attachment) { + List result = new ArrayList<>(MAX_FILES_TO_INSPECT); + for (String key : attachment) { + for (FormData.FormValue formValue : attachment.get(key)) { + if (result.size() >= MAX_FILES_TO_INSPECT) { + return result; + } + try { + // null means no filename attribute → form field → skip + if (formValue.getFileName() == null) { + continue; + } + result.add(readContent(formValue)); + } catch (Exception e) { + log.debug("Failed to process form value", e); + } + } + } + return result; + } + + @VisibleForTesting + static String readContent(FormData.FormValue formValue) { + String contentType = null; + HeaderMap headers = formValue.getHeaders(); + if (headers != null) { + contentType = headers.getFirst(Headers.CONTENT_TYPE); + } + + // Try getPath() first: works for 2.0 (all files on disk) and 2.2+ disk files. + try { + Path path = formValue.getPath(); + try (InputStream is = Files.newInputStream(path)) { + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } + } catch (Exception ignored) { + // In Undertow 2.2+, in-memory uploads throw here (no path). + } + + // Fallback for Undertow 2.2+ in-memory uploads via cached reflection. + if (GET_FILE_ITEM != null && FILE_ITEM_GET_INPUT_STREAM != null) { + try { + Object fileItem = GET_FILE_ITEM.invoke(formValue); + if (fileItem != null) { + try (InputStream is = (InputStream) FILE_ITEM_GET_INPUT_STREAM.invoke(fileItem)) { + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } + } + } catch (Exception e) { + log.debug("Failed to read in-memory upload via reflection", e); + } + } + + return ""; + } + + private FormDataContentHelper() {} +} diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java index 52bd0b73897..d811682eeca 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java @@ -53,7 +53,7 @@ public Reference[] additionalMuzzleReferences() { @Override public String[] helperClassNames() { - return new String[] {packageName + ".FormDataMap"}; + return new String[] {packageName + ".FormDataMap", packageName + ".FormDataContentHelper"}; } public void methodAdvice(MethodTransformer transformer) { @@ -87,7 +87,9 @@ static void after( cbp.getCallback(EVENTS.requestBodyProcessed()); BiFunction, Flow> filenamesCb = cbp.getCallback(EVENTS.requestFilesFilenames()); - if (bodyCallback == null && filenamesCb == null) { + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (bodyCallback == null && filenamesCb == null && contentCb == null) { return; } FormData attachment = exchange.getAttachment(FORM_DATA); @@ -139,6 +141,25 @@ static void after( } } } + + if (contentCb != null && t == null) { + List filesContent = FormDataContentHelper.collectContents(attachment); + if (!filesContent.isEmpty()) { + Flow contentFlow = contentCb.apply(reqCtx, filesContent); + Flow.Action contentAction = contentFlow.getAction(); + if (contentAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = + (Flow.Action.RequestBlockingAction) contentAction; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null && t == null) { + boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (success) { + t = new BlockingException("Blocked request (multipart file upload content)"); + } + } + } + } + } } } } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy index 6fc47cd0e15..7ca4cb0cfd8 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy @@ -201,6 +201,11 @@ abstract class UndertowServletTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + @Override boolean testBlockingOnResponse() { true diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/java/datadog/trace/instrumentation/undertow/FormDataContentHelperTest.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/java/datadog/trace/instrumentation/undertow/FormDataContentHelperTest.java new file mode 100644 index 00000000000..5f5ef6b1499 --- /dev/null +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/java/datadog/trace/instrumentation/undertow/FormDataContentHelperTest.java @@ -0,0 +1,162 @@ +package datadog.trace.instrumentation.undertow; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.undertow.server.handlers.form.FormData; +import io.undertow.util.HeaderMap; +import java.io.IOException; +import java.lang.reflect.Field; +import java.lang.reflect.Proxy; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayDeque; +import java.util.Arrays; +import java.util.Deque; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class FormDataContentHelperTest { + + @TempDir Path tempDir; + + @Test + void diskFile_contentRead() throws IOException { + byte[] content = "file content".getBytes(StandardCharsets.ISO_8859_1); + Path file = Files.createTempFile(tempDir, "upload", ".bin"); + Files.write(file, content); + + FormData fd = new FormData(10); + fd.add("upload", file, "test.bin", new HeaderMap()); + + List result = FormDataContentHelper.collectContents(fd); + assertEquals(1, result.size()); + assertEquals("file content", result.get(0)); + } + + @Test + void formField_skipped() throws IOException { + FormData fd = new FormData(10); + fd.add("name", "John"); + + List result = FormDataContentHelper.collectContents(fd); + assertTrue(result.isEmpty()); + } + + @Test + void emptyFilename_contentRead() throws Exception { + // filename="" means a file upload with no filename attribute — content still inspected + byte[] content = "data".getBytes(StandardCharsets.ISO_8859_1); + Path file = Files.createTempFile(tempDir, "upload", ".bin"); + Files.write(file, content); + + FormData fd = new FormData(10); + fd.add("upload", file, "", new HeaderMap()); + + List result = FormDataContentHelper.collectContents(fd); + assertEquals(1, result.size()); + assertEquals("data", result.get(0)); + } + + @Test + void truncation_atMaxContentBytes() throws IOException { + byte[] content = new byte[FormDataContentHelper.MAX_CONTENT_BYTES + 500]; + Arrays.fill(content, (byte) 'X'); + Path file = Files.createTempFile(tempDir, "upload", ".bin"); + Files.write(file, content); + + FormData fd = new FormData(10); + fd.add("upload", file, "big.bin", new HeaderMap()); + + List result = FormDataContentHelper.collectContents(fd); + assertEquals(1, result.size()); + assertEquals(FormDataContentHelper.MAX_CONTENT_BYTES, result.get(0).length()); + } + + @Test + void maxFilesLimit_enforced() throws IOException { + FormData fd = new FormData(100); + int limit = FormDataContentHelper.MAX_FILES_TO_INSPECT; + for (int i = 0; i < limit + 1; i++) { + Path file = Files.createTempFile(tempDir, "f" + i, ".bin"); + Files.write(file, ("content_" + i).getBytes(StandardCharsets.ISO_8859_1)); + fd.add("file" + i, file, "f" + i + ".bin", new HeaderMap()); + } + + List result = FormDataContentHelper.collectContents(fd); + assertEquals(limit, result.size()); + } + + @Test + void mixedFieldsAndFiles() throws IOException { + Path file = Files.createTempFile(tempDir, "upload", ".bin"); + Files.write(file, "file content".getBytes(StandardCharsets.ISO_8859_1)); + + FormData fd = new FormData(10); + fd.add("name", "John"); + fd.add("upload", file, "test.bin", new HeaderMap()); + + List result = FormDataContentHelper.collectContents(fd); + assertEquals(1, result.size()); + assertEquals("file content", result.get(0)); + } + + @Test + void inMemoryFile_contentRead_viaProxy() throws Exception { + // Simulate an in-memory file upload (Undertow 2.2+) using a Proxy. + // getPath() throws IllegalStateException for in-memory uploads; the helper + // falls back to getFileItem().getInputStream() via reflection. + byte[] content = "in-memory content".getBytes(StandardCharsets.ISO_8859_1); + + FormData fd = new FormData(10); + addInMemoryFileValue(fd, "upload", "mem.bin", content); + + // The reflection fallback won't work in a test environment where FileItem + // isn't actually loaded, so we just verify the helper doesn't throw and + // returns either the content (if reflection works) or "" (graceful fallback). + List result = FormDataContentHelper.collectContents(fd); + assertEquals(1, result.size()); + // Either the reflection path worked or the graceful fallback returned "" + assertTrue(result.get(0).equals("in-memory content") || result.get(0).isEmpty()); + } + + @SuppressWarnings("unchecked") + private static void addInMemoryFileValue( + FormData fd, String name, String filename, byte[] content) throws Exception { + Field valuesField = FormData.class.getDeclaredField("values"); + valuesField.setAccessible(true); + Map> values = + (Map>) valuesField.get(fd); + + // Use a Proxy so this compiles against Undertow 2.0 and also works against 2.2.x. + // getPath() throws to simulate an in-memory upload. + FormData.FormValue inMemory = + (FormData.FormValue) + Proxy.newProxyInstance( + FormData.FormValue.class.getClassLoader(), + new Class[] {FormData.FormValue.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getFileName": + return filename; + case "getHeaders": + return new HeaderMap(); + case "getPath": + throw new IllegalStateException("in-memory upload has no path"); + case "isFile": + case "isFileItem": + case "isBigField": + return false; + default: + return null; + } + }); + + Deque deque = new ArrayDeque<>(); + deque.add(inMemory); + values.put(name, deque); + } +} diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy b/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy index c7a1924eb12..bb6fd2289c6 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy +++ b/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy @@ -198,6 +198,11 @@ class UndertowServletTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + boolean hasResponseSpan(ServerEndpoint endpoint) { // FIXME: re-enable when jakarta servlet will be fully supported // return endpoint == REDIRECT || endpoint == NOT_FOUND From 4924aeca5e15ee6d3501a3b7237e0bec54e370e6 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 24 Jun 2026 13:42:17 +0200 Subject: [PATCH 2/4] fix: use Class.forName(name, false, loader) to avoid forbidden API violation --- .../trace/instrumentation/play25/appsec/BodyParserHelpers.java | 2 +- .../trace/instrumentation/play26/appsec/BodyParserHelpers.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java index 0df0e62b9c1..8789a03d8b1 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java @@ -84,7 +84,7 @@ public class BodyParserHelpers { } catch (Exception ignored) { } try { - file = Class.forName("play.api.libs.Files$TemporaryFile").getMethod("file"); + file = Class.forName("play.api.libs.Files$TemporaryFile", false, BodyParserHelpers.class.getClassLoader()).getMethod("file"); } catch (Exception ignored) { } FILE_PART_REF = ref; diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java index d0ee2e49470..e9b51c93cad 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java @@ -94,7 +94,7 @@ public class BodyParserHelpers { } catch (Exception ignored) { } try { - file = Class.forName("play.api.libs.Files$TemporaryFile").getMethod("file"); + file = Class.forName("play.api.libs.Files$TemporaryFile", false, BodyParserHelpers.class.getClassLoader()).getMethod("file"); } catch (Exception ignored) { } FILE_PART_REF = ref; From b5928a55ceecdad472ed8e50a9c74b7b77fb4607 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 24 Jun 2026 13:47:28 +0200 Subject: [PATCH 3/4] chore: spotless format Class.forName call --- .../instrumentation/play25/appsec/BodyParserHelpers.java | 7 ++++++- .../instrumentation/play26/appsec/BodyParserHelpers.java | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java index 8789a03d8b1..0eba7744dff 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpers.java @@ -84,7 +84,12 @@ public class BodyParserHelpers { } catch (Exception ignored) { } try { - file = Class.forName("play.api.libs.Files$TemporaryFile", false, BodyParserHelpers.class.getClassLoader()).getMethod("file"); + file = + Class.forName( + "play.api.libs.Files$TemporaryFile", + false, + BodyParserHelpers.class.getClassLoader()) + .getMethod("file"); } catch (Exception ignored) { } FILE_PART_REF = ref; diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java index e9b51c93cad..4f1fff4b5fa 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java @@ -94,7 +94,12 @@ public class BodyParserHelpers { } catch (Exception ignored) { } try { - file = Class.forName("play.api.libs.Files$TemporaryFile", false, BodyParserHelpers.class.getClassLoader()).getMethod("file"); + file = + Class.forName( + "play.api.libs.Files$TemporaryFile", + false, + BodyParserHelpers.class.getClassLoader()) + .getMethod("file"); } catch (Exception ignored) { } FILE_PART_REF = ref; From e2d4b8176da9bf9f5811723d2bb3802087657d8e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 24 Jun 2026 14:41:26 +0200 Subject: [PATCH 4/4] fix: increase multipart maxFileSize/maxRequestSize in Undertow test servers The default limit of 1024 bytes caused RequestTooBigException for the truncation test (4096+500 bytes) and max-files test. Unlimited (-1) allows the content tests to reach the multipart handler. --- .../undertow-2.0/src/test/groovy/UndertowServletTest.groovy | 2 +- .../undertow-2.2/src/test/groovy/UndertowServletTest.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy index 7ca4cb0cfd8..846389417ba 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy @@ -47,7 +47,7 @@ abstract class UndertowServletTest extends HttpServerTest { final ServletContainer container = ServletContainer.Factory.newInstance() DeploymentInfo builder = new DeploymentInfo() - .setDefaultMultipartConfig(new MultipartConfigElement(System.getProperty('java.io.tmpdir'), 1024, 1024, 1024)) + .setDefaultMultipartConfig(new MultipartConfigElement(System.getProperty('java.io.tmpdir'), -1, -1, 1024)) .setClassLoader(UndertowServletTest.getClassLoader()) .setContextPath("/$CONTEXT") .setDeploymentName("servletContext.war") diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy b/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy index bb6fd2289c6..4287b8a8ebe 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy +++ b/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowServletTest.groovy @@ -40,7 +40,7 @@ class UndertowServletTest extends HttpServerTest { final ServletContainer container = ServletContainer.Factory.newInstance() DeploymentInfo builder = new DeploymentInfo() - .setDefaultMultipartConfig(new MultipartConfigElement(System.getProperty('java.io.tmpdir'), 1024, 1024, 1024)) + .setDefaultMultipartConfig(new MultipartConfigElement(System.getProperty('java.io.tmpdir'), -1, -1, 1024)) .setClassLoader(UndertowServletTest.getClassLoader()) .setContextPath("/$CONTEXT") .setDeploymentName("servletContext.war")