Bound multipart text/plain field materialization in Jersey AppSec

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

@@ -28,11 +28,6 @@ public static void collectBodyPart(
       Map<String, List<String>> bodyMap,
       List<String> filenames,
       List<String> filesContent) {
-    if (bodyMap != null
-        && MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())) {
-      // BodyPartEntity allows re-reading the part without consuming the stream
-      bodyMap.computeIfAbsent(bodyPart.getName(), k -> new ArrayList<>()).add(bodyPart.getValue());
-    }
     FormDataContentDisposition cd;
     try {
       cd = bodyPart.getFormDataContentDisposition();
@@ -41,6 +36,16 @@ public static void collectBodyPart(
       // so a single bad part does not abort processing of the remaining parts.
       cd = null;
     }
+    if (bodyMap != null
+        && cd != null
+        && MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())
+        && totalBodyMapValues(bodyMap) < MAX_FILES_TO_INSPECT) {
+      // readContent() reads the part through a byte-capped decoder (MAX_CONTENT_BYTES) instead of
+      // the unbounded getValue(); the cap counts total accumulated values across all field names,
+      // not distinct keys, so repeating the same field name cannot bypass the limit. cd.getName()
+      // is a safe field accessor, unlike bodyPart.getName() which re-parses the disposition header.
+      bodyMap.computeIfAbsent(cd.getName(), k -> new ArrayList<>()).add(readContent(bodyPart));
+    }
     // rawFilename == null  → no filename attribute → form field → skip filenames and content
     // rawFilename == ""    → filename attribute present but empty → content YES, filenames NO
     // rawFilename != ""    → file with name → both
@@ -53,21 +58,37 @@ public static void collectBodyPart(
     }
   }
 
+  private static int totalBodyMapValues(Map<String, List<String>> bodyMap) {
+    int total = 0;
+    for (List<String> values : bodyMap.values()) {
+      total += values.size();
+    }
+    return total;
+  }
+
   public static String readContent(FormDataBodyPart bodyPart) {
     try {
       // getEntityAs(InputStream.class) is backed by BodyPartEntity which supports re-reading:
       // each call creates a fresh stream from the buffered MIME part data.
       try (InputStream is = bodyPart.getEntityAs(InputStream.class)) {
         if (is == null) return "";
-        String contentType =
-            bodyPart.getMediaType() != null ? bodyPart.getMediaType().toString() : null;
-        return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType);
+        return MultipartContentDecoder.readInputStream(
+            is, MAX_CONTENT_BYTES, contentTypeWithDefaultUtf8(bodyPart.getMediaType()));
       }
     } catch (IOException ignored) {
       return "";
     }
   }
 
