Fix tryCommitBlockingResponse return-value check; simplify appsec guard; add play-2.5 unit tests

Author: jandro996

dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpersTest.java

@@ -0,0 +1,103 @@
+package datadog.trace.instrumentation.play25.appsec;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.math.BigDecimal;
+import java.util.List;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+import play.api.libs.json.JsValue;
+
+class BodyParserHelpersTest {
+
+  private static JsValue parse(String json) {
+    return play.api.libs.json.Json$.MODULE$.parse(json);
+  }
+
+  @Test
+  void jsValueToJavaObject_nullInputReturnsNull() {
+    assertNull(BodyParserHelpers.jsValueToJavaObject(null));
+  }
+
+  @Test
+  void jsValueToJavaObject_jsNullReturnsNull() {
+    assertNull(BodyParserHelpers.jsValueToJavaObject(parse("null")));
+  }
+
+  @Test
+  void jsValueToJavaObject_string() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("\"hello\""));
+    assertEquals("hello", result);
+  }
+
+  @Test
+  void jsValueToJavaObject_number() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("42"));
+    assertTrue(result instanceof BigDecimal);
+    assertEquals(0, ((BigDecimal) result).compareTo(new BigDecimal("42")));
+  }
+
+  @Test
+  void jsValueToJavaObject_booleanTrue() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("true"));
+    assertEquals(Boolean.TRUE, result);
+  }
+
+  @Test
+  void jsValueToJavaObject_booleanFalse() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("false"));
+    assertEquals(Boolean.FALSE, result);
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  void jsValueToJavaObject_object() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("{\"key\":\"value\",\"num\":1}"));
+    assertTrue(result instanceof Map);
+    Map<String, Object> map = (Map<String, Object>) result;
+    assertEquals("value", map.get("key"));
+    assertTrue(map.get("num") instanceof BigDecimal);
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  void jsValueToJavaObject_array() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("[\"a\",\"b\",\"c\"]"));
+    assertTrue(result instanceof List);
+    List<Object> list = (List<Object>) result;
+    assertEquals(3, list.size());
+    assertEquals("a", list.get(0));
+    assertEquals("b", list.get(1));
+    assertEquals("c", list.get(2));
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  void jsValueToJavaObject_nestedObject() {
+    Object result =
+        BodyParserHelpers.jsValueToJavaObject(parse("{\"outer\":{\"inner\":\"deep\"}}"));
+    assertTrue(result instanceof Map);
+    Map<String, Object> outer = (Map<String, Object>) result;
+    assertTrue(outer.get("outer") instanceof Map);
+    Map<String, Object> inner = (Map<String, Object>) outer.get("outer");
+    assertEquals("deep", inner.get("inner"));
+  }
+
+  @Test
+  void jsValueToJavaObject_zeroRecursionReturnsNull() {
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("{\"key\":\"value\"}"), 0);
+    assertNull(result);
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  void jsValueToJavaObject_recursionLimitTruncatesNesting() {
+    // depth=1: outer object is converted, but children exceed the limit and become null
+    Object result = BodyParserHelpers.jsValueToJavaObject(parse("{\"a\":{\"b\":\"val\"}}"), 1);
+    assertTrue(result instanceof Map);
+    Map<String, Object> map = (Map<String, Object>) result;
+    assertNull(map.get("a"));
+  }
+}

dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java

@@ -142,14 +142,11 @@ private static MultipartFormData<?> handleMultipartFormData(MultipartFormData<?>
   }
 
   private static void handleMultipartFilenames(Iterator<?> iterator) {
-    AgentSpan span = activeSpan();
-    if (span == null) {
+    if (!isAppsecActiveForRequest()) {
       return;
     }
+    AgentSpan span = activeSpan();
     RequestContext reqCtx = span.getRequestContext();
-    if (reqCtx == null || reqCtx.getData(RequestContextSlot.APPSEC) == null) {
-      return;
-    }
 
     List<String> filenames = new ArrayList<>();
     while (iterator.hasNext()) {

dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java

@@ -102,8 +102,9 @@ static void after(
           Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
           BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
           if (blockResponseFunction != null) {
-            blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
-            if (t == null) {
+            boolean success =
+                blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
+            if (success && t == null) {
               t =
                   new BlockingException(
                       "Blocked request (for MultiPartUploadHandler/parseBlocking)");
@@ -130,8 +131,10 @@ static void after(
                 (Flow.Action.RequestBlockingAction) filenamesAction;
             BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
             if (brf != null) {
-              brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
-              t = new BlockingException("Blocked request (multipart file upload)");
+              boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
+              if (success) {
+                t = new BlockingException("Blocked request (multipart file upload)");
+              }
             }
           }
         }