+  // Jersey's own getValue() decodes undeclared-charset text parts as UTF-8; match that default
+  // here instead of falling through to MultipartContentDecoder's platform-dependent fallback.
+  private static String contentTypeWithDefaultUtf8(MediaType mediaType) {
+    String contentType = mediaType != null ? mediaType.toString() : null;
+    return MultipartContentDecoder.extractCharset(contentType) == null
+        ? (contentType == null ? "charset=UTF-8" : contentType + "; charset=UTF-8")
+        : contentType;
+  }
+
   public static BlockingException tryBlock(RequestContext ctx, Flow<Void> flow, String message) {
     Flow.Action action = flow.getAction();
     if (action instanceof Flow.Action.RequestBlockingAction) {

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

@@ -16,11 +16,12 @@ class MultiPartHelperTest extends Specification {
 
   def "text/plain part is added to body map"() {
     given:
+    def cd = Mock(FormDataContentDisposition)
+    cd.getName() >> 'field'
     def bodyPart = Mock(FormDataBodyPart)
     bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
-    bodyPart.getName() >> 'field'
-    bodyPart.getValue() >> 'value'
-    bodyPart.getFormDataContentDisposition() >> null
+    bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('value'.bytes)
+    bodyPart.getFormDataContentDisposition() >> cd
     def map = [:]
 
     when:
@@ -56,11 +57,12 @@ class MultiPartHelperTest extends Specification {
 
   def "multiple values for same field are accumulated"() {
     given:
+    def cd = Mock(FormDataContentDisposition)
+    cd.getName() >> 'tag'
     def bodyPart = Mock(FormDataBodyPart)
     bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
-    bodyPart.getName() >> 'tag'
-    bodyPart.getValue() >>> ['a', 'b']
-    bodyPart.getFormDataContentDisposition() >> null
+    bodyPart.getEntityAs(InputStream) >>> [new ByteArrayInputStream('a'.bytes), new ByteArrayInputStream('b'.bytes)]
+    bodyPart.getFormDataContentDisposition() >> cd
     def map = [:]
 
     when:
@@ -71,6 +73,134 @@ class MultiPartHelperTest extends Specification {
     map == [tag: ['a', 'b']]
   }
 
+  def "text/plain field value longer than MAX_CONTENT_BYTES is truncated"() {
+    given:
+    def longValue = 'a' * (MultiPartHelper.MAX_CONTENT_BYTES + 100)
+    def cd = Mock(FormDataContentDisposition)
+    cd.getName() >> 'field'
+    def bodyPart = Mock(FormDataBodyPart)
+    bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+    bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream(longValue.bytes)
+    bodyPart.getFormDataContentDisposition() >> cd
+    def map = [:]
+
+    when:
+    MultiPartHelper.collectBodyPart(bodyPart, map, null, null)
+
+    then:
+    map['field'][0] == 'a' * MultiPartHelper.MAX_CONTENT_BYTES
+  }
+
+  def "text/plain part with no declared charset decodes non-ASCII content as UTF-8"() {
+    given:
+    def cd = Mock(FormDataContentDisposition)
+    cd.getName() >> 'field'
+    def bodyPart = Mock(FormDataBodyPart)
+    bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+    bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('héllo wörld'.getBytes('UTF-8'))
+    bodyPart.getFormDataContentDisposition() >> cd
+    def map = [:]
+
+    when:
+    MultiPartHelper.collectBodyPart(bodyPart, map, null, null)
+
+    then:
+    map == [field: ['héllo wörld']]
+  }
+
+  def "text/plain part with explicit non-UTF-8 charset decodes using that charset"() {
+    given:
+    def cd = Mock(FormDataContentDisposition)
+    cd.getName() >> 'field'
+    def mediaType = new MediaType('text', 'plain', [charset: 'ISO-8859-1'])
+    def bodyPart = Mock(FormDataBodyPart)
+    bodyPart.getMediaType() >> mediaType
+    bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('café'.getBytes('ISO-8859-1'))
+    bodyPart.getFormDataContentDisposition() >> cd
+    def map = [:]
+
+    when:
+    MultiPartHelper.collectBodyPart(bodyPart, map, null, null)
+
+    then:
+    map == [field: ['café']]
+  }
+
+  def "MAX_FILES_TO_INSPECT limits number of distinct body map field names"() {
+    given:
+    def parts = (1..MultiPartHelper.MAX_FILES_TO_INSPECT + 2).collect { i ->
+      def cd = Mock(FormDataContentDisposition)
+      cd.getName() >> "field${i}"
+      def bp = Mock(FormDataBodyPart)
+      bp.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+      bp.getEntityAs(InputStream) >> new ByteArrayInputStream("value${i}".bytes)
+      bp.getFormDataContentDisposition() >> cd
+      bp
+    }
+    def map = [:]
+
+    when:
+    parts.each { MultiPartHelper.collectBodyPart(it, map, null, null) }
+
+    then:
+    map.size() == MultiPartHelper.MAX_FILES_TO_INSPECT
+  }
+
+  def "MAX_FILES_TO_INSPECT limits total accumulated values, even for a repeated field name"() {
+    given: "the map filled up to the cap with distinct field names"
+    def existing = (1..MultiPartHelper.MAX_FILES_TO_INSPECT).collect { i ->
+      def cd = Mock(FormDataContentDisposition)
+      cd.getName() >> "field${i}"
+      def bp = Mock(FormDataBodyPart)
+      bp.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+      bp.getEntityAs(InputStream) >> { new ByteArrayInputStream("value${i}".bytes) }
+      bp.getFormDataContentDisposition() >> cd
+      bp
+    }
+    def map = [:]
+    existing.each { MultiPartHelper.collectBodyPart(it, map, null, null) }
+
+    and: "a body part reusing an already-collected field name"
+    def repeatCd = Mock(FormDataContentDisposition)
+    repeatCd.getName() >> 'field1'
+    def repeat = Mock(FormDataBodyPart)
+    repeat.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+    repeat.getEntityAs(InputStream) >> new ByteArrayInputStream('extra'.bytes)
+    repeat.getFormDataContentDisposition() >> repeatCd
+
+    and: "a body part with a brand-new field name"
+    def freshCd = Mock(FormDataContentDisposition)
+    freshCd.getName() >> 'brandNew'
+    def fresh = Mock(FormDataBodyPart)
+    fresh.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+    fresh.getEntityAs(InputStream) >> new ByteArrayInputStream('nope'.bytes)
+    fresh.getFormDataContentDisposition() >> freshCd
+
+    when:
+    MultiPartHelper.collectBodyPart(repeat, map, null, null)
+    MultiPartHelper.collectBodyPart(fresh, map, null, null)
+
+    then: "the total value cap rejects both the repeated field's extra value and the brand-new field"
+    map['field1'] == ['value1']
+    map.size() == MultiPartHelper.MAX_FILES_TO_INSPECT
+    !map.containsKey('brandNew')
+  }
+
+  def "malformed Content-Disposition on a text/plain part skips body map gracefully"() {
+    given:
+    def bodyPart = Mock(FormDataBodyPart)
+    bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
+    bodyPart.getFormDataContentDisposition() >> { throw new IllegalArgumentException("bad CD") }
+    def map = [:]
+
+    when:
+    MultiPartHelper.collectBodyPart(bodyPart, map, null, null)
+
+    then:
+    map.isEmpty()
+    noExceptionThrown()
+  }
+
   // collectBodyPart — filenames
 
   def "filename is added to list when present"() {
@@ -96,7 +226,7 @@ class MultiPartHelperTest extends Specification {
     def bodyPart = Mock(FormDataBodyPart)
     bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
     bodyPart.getName() >> 'f'
-    bodyPart.getValue() >> 'v'
+    bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('v'.bytes)
     bodyPart.getFormDataContentDisposition() >> cd
 
     expect:
@@ -107,10 +237,10 @@ class MultiPartHelperTest extends Specification {
     given:
     def cd = Mock(FormDataContentDisposition)
     cd.getFileName() >> 'upload.txt'
+    cd.getName() >> 'file'
     def bodyPart = Mock(FormDataBodyPart)
     bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE
-    bodyPart.getName() >> 'file'
-    bodyPart.getValue() >> 'content'
+    bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('content'.bytes)
     bodyPart.getFormDataContentDisposition() >> cd
     def map = [:]
     def filenames = []

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

@@ -28,11 +28,6 @@ public static void collectBodyPart(
       Map<String, List<String>> bodyMap,
       List<String> filenames,
       List<String> filesContent) {
-    if (bodyMap != null
-        && MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())) {
-      // BodyPartEntity allows re-reading the part without consuming the stream
-      bodyMap.computeIfAbsent(bodyPart.getName(), k -> new ArrayList<>()).add(bodyPart.getValue());
-    }
     FormDataContentDisposition cd;
     try {
       cd = bodyPart.getFormDataContentDisposition();
@@ -41,6 +36,16 @@ public static void collectBodyPart(
       // so a single bad part does not abort processing of the remaining parts.
       cd = null;
     }
+    if (bodyMap != null
+        && cd != null
+        && MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())
+        && totalBodyMapValues(bodyMap) < MAX_FILES_TO_INSPECT) {
+      // readContent() reads the part through a byte-capped decoder (MAX_CONTENT_BYTES) instead of
+      // the unbounded getValue(); the cap counts total accumulated values across all field names,
+      // not distinct keys, so repeating the same field name cannot bypass the limit. cd.getName()
+      // is a safe field accessor, unlike bodyPart.getName() which re-parses the disposition header.
+      bodyMap.computeIfAbsent(cd.getName(), k -> new ArrayList<>()).add(readContent(bodyPart));
+    }
     // rawFilename == null  → no filename attribute → form field → skip filenames and content
     // rawFilename == ""    → filename attribute present but empty → content YES, filenames NO
     // rawFilename != ""    → file with name → both
@@ -53,21 +58,37 @@ public static void collectBodyPart(
     }
   }
 
+  private static int totalBodyMapValues(Map<String, List<String>> bodyMap) {
+    int total = 0;
+    for (List<String> values : bodyMap.values()) {
+      total += values.size();
+    }
+    return total;
+  }
+
   public static String readContent(FormDataBodyPart bodyPart) {
     try {
       // getEntityAs(InputStream.class) is backed by BodyPartEntity which supports re-reading:
       // each call creates a fresh stream from the buffered MIME part data.
       try (InputStream is = bodyPart.getEntityAs(InputStream.class)) {
         if (is == null) return "";
-        String contentType =
-            bodyPart.getMediaType() != null ? bodyPart.getMediaType().toString() : null;
-        return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType);
+        return MultipartContentDecoder.readInputStream(
+            is, MAX_CONTENT_BYTES, contentTypeWithDefaultUtf8(bodyPart.getMediaType()));
       }
     } catch (IOException ignored) {
       return "";
     }
   }
 
+  // Jersey's own getValue() decodes undeclared-charset text parts as UTF-8; match that default
+  // here instead of falling through to MultipartContentDecoder's platform-dependent fallback.
+  private static String contentTypeWithDefaultUtf8(MediaType mediaType) {
+    String contentType = mediaType != null ? mediaType.toString() : null;
+    return MultipartContentDecoder.extractCharset(contentType) == null
+        ? (contentType == null ? "charset=UTF-8" : contentType + "; charset=UTF-8")
+        : contentType;
+  }
+
   public static BlockingException tryBlock(RequestContext ctx, Flow<Void> flow, String message) {
     Flow.Action action = flow.getAction();
     if (action instanceof Flow.Action.RequestBlockingAction) {