From 230cb87aba9b994f6d67bb11eecc8ff28f32eb50 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 11:57:29 +0200 Subject: [PATCH 01/21] Rename ContextInterpreter.clientIpWithoutAppSec to collectClientIp --- .../datadog/trace/core/propagation/ContextInterpreter.java | 6 +++--- .../java/datadog/trace/api/{appsec => http}/MediaType.java | 0 .../api/{appsec => http}/MediaTypeSpecification.groovy | 0 3 files changed, 3 insertions(+), 3 deletions(-) rename internal-api/src/main/java/datadog/trace/api/{appsec => http}/MediaType.java (100%) rename internal-api/src/test/groovy/datadog/trace/api/{appsec => http}/MediaTypeSpecification.groovy (100%) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java index 14a6459ca0f..ea63f83e702 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java @@ -66,7 +66,7 @@ public abstract class ContextInterpreter implements AgentPropagation.KeyClassifi private TagContext.HttpHeaders httpHeaders; private final String customIpHeaderName; private final boolean clientIpResolutionEnabled; - private final boolean clientIpWithoutAppSec; + private final boolean collectClientIp; private final boolean aiGuardEnabled; private boolean collectIpHeaders; private final boolean requestHeaderTagsCommaAllowed; @@ -83,7 +83,7 @@ protected static String toLowerCase(String key) { protected ContextInterpreter(Config config) { this.customIpHeaderName = config.getTraceClientIpHeader(); this.clientIpResolutionEnabled = config.isTraceClientIpResolverEnabled(); - this.clientIpWithoutAppSec = config.isClientIpEnabled(); + this.collectClientIp = config.isClientIpEnabled(); this.aiGuardEnabled = config.isAiGuardEnabled(); this.propagationTagsFactory = PropagationTags.factory(config); this.requestHeaderTagsCommaAllowed = config.isRequestHeaderTagsCommaAllowed(); @@ -278,7 +278,7 @@ public ContextInterpreter reset(TraceConfig traceConfig) { fullContext = true; httpHeaders = null; collectIpHeaders = - this.clientIpWithoutAppSec + this.collectClientIp || this.clientIpResolutionEnabled && (ActiveSubsystems.APPSEC_ACTIVE || this.aiGuardEnabled); headerTags = traceConfig.getRequestHeaderTags(); diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/MediaType.java b/internal-api/src/main/java/datadog/trace/api/http/MediaType.java similarity index 100% rename from internal-api/src/main/java/datadog/trace/api/appsec/MediaType.java rename to internal-api/src/main/java/datadog/trace/api/http/MediaType.java diff --git a/internal-api/src/test/groovy/datadog/trace/api/appsec/MediaTypeSpecification.groovy b/internal-api/src/test/groovy/datadog/trace/api/http/MediaTypeSpecification.groovy similarity index 100% rename from internal-api/src/test/groovy/datadog/trace/api/appsec/MediaTypeSpecification.groovy rename to internal-api/src/test/groovy/datadog/trace/api/http/MediaTypeSpecification.groovy From 978847463b02daeedca7879d4969d368ac9f8961 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 11:58:48 +0200 Subject: [PATCH 02/21] Move MediaType out of the appsec package into datadog.trace.api.http MediaType is a general-purpose media type parsing utility with no AppSec specific logic, so it does not belong under datadog.trace.api.appsec. --- .../src/main/java/com/datadog/appsec/util/BodyParser.java | 2 +- .../datadog/appsec/gateway/GatewayBridgeSpecification.groovy | 2 +- .../trace/instrumentation/okhttp2/AppSecInterceptor.java | 2 +- .../trace/instrumentation/okhttp3/AppSecInterceptor.java | 2 +- .../main/java/datadog/trace/lambda/ContentTypeBodyParser.java | 2 +- .../src/main/java/datadog/trace/lambda/LambdaEventParser.java | 2 +- .../main/java/datadog/trace/api/appsec/HttpClientPayload.java | 1 + .../src/main/java/datadog/trace/api/http/MediaType.java | 2 +- .../groovy/datadog/trace/api/http/MediaTypeSpecification.groovy | 2 +- 9 files changed, 9 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/util/BodyParser.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/util/BodyParser.java index f7f8a4ce668..159ec8c9f4b 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/util/BodyParser.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/util/BodyParser.java @@ -8,7 +8,7 @@ import com.squareup.moshi.JsonDataException; import com.squareup.moshi.JsonReader; import com.squareup.moshi.JsonWriter; -import datadog.trace.api.appsec.MediaType; +import datadog.trace.api.http.MediaType; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index f648fabfc83..0ebdc5f6611 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -14,7 +14,7 @@ import datadog.trace.api.ProductTraceSource import datadog.trace.api.TagMap import datadog.trace.api.appsec.HttpClientRequest import datadog.trace.api.appsec.HttpClientResponse -import datadog.trace.api.appsec.MediaType +import datadog.trace.api.http.MediaType import datadog.trace.api.config.GeneralConfig import datadog.trace.api.function.TriConsumer import datadog.trace.api.function.TriFunction diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java index 82b3e0189ad..2a6a094fd90 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java +++ b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java @@ -14,12 +14,12 @@ import datadog.trace.api.appsec.HttpClientPayload; import datadog.trace.api.appsec.HttpClientRequest; import datadog.trace.api.appsec.HttpClientResponse; -import datadog.trace.api.appsec.MediaType; 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.MediaType; import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java index 7cfecfbebb1..9d07c14d1f5 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java +++ b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java @@ -8,12 +8,12 @@ import datadog.trace.api.appsec.HttpClientPayload; import datadog.trace.api.appsec.HttpClientRequest; import datadog.trace.api.appsec.HttpClientResponse; -import datadog.trace.api.appsec.MediaType; 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.MediaType; import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/ContentTypeBodyParser.java b/dd-trace-core/src/main/java/datadog/trace/lambda/ContentTypeBodyParser.java index abfc0e20585..0f869f2fd71 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/ContentTypeBodyParser.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/ContentTypeBodyParser.java @@ -1,6 +1,6 @@ package datadog.trace.lambda; -import datadog.trace.api.appsec.MediaType; +import datadog.trace.api.http.MediaType; import datadog.trace.lambda.MultipartSplitter.Part; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java index 6dfcca2729c..86aa6999eae 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java @@ -5,7 +5,7 @@ import com.squareup.moshi.JsonAdapter; import com.squareup.moshi.Moshi; import datadog.trace.api.Config; -import datadog.trace.api.appsec.MediaType; +import datadog.trace.api.http.MediaType; import datadog.trace.lambda.ContentTypeBodyParser.ParseContext; import java.io.ByteArrayInputStream; import java.io.IOException; diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/HttpClientPayload.java b/internal-api/src/main/java/datadog/trace/api/appsec/HttpClientPayload.java index 9be785a1037..0070750f7e7 100644 --- a/internal-api/src/main/java/datadog/trace/api/appsec/HttpClientPayload.java +++ b/internal-api/src/main/java/datadog/trace/api/appsec/HttpClientPayload.java @@ -1,5 +1,6 @@ package datadog.trace.api.appsec; +import datadog.trace.api.http.MediaType; import java.io.InputStream; import java.util.List; import java.util.Map; diff --git a/internal-api/src/main/java/datadog/trace/api/http/MediaType.java b/internal-api/src/main/java/datadog/trace/api/http/MediaType.java index 979562160c8..8437f2c859b 100644 --- a/internal-api/src/main/java/datadog/trace/api/http/MediaType.java +++ b/internal-api/src/main/java/datadog/trace/api/http/MediaType.java @@ -1,4 +1,4 @@ -package datadog.trace.api.appsec; +package datadog.trace.api.http; import java.util.Locale; diff --git a/internal-api/src/test/groovy/datadog/trace/api/http/MediaTypeSpecification.groovy b/internal-api/src/test/groovy/datadog/trace/api/http/MediaTypeSpecification.groovy index b0d7b9462ef..d7b98a9857a 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/http/MediaTypeSpecification.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/http/MediaTypeSpecification.groovy @@ -1,4 +1,4 @@ -package datadog.trace.api.appsec +package datadog.trace.api.http import spock.lang.Specification From 9fbe0403ceb1dd7d8f06bd2c25ec422b95893473 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:00:23 +0200 Subject: [PATCH 03/21] Add reusable AppSec block failure e2e coverage to HttpServerTest --- .../agent/test/base/HttpServerTest.groovy | 143 ++++++++++++++++-- 1 file changed, 134 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 46abd76a6da..b91030cdac3 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -12,6 +12,7 @@ import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.api.ProductActivation +import datadog.trace.api.appsec.AppSecContext import datadog.trace.api.config.GeneralConfig import datadog.trace.api.config.TracerConfig import datadog.trace.api.datastreams.DataStreamsContext @@ -26,6 +27,7 @@ import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.http.StoredBodySupplier import datadog.trace.api.iast.IastContext +import datadog.trace.api.internal.TraceSegment import datadog.trace.api.normalize.SimpleHttpPathNormalizer import datadog.trace.api.rum.RumInjector import datadog.trace.api.telemetry.Endpoint @@ -416,6 +418,21 @@ abstract class HttpServerTest extends WithHttpServer { true } + /** + * Whether the server instrumentation reports a block failure (see {@code + * AppSecContext#reportBlockFailure()}) when the blocking response cannot be committed. Opt in by + * overriding this once the framework call sites go through {@code + * BlockResponseFunction#tryCommitBlockingResponse(RequestContext, RequestBlockingAction)}. + */ + boolean testBlockFailure() { + false + } + + /** The blocking point exercised by the block failure test. */ + BlockFailureVariant blockFailureVariant() { + BlockFailureVariant.REQUEST_HEADERS + } + /** Tomcat 5.5 can't seem to handle the encoded URIs */ boolean testEncodedPath() { true @@ -2072,6 +2089,44 @@ abstract class HttpServerTest extends WithHttpServer { } } + def 'test block failure is reported when the blocking response cannot be committed'() { + setup: + assumeTrue(testBlockFailure()) + def variant = blockFailureVariant() + assumeTrue(variant != BlockFailureVariant.PATH_PARAMS || testPathParam() != null) + IGCallbacks.Context.blockFailureReported = false + + def request = request(variant.endpoint, 'GET', null) + .header(variant.header, variant.headerValue) + .header(IG_BLOCK_FAIL_HEADER, 'true') + .build() + + when: + def response = executeIgnoringIoErrors(request) + + then: 'no blocking response was committed' + response == null || !(response.code() in [301, 413, 418]) + + and: 'the failure to block was reported to the AppSec context' + IGCallbacks.Context.blockFailureReported + } + + /** + * Executes a request that is expected not to produce a blocking response. When the blocking + * response cannot be committed the server may have nothing left to write, so the connection can + * be closed without a complete HTTP response. + */ + protected Response executeIgnoringIoErrors(Request request) { + try { + def response = client.newCall(request).execute() + response.body().bytes() + response.close() + response + } catch (IOException ignored) { + null + } + } + @Flaky(value = "https://github.com/DataDog/dd-trace-java/issues/7061", suites = ["JettyContinuationHandlerV0ForkedTest", "JettyContinuationHandlerV1ForkedTest"]) def 'test blocking of request for request body variant #variant'() { setup: @@ -2699,6 +2754,7 @@ abstract class HttpServerTest extends WithHttpServer { static final String IG_EXTRA_SPAN_NAME_HEADER = "x-ig-write-tags" static final String IG_TEST_HEADER = "x-ig-test-header" static final String IG_BLOCK_HEADER = "x-block" + static final String IG_BLOCK_FAIL_HEADER = "x-block-fail" static final String IG_BLOCK_RESPONSE_HEADER = "x-block-response" static final String IG_PARAMETERS_BLOCK_HEADER = "x-block-parameters" static final String IG_BODY_END_BLOCK_HEADER = "x-block-body-end" @@ -2714,8 +2770,60 @@ abstract class HttpServerTest extends WithHttpServer { static final String IG_PATH_PARAMS_TAG = "ig-path-params" static final String IG_SESSION_ID_TAG = "ig-session-id" + /** + * The blocking point at which a test suite wants the block failure test to be exercised. Each + * variant pairs the endpoint to hit with the instrumentation gateway header that makes the fake + * AppSec callbacks block there. + */ + static enum BlockFailureVariant { + /** Blocks on {@code requestHeaderDone}, supported by every blocking instrumentation. */ + REQUEST_HEADERS(SUCCESS, IG_BLOCK_HEADER, 'json'), + /** Blocks on {@code requestPathParams}, for instrumentations that only publish path params. */ + PATH_PARAMS(PATH_PARAM, IG_PARAMETERS_BLOCK_HEADER, 'true') + + final ServerEndpoint endpoint + final String header + final String headerValue + + private BlockFailureVariant(ServerEndpoint endpoint, String header, String headerValue) { + this.endpoint = endpoint + this.header = header + this.headerValue = headerValue + } + } + + /** Simulates a server that cannot commit the blocking response. */ + static enum FailingBlockResponseFunction implements BlockResponseFunction { + INSTANCE + + @Override + boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, + BlockingContentType templateType, Map extraHeaders, String securityResponseId) { + false + } + } + class IGCallbacks { - static class Context { + static class Context implements AppSecContext { + /** + * Set by the last request that reported a block failure. Tests that read it reset it first; + * it has to be static because the assertion happens outside the request context. + */ + static volatile boolean blockFailureReported + + /** Replaces the server's block response function with one that fails to commit. */ + boolean failBlocking + + @Override + boolean isManuallyKept() { + false + } + + @Override + void reportBlockFailure() { + blockFailureReported = true + } + String matchingHeaderValue String doneHeaderValue String extraSpanName @@ -2739,6 +2847,20 @@ abstract class HttpServerTest extends WithHttpServer { string == null ? "" : string } + /** + * Builds the blocking flow for a blocking point. When the request asked for a block failure + * (see {@link HttpServerTest#IG_BLOCK_FAIL_HEADER}), the server's block response function is + * first replaced by one that cannot commit, so the instrumentation is expected to report a + * block failure on the AppSec context. + */ + static final Flow blockingFlow(RequestContext rqCtxt, Flow.Action.RequestBlockingAction action) { + Context context = rqCtxt.getData(RequestContextSlot.APPSEC) + if (context?.failBlocking) { + rqCtxt.blockResponseFunction = FailingBlockResponseFunction.INSTANCE + } + new RbaFlow(action) + } + final Supplier> requestStartedCb = ({ -> @@ -2779,6 +2901,9 @@ abstract class HttpServerTest extends WithHttpServer { if (IG_BLOCK_HEADER.equalsIgnoreCase(key)) { context.blockingContentType = value } + if (IG_BLOCK_FAIL_HEADER.equalsIgnoreCase(key)) { + context.failBlocking = true + } if (IG_BLOCK_RESPONSE_HEADER.equalsIgnoreCase(key)) { context.responseBlock = value } @@ -2808,11 +2933,11 @@ abstract class HttpServerTest extends WithHttpServer { } if (context.blockingContentType && context.blockingContentType != 'none') { - new RbaFlow( + blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(418, BlockingContentType.valueOf(context.blockingContentType.toUpperCase(Locale.ROOT)))) } else if (context.blockingContentType && context.blockingContentType == 'none') { - new RbaFlow( + blockingFlow(rqCtxt, Flow.Action.RequestBlockingAction.forRedirect(301, 'https://www.google.com/')) } else { Flow.ResultFlow.empty() @@ -2862,7 +2987,7 @@ abstract class HttpServerTest extends WithHttpServer { } activeSpan().localRootSpan.setTag('request.body', supplier.get() as String) if (context.bodyEndBlock) { - new RbaFlow( + blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON) ) } else { @@ -2892,7 +3017,7 @@ abstract class HttpServerTest extends WithHttpServer { rqCtxt.traceSegment.setTagTop('request.body.converted', obj as String) Context context = rqCtxt.getData(RequestContextSlot.APPSEC) if (context.bodyConvertedBlock) { - new RbaFlow( + blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON) ) } else { @@ -2936,7 +3061,7 @@ abstract class HttpServerTest extends WithHttpServer { Context context = rqCtxt.getData(RequestContextSlot.APPSEC) context.responseBody = body if (context.responseBlock) { - new RbaFlow( + blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON) ) } else { @@ -2971,12 +3096,12 @@ abstract class HttpServerTest extends WithHttpServer { context.tags.put(IG_RESPONSE_HEADER_TAG, context.igResponseHeaderValue) } if (context.responseBlock == 'none') { - new RbaFlow( + blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(301, BlockingContentType.NONE, [Location: 'https://www.google.com/']) ) } else if (context.responseBlock == 'json') { - new RbaFlow( + blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON) ) } else { @@ -2988,7 +3113,7 @@ abstract class HttpServerTest extends WithHttpServer { RequestContext rqCtxt, Map map -> Context context = rqCtxt.getData(RequestContextSlot.APPSEC) if (context.parametersBlock) { - return new RbaFlow( + return blockingFlow(rqCtxt, new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON) ) } From 2d0901d8d18b9a2e2e56485f793a4fa62b694637 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:05:39 +0200 Subject: [PATCH 04/21] Use RequestContext overload of tryCommitBlockingResponse in jax-rs/jakarta-rs annotations --- .../jakarta3/MessageBodyWriterInstrumentation.java | 4 +++- .../jaxrs2/MessageBodyWriterInstrumentation.java | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java index f259d4cde5b..52af9511e92 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java @@ -74,7 +74,9 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba)) { + reqCtx.getTraceSegment().effectivelyBlocked(); + } throw new BlockingException("Blocked request (for MessageBodyWriter)"); } diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java index 8262c211420..7707dd638d3 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java @@ -79,7 +79,9 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba)) { + reqCtx.getTraceSegment().effectivelyBlocked(); + } throw new BlockingException("Blocked request (for MessageBodyWriter)"); } From d0ffa17e99850820b1e5fa12d87b1587e83b831a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:07:42 +0200 Subject: [PATCH 05/21] Migrate RASP LFI/SSRF blocking to RequestContext tryCommitBlockingResponse overload --- .../trace/instrumentation/java/lang/FileIORaspHelper.java | 6 ++++-- .../trace/instrumentation/java/net/URLSinkCallSite.java | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java index 7dd48871dfe..3706cfc8a3d 100644 --- a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java +++ b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java @@ -143,9 +143,11 @@ private void invokeRaspCallback( BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + if (brf.tryCommitBlockingResponse(ctx, rba)) { + ctx.getTraceSegment().effectivelyBlocked(); + } + throw new BlockingException("Blocked request (for LFI attempt)"); } - throw new BlockingException("Blocked request (for LFI attempt)"); } } catch (final BlockingException e) { // re-throw blocking exceptions diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java index 7752797fa16..9703877cd5e 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java @@ -85,9 +85,11 @@ private static void raspCallback(@Nonnull final URL url) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + if (brf.tryCommitBlockingResponse(ctx, rba)) { + ctx.getTraceSegment().effectivelyBlocked(); + } + throw new BlockingException("Blocked request (for SSRF attempt)"); } - throw new BlockingException("Blocked request (for SSRF attempt)"); } } catch (final BlockingException e) { throw e; From c99be1d705301a20f4d3166070d08d2d086ba823 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:11:18 +0200 Subject: [PATCH 06/21] Report AppSec block failures from ratpack blocking call sites Switch every ratpack-1.5 blocking call site to the BlockResponseFunction.tryCommitBlockingResponse(RequestContext, RequestBlockingAction) overload so a blocking response that cannot be committed is reported via AppSecContext.reportBlockFailure(). effectivelyBlocked() is not called at these call sites: ratpack blocks through Netty's NettyBlockResponseFunction, whose BlockingResponseHandler already marks the trace segment. Enable the shared HttpServerTest block failure coverage for ratpack using the PATH_PARAMS variant, which exercises PathBindingPublishingHandler (the request header blocking point belongs to the netty instrumentation). --- .../instrumentation/ratpack/ContextParseAdvice.java | 4 +++- .../instrumentation/ratpack/JsonRendererAdvice.java | 4 +++- .../ratpack/PathBindingPublishingHandler.java | 4 +++- .../RatpackRequestBodyCallGetBufferAdvice.java | 4 +++- .../RatpackRequestBodyGetTextCalledAdvice.java | 4 +++- .../ratpack/RequestBodyCollectionPublisher.java | 4 +++- .../test/groovy/server/RatpackHttpServerTest.groovy | 12 ++++++++++++ 7 files changed, 30 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContextParseAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContextParseAdvice.java index e4b03268eff..ffaae71775c 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContextParseAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContextParseAdvice.java @@ -45,7 +45,9 @@ static void after( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's + // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. + brf.tryCommitBlockingResponse(reqCtx, rba); t = new BlockingException("Blocked request (for DefaultContext/parse)"); } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/JsonRendererAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/JsonRendererAdvice.java index 0f1b75320f8..843ef4f43ea 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/JsonRendererAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/JsonRendererAdvice.java @@ -42,7 +42,9 @@ static void enter( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's + // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. + brf.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for JsonRenderer/render)"); } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/PathBindingPublishingHandler.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/PathBindingPublishingHandler.java index a41a1cbfdda..cbdfaefc6a2 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/PathBindingPublishingHandler.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/PathBindingPublishingHandler.java @@ -67,7 +67,9 @@ private boolean maybePublishTokens(Context ctx) { return true; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's + // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. + blockResponseFunction.tryCommitBlockingResponse(requestContext, rba); return false; } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyCallGetBufferAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyCallGetBufferAdvice.java index 43c9de353d2..850812e3494 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyCallGetBufferAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyCallGetBufferAdvice.java @@ -45,7 +45,9 @@ static Throwable before( return null; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's + // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); return new BlockingException("Blocked request (for ByteBufBackedTypedData/getBuffer)"); } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java index 847041f83f8..af51a7ac6a5 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java @@ -36,7 +36,9 @@ static void after( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's + // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (throwable == null) { throwable = new BlockingException("Blocked request (for ByteBufBackedTypedData/getText)"); } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RequestBodyCollectionPublisher.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RequestBodyCollectionPublisher.java index 6ec7376249e..b8ff0e3e36c 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RequestBodyCollectionPublisher.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RequestBodyCollectionPublisher.java @@ -82,7 +82,9 @@ private void block(Flow.Action.RequestBlockingAction rba, Throwable t) { if (blockResponseFunction == null) { return; } - blockResponseFunction.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's + // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. + blockResponseFunction.tryCommitBlockingResponse(requestContext, rba); // we can't directly interrupt user code here by throwing an exception // user code must listen for errors and implement its own logic to prevent diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy index 0b665683b2b..a0ede81d695 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -108,6 +108,18 @@ class RatpackHttpServerTest extends HttpServerTest { true } + @Override + boolean testBlockFailure() { + true + } + + @Override + BlockFailureVariant blockFailureVariant() { + // Ratpack's own blocking call site for request headers lives in the Netty instrumentation; + // path params are published by PathBindingPublishingHandler, a Ratpack-specific call site. + BlockFailureVariant.PATH_PARAMS + } + @Override Serializable expectedServerSpanRoute(ServerEndpoint endpoint) { return String From cb8aa4eeb39d9dbde2177af1d5a22caf8ac0445e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:14:15 +0200 Subject: [PATCH 07/21] Migrate internal-api blocking glue to the RequestContext overload Switch the four blocking call sites in internal-api (StoredCharBody request-body block, AppSecEventTracker#dispatch, and the CMDI/SHI RASP checks in ProcessImplInstrumentationHelpers) from BlockResponseFunction#tryCommitBlockingResponse(TraceSegment, ...) to the (RequestContext, RequestBlockingAction) overload, so a silently failed commit is reported through AppSecContext#reportBlockFailure(). TraceSegment#effectivelyBlocked() is now called explicitly at each call site when the commit succeeds, following the jersey/resteasy pattern. StoredCharBody now throws BlockingException only when a BlockResponseFunction is present, matching the WAF request-body pattern used by jersey. The RASP checks keep throwing unconditionally: aborting the exec/shell attempt must not depend on being able to commit a blocking response. --- .../datadog/trace/api/appsec/AppSecEventTracker.java | 4 +++- .../java/datadog/trace/api/http/StoredCharBody.java | 6 ++++-- .../java/lang/ProcessImplInstrumentationHelpers.java | 12 ++++++++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java index 8fb38b7cebb..fdc1be30e5e 100644 --- a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java +++ b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java @@ -386,7 +386,9 @@ private boolean dispatch( final BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + if (brf.tryCommitBlockingResponse(ctx, rba)) { + ctx.getTraceSegment().effectivelyBlocked(); + } } return true; } diff --git a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java index aee387eb5ac..7666d9955d3 100644 --- a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java +++ b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java @@ -180,9 +180,11 @@ public synchronized void maybeNotifyAndBlock() { BlockResponseFunction blockResponseFunction = httpContext.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(httpContext.getTraceSegment(), rba); + if (blockResponseFunction.tryCommitBlockingResponse(httpContext, rba)) { + httpContext.getTraceSegment().effectivelyBlocked(); + } + throw new BlockingException("Blocked request (for request body stream read)"); } - throw new BlockingException("Blocked request (for request body stream read)"); } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 97b8eb15c29..5cdf9841555 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -243,8 +243,12 @@ public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + if (brf.tryCommitBlockingResponse(ctx, rba)) { + ctx.getTraceSegment().effectivelyBlocked(); + } } + // Thrown even without a BlockResponseFunction: RASP must abort the exec attempt even when + // no blocking response can be committed. throw new BlockingException("Blocked request (for CMDI attempt)"); } } catch (final BlockingException e) { @@ -294,8 +298,12 @@ public static void shiRaspCheck(@Nonnull final String cmd) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + if (brf.tryCommitBlockingResponse(ctx, rba)) { + ctx.getTraceSegment().effectivelyBlocked(); + } } + // Thrown even without a BlockResponseFunction: RASP must abort the shell command attempt + // even when no blocking response can be committed. throw new BlockingException("Blocked request (for SHI attempt)"); } } catch (final BlockingException e) { From 8e99eaf58f206b94dacafb187626b981d890209a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:17:36 +0200 Subject: [PATCH 08/21] Report AppSec block failures in Play instrumentation Migrate every Play blocking call site to BlockResponseFunction#tryCommitBlockingResponse(RequestContext, RequestBlockingAction) so a failed commit is reported through AppSecContext#reportBlockFailure(). PathExtractionHelpers threw the BlockingException even when no block response function was available, blocking the request with no chance of a blocking response ever being committed; the throw now happens strictly inside the guard. Play runs on the Netty server, whose block response function commits synchronously and calls TraceSegment#effectivelyBlocked() itself, so the call sites must not call it again. Enable the shared HttpServerTest block failure coverage for the Play 2.5 and 2.6 AppSec suites on the path params blocking point. --- .../play25/appsec/BodyParserHelpers.java | 25 +++++++++---------- .../appsec/ResultsStatusApplyAdvice.java | 2 +- .../appsec/StatusHeaderSendJsonAdvice.java | 2 +- .../play25/server/PlayServerTest.groovy | 11 ++++++++ .../play26/appsec/BodyParserHelpers.java | 25 +++++++++---------- .../appsec/ResultsStatusInstrumentation.java | 2 +- .../appsec/StatusHeaderInstrumentation.java | 2 +- .../play26/server/PlayServerTest.groovy | 11 ++++++++ .../play/appsec/PathExtractionHelpers.java | 8 ++++-- 9 files changed, 56 insertions(+), 32 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 0eba7744dff..8d2e915fed1 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 @@ -248,10 +248,10 @@ private static void executeFilenamesCallback( 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)"); - } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (multipart file upload)"); } } } @@ -333,10 +333,10 @@ private static void executeFilesContentCallback( 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)"); - } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (multipart file upload content)"); } } } @@ -370,11 +370,10 @@ private static void executeCallback( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - boolean success = - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (success) { - throw new BlockingException("Blocked request (for " + details + ")"); - } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (for " + details + ")"); } } } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java index 64e2ce7fbb5..1e6e180dbea 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java @@ -45,7 +45,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for Results$Status/apply)"); } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java index 51a6084c350..7ff6b227273 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java @@ -50,7 +50,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for StatusHeader/sendJson)"); } 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 fd10be014f3..61903630217 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 @@ -108,6 +108,17 @@ class PlayServerTest extends HttpServerTest { true } + @Override + boolean testBlockFailure() { + true + } + + @Override + BlockFailureVariant blockFailureVariant() { + // play publishes the blocking path params callback from PathExtractionHelpers + BlockFailureVariant.PATH_PARAMS + } + @Override String testPathParam() { '/path/?/param' 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 4f1fff4b5fa..001b50af18a 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 @@ -259,10 +259,10 @@ private static void executeFilenamesCallback( 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)"); - } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (multipart file upload)"); } } } @@ -344,10 +344,10 @@ private static void executeFilesContentCallback( 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)"); - } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (multipart file upload content)"); } } } @@ -407,11 +407,10 @@ private static void executeCallback( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - boolean success = - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (success) { - throw new BlockingException("Blocked request (for " + details + ")"); - } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (for " + details + ")"); } } } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java index 659a47a02bf..30cc2ecfe52 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java @@ -87,7 +87,7 @@ static void after( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for Results$Status/apply)"); } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java index 904c3731d3f..ca855a58492 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java @@ -88,7 +88,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for StatusHeader/sendJson)"); } 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 0ac729a38ac..66433eb2a28 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 @@ -17,6 +17,17 @@ class PlayServerTest extends AbstractPlayServerTest { true } + @Override + boolean testBlockFailure() { + true + } + + @Override + BlockFailureVariant blockFailureVariant() { + // play publishes the blocking path params callback from PathExtractionHelpers + BlockFailureVariant.PATH_PARAMS + } + def 'test instrumentation gateway xml request body'() { setup: def request = request( diff --git a/dd-java-agent/instrumentation/play/play-appsec-common/src/main/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpers.java b/dd-java-agent/instrumentation/play/play-appsec-common/src/main/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpers.java index 8da233f0388..82936363be3 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-common/src/main/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpers.java +++ b/dd-java-agent/instrumentation/play/play-appsec-common/src/main/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpers.java @@ -50,9 +50,13 @@ private static BlockingException doCallRequestPathParamsCallback( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (brf == null) { + // nothing can commit the blocking response, so the request must not be blocked + return null; } + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); return new BlockingException("Blocked request (for " + origin + ")"); } } From 678a75b6e383d6ef10de1ff883d60f95c32d451a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:18:39 +0200 Subject: [PATCH 09/21] Report AppSec block failures from vertx-web instrumentation Migrate every vertx-web 3.4/4.0/5.0 blocking call site to BlockResponseFunction#tryCommitBlockingResponse(RequestContext, RequestBlockingAction), which centralises AppSecContext#reportBlockFailure() when the blocking response cannot be committed. effectivelyBlocked() is deliberately not called at these call sites: vertx-web reuses Netty's block response function, which already finishes the span synchronously when it commits the response. Also fix RoutingContextJsonAdvice, which only assigned a BlockingException when the instrumented method had not thrown. A concurrent exception would silently swallow the block, so the BlockingException is now thrown unconditionally inside the block response function guard. Enable the shared HttpServerTest block failure test in the three vertx-web server test suites, using the path params variant since path params are published by the vertx-web instrumentation itself. --- .../vertx_3_4/server/FileUploadHelper.java | 5 +++-- .../server/PathParameterPublishingHelper.java | 5 ++++- .../vertx_3_4/server/RoutingContextJsonAdvice.java | 13 +++++-------- .../server/RoutingContextSessionAdvice.java | 4 +++- .../vertx_3_4/server/WafPublishingBodyHandler.java | 4 +++- .../groovy/server/VertxHttpServerForkedTest.groovy | 12 ++++++++++++ .../vertx_4_0/server/FileUploadHelper.java | 5 +++-- .../server/PathParameterPublishingHelper.java | 5 ++++- .../vertx_4_0/server/RoutingContextJsonAdvice.java | 13 +++++-------- .../server/RoutingContextJsonResponseAdvice.java | 4 +++- .../server/RoutingContextSessionAdvice.java | 4 +++- .../vertx_4_0/server/WafPublishingBodyHandler.java | 4 +++- .../groovy/server/VertxHttpServerForkedTest.groovy | 12 ++++++++++++ .../vertx_5_0/server/FileUploadHelper.java | 5 +++-- .../vertx_5_0/server/WafPublishingBodyHandler.java | 4 +++- .../groovy/server/VertxHttpServerForkedTest.groovy | 12 ++++++++++++ 16 files changed, 81 insertions(+), 30 deletions(-) diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java index 913003e8c28..017e23d14f6 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java @@ -22,8 +22,9 @@ public static BlockingException commitBlockingResponse( if (action instanceof Flow.Action.RequestBlockingAction) { BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse( - reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + brf.tryCommitBlockingResponse(reqCtx, (Flow.Action.RequestBlockingAction) action); return new BlockingException(reason); } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/PathParameterPublishingHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/PathParameterPublishingHelper.java index 23e2a0c5728..53f1a255065 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/PathParameterPublishingHelper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/PathParameterPublishingHelper.java @@ -49,7 +49,10 @@ public static Throwable publishParams(Map params) { log.warn("Can't block. Don't know how to block on this server"); } else { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is + // committed. + brf.tryCommitBlockingResponse(requestContext, rba); be = new BlockingException("Blocked request (for route/matches)"); } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java index 8cb28041ec8..44b1fa91acb 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java @@ -18,10 +18,7 @@ @RequiresRequestContext(RequestContextSlot.APPSEC) class RoutingContextJsonAdvice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - static void after( - @Advice.Return Object obj_, - @ActiveRequestContext RequestContext reqCtx, - @Advice.Thrown(readOnly = false) Throwable throwable) { + static void after(@Advice.Return Object obj_, @ActiveRequestContext RequestContext reqCtx) { if (obj_ == null) { return; } @@ -45,10 +42,10 @@ static void after( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (throwable == null) { - throwable = new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); - } + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdvice.java index 5d7a0504a74..74ab1d69138 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdvice.java @@ -41,7 +41,9 @@ static void after( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for session)"); } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/WafPublishingBodyHandler.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/WafPublishingBodyHandler.java index 482ace0ef7e..821cdc12f85 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/WafPublishingBodyHandler.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/WafPublishingBodyHandler.java @@ -70,7 +70,9 @@ private void publishRequestBody(Object body) { return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException( "Blocked request (for Buffer/toString or Buffer/toJson{Object,Array})"); } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 7fadc69bac5..1e821c066ce 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -144,6 +144,18 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBlockFailure() { + true + } + + @Override + BlockFailureVariant blockFailureVariant() { + // Path params are published by the vert.x-web instrumentation itself, whereas request headers + // are published (and blocked on) by the underlying netty instrumentation. + BlockFailureVariant.PATH_PARAMS + } + @Override Class expectedExceptionType() { return RuntimeException diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java index c0eef8cab2e..1d41c203934 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java @@ -22,8 +22,9 @@ public static BlockingException commitBlockingResponse( if (action instanceof Flow.Action.RequestBlockingAction) { BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse( - reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + brf.tryCommitBlockingResponse(reqCtx, (Flow.Action.RequestBlockingAction) action); return new BlockingException(reason); } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/PathParameterPublishingHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/PathParameterPublishingHelper.java index 3c6f230fc01..2be6e3b248d 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/PathParameterPublishingHelper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/PathParameterPublishingHelper.java @@ -47,7 +47,10 @@ public static Throwable publishParams(Map params) { log.warn("Can't block. Don't know how to block on this server"); } else { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is + // committed. + brf.tryCommitBlockingResponse(requestContext, rba); return new BlockingException("Blocked request (for route/matches)"); } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java index b1699e5c8f7..3fccc3d1a49 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java @@ -26,10 +26,7 @@ static void before() { } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - static void after( - @Advice.Return Object obj_, - @ActiveRequestContext RequestContext reqCtx, - @Advice.Thrown(readOnly = false) Throwable throwable) { + static void after(@Advice.Return Object obj_, @ActiveRequestContext RequestContext reqCtx) { // in newer versions of vert.x rc.getBodyAsJson() calls internally rc.body().asJsonObject() // so we need to prevent sending the body twice to the WAF @@ -60,10 +57,10 @@ static void after( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (throwable == null) { - throwable = new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); - } + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonResponseAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonResponseAdvice.java index 1dfff50beef..5f51dd782d5 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonResponseAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonResponseAdvice.java @@ -43,7 +43,9 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for RoutingContext/json)"); } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdvice.java index b80d8e9077f..1c9be5993ca 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdvice.java @@ -41,7 +41,9 @@ static void after( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for session)"); } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/WafPublishingBodyHandler.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/WafPublishingBodyHandler.java index d23a70e30e3..71f7e8a6849 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/WafPublishingBodyHandler.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/WafPublishingBodyHandler.java @@ -70,7 +70,9 @@ private void publishRequestBody(Object body) { return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException( "Blocked request (for Buffer/toString or Buffer/toJson{Object,Array})"); } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 62455354611..1385b59833e 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -150,6 +150,18 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBlockFailure() { + true + } + + @Override + BlockFailureVariant blockFailureVariant() { + // Path params are published by the vert.x-web instrumentation itself, whereas request headers + // are published (and blocked on) by the underlying netty instrumentation. + BlockFailureVariant.PATH_PARAMS + } + @Override boolean isRequestBodyNoStreaming() { true diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java index 045d825444a..a4cd402370c 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java @@ -22,8 +22,9 @@ public static BlockingException commitBlockingResponse( if (action instanceof Flow.Action.RequestBlockingAction) { BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse( - reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + brf.tryCommitBlockingResponse(reqCtx, (Flow.Action.RequestBlockingAction) action); return new BlockingException(reason); } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/WafPublishingBodyHandler.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/WafPublishingBodyHandler.java index 92364ba2cec..5c174b81021 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/WafPublishingBodyHandler.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/WafPublishingBodyHandler.java @@ -69,7 +69,9 @@ private void publishRequestBody(Object body) { return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response + // function, which finishes the span synchronously when the blocking response is committed. + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException( "Blocked request (for Buffer/toString or Buffer/toJson{Object,Array})"); } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index cdd4bca1c19..3b784751929 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -145,6 +145,18 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBlockFailure() { + true + } + + @Override + BlockFailureVariant blockFailureVariant() { + // Path params are published by the vert.x-web instrumentation itself, whereas request headers + // are published (and blocked on) by the underlying netty instrumentation. + BlockFailureVariant.PATH_PARAMS + } + @Override boolean isRequestBodyNoStreaming() { true From 47ba6432ed406d1754dcc79b05f838b13a00891d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:26:13 +0200 Subject: [PATCH 10/21] Move Play 2.5 response body blocking glue into a static helper The Results$Status/apply and StatusHeader/sendJson advices carried the WAF callback lookup and the block response function call inline in their advice bodies, which get inlined into the instrumented Play classes. Both now delegate to BodyParserHelpers#handleResponseBody, reusing the existing executeCallback glue, so the blocking logic lives in a single static helper and stays testable, matching the pattern of the rest of the AppSec blocking call sites. --- .../play25/appsec/BodyParserHelpers.java | 22 +++++++++++++ .../appsec/ResultsStatusApplyAdvice.java | 31 ++----------------- .../appsec/StatusHeaderInstrumentation.java | 7 +++++ .../appsec/StatusHeaderSendJsonAdvice.java | 31 +------------------ 4 files changed, 32 insertions(+), 59 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 8d2e915fed1..3fff9264195 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 @@ -378,6 +378,28 @@ private static void executeCallback( } } + /** + * Publishes a response body to the WAF and blocks the request if the WAF requires it. Kept here + * so that inline advices don't carry the block response function logic in their bodies. + * + * @param reqCtx the active request context + * @param body the response body, already converted to plain java objects + * @param details the call site description used in the {@link BlockingException} message + */ + public static void handleResponseBody(RequestContext reqCtx, Object body, String details) { + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + if (cbp == null) { + return; + } + BiFunction> callback = + cbp.getCallback(EVENTS.responseBody()); + if (callback == null) { + return; + } + + executeCallback(reqCtx, callback, body, details); + } + private static Object tryConvertingScalaContainers(Object obj, int depth) { if (depth == 0) { return obj; diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java index 1e6e180dbea..70896264b86 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/ResultsStatusApplyAdvice.java @@ -1,18 +1,11 @@ package datadog.trace.instrumentation.play25.appsec; -import static datadog.trace.api.gateway.Events.EVENTS; import static datadog.trace.instrumentation.play25.appsec.BodyParserHelpers.jsValueToJavaObject; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; -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.bootstrap.instrumentation.api.AgentTracer; -import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; import play.api.libs.json.JsValue; @@ -27,27 +20,7 @@ static void before( return; } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - if (cbp == null) { - return; - } - BiFunction> callback = - cbp.getCallback(EVENTS.responseBody()); - if (callback == null) { - return; - } - - Flow flow = callback.apply(reqCtx, jsValueToJavaObject((JsValue) content)); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction == null) { - return; - } - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - - throw new BlockingException("Blocked request (for Results$Status/apply)"); - } + BodyParserHelpers.handleResponseBody( + reqCtx, jsValueToJavaObject((JsValue) content), "Results$Status/apply"); } } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderInstrumentation.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderInstrumentation.java index c3362e119a9..187da47e0cf 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderInstrumentation.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderInstrumentation.java @@ -26,6 +26,13 @@ public String instrumentedType() { return "play.mvc.StatusHeader"; } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".BodyParserHelpers", packageName + ".BodyParserHelpers$ScalaIteratorAdapter", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java index 7ff6b227273..c127e45a17d 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/main/java/datadog/trace/instrumentation/play25/appsec/StatusHeaderSendJsonAdvice.java @@ -1,19 +1,11 @@ package datadog.trace.instrumentation.play25.appsec; -import static datadog.trace.api.gateway.Events.EVENTS; - import com.fasterxml.jackson.databind.JsonNode; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; -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.bootstrap.CallDepthThreadLocalMap; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; import play.mvc.StatusHeader; @@ -32,28 +24,7 @@ static void before( return; } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - if (cbp == null) { - return; - } - BiFunction> callback = - cbp.getCallback(EVENTS.responseBody()); - if (callback == null) { - return; - } - - Flow flow = callback.apply(reqCtx, json); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction == null) { - return; - } - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - - throw new BlockingException("Blocked request (for StatusHeader/sendJson)"); - } + BodyParserHelpers.handleResponseBody(reqCtx, json, "StatusHeader/sendJson"); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) From 7bdf5b534a86eabe066e15a58d9b4adc2492bc39 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 12:44:09 +0200 Subject: [PATCH 11/21] Maximize @DataDog/asm-java sole ownership for AppSec blocking glue in CODEOWNERS Adds sole-asm-java routing for blocking-glue files across vertx-web, ratpack, jax-rs/jakarta-rs, netty, and cross-framework helpers that were previously apm-idm-java-only or shadowed by the dual-owned appsec/* patterns (last-match-wins). Appended after the existing dual block so it wins; does not touch the ~40-file dual category left as a team decision. --- .github/CODEOWNERS | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d61754b6290..7182eb0f40f 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -273,6 +273,42 @@ /dd-java-agent/instrumentation/spring/spring-security/ @DataDog/asm-java @DataDog/apm-idm-java /dd-java-agent/instrumentation/velocity-1.5/ @DataDog/asm-java @DataDog/apm-idm-java +# AppSec blocking glue - exclusive asm-java ownership (must stay after the dual-owned +# /dd-java-agent/instrumentation/**/*appsec* patterns above to win last-match-wins) + +# vertx-web (3.4 / 4.0 / 5.0) +/dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/RoutingContext*Advice.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/FileUploadHelper.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/PathParameterPublishingHelper.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/BlockingExceptionHandler.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/Vertx*Instrumentation.java @DataDog/asm-java + +# ratpack-1.5 (flat package, no appsec/ subpackage) +/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/**/ContextParseAdvice.java @DataDog/asm-java +/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/**/JsonRendererAdvice.java @DataDog/asm-java +/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/**/PathBindingPublishingHandler.java @DataDog/asm-java +/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/**/RatpackRequestBody*Advice.java @DataDog/asm-java +/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/**/RequestBodyCollectionPublisher.java @DataDog/asm-java + +# jax-rs / jakarta-rs annotations +/dd-java-agent/instrumentation/rs/**/MessageBodyWriterInstrumentation.java @DataDog/asm-java + +# netty-4.1 (blocking glue already migrated by #12519, currently apm-idm-java only) +/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/**/NettyMultipartHelper.java @DataDog/asm-java +/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/**/HttpPostRequestDecoderInstrumentation.java @DataDog/asm-java +/dd-java-agent/instrumentation/netty/**/server/BlockingResponseHandler.java @DataDog/asm-java +/dd-java-agent/instrumentation/netty/**/server/MaybeBlockResponseHandler.java @DataDog/asm-java + +# cross-framework blocking helpers and BlockResponseFunction implementations +/dd-java-agent/instrumentation/**/*BlockingHelper.java @DataDog/asm-java +/dd-java-agent/instrumentation/**/*BlockResponseFunction.java @DataDog/asm-java +/dd-java-agent/instrumentation/undertow/undertow-common/src/main/java/**/UndertowBlockingHandler.java @DataDog/asm-java +/dd-java-agent/instrumentation/jetty/jetty-server/*/src/main/*/**/JettyCommitResponse*.java @DataDog/asm-java + +# bootstrap / internal-api blocking and RASP glue +/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/blocking/ @DataDog/asm-java +/internal-api/src/*/*/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @DataDog/asm-java + # @DataDog/ci-app-libraries /dd-java-agent/agent-ci-visibility/ @DataDog/ci-app-libraries /dd-smoke-tests/backend-mock/ @DataDog/ci-app-libraries From aea500b01efa4e41d6cc20beecd28170506bae17 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 14:20:32 +0200 Subject: [PATCH 12/21] Ensure RASP blocking always aborts and dedupe blocking-glue call sites - FileIORaspHelper/URLSinkCallSite: throw BlockingException even when no BlockResponseFunction is available, so LFI/SSRF attempts are always aborted, matching the existing CMDI/SHI invariant - ProcessImplInstrumentationHelpers: extract shared commitBlockingResponse() helper for the CMDI/SHI call sites (pure dedup, no behavior change) - play-appsec-2.5/2.6 BodyParserHelpers: extract shared commitBlockAndThrow() helper for the three multipart call sites (pure dedup, no behavior change) - RatpackRequestBodyGetTextCalledAdvice: drop the unused @Advice.Thrown(readOnly=false) parameter and throw unconditionally, matching the RoutingContextSessionAdvice/JsonAdvice pattern - HttpServerTest.groovy: close the response in a finally block in executeIgnoringIoErrors to avoid a resource leak - CODEOWNERS: split the vertx-web wildcard glob into explicit per-version entries (3.4/4.0/5.0) for Vertx*Instrumentation.java --- .github/CODEOWNERS | 4 ++- .../agent/test/base/HttpServerTest.groovy | 6 ++-- .../java/lang/FileIORaspHelper.java | 4 ++- .../java/net/URLSinkCallSite.java | 4 ++- .../play25/appsec/BodyParserHelpers.java | 35 ++++++++----------- .../play26/appsec/BodyParserHelpers.java | 35 ++++++++----------- ...RatpackRequestBodyGetTextCalledAdvice.java | 7 ++-- .../ProcessImplInstrumentationHelpers.java | 24 ++++++------- 8 files changed, 53 insertions(+), 66 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 7182eb0f40f..4b99ecc5a86 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -281,7 +281,9 @@ /dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/FileUploadHelper.java @DataDog/asm-java /dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/PathParameterPublishingHelper.java @DataDog/asm-java /dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/BlockingExceptionHandler.java @DataDog/asm-java -/dd-java-agent/instrumentation/vertx/vertx-web/*/src/main/java/**/Vertx*Instrumentation.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/**/Vertx*Instrumentation.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/**/Vertx*Instrumentation.java @DataDog/asm-java +/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/**/Vertx*Instrumentation.java @DataDog/asm-java # ratpack-1.5 (flat package, no appsec/ subpackage) /dd-java-agent/instrumentation/ratpack-1.5/src/main/java/**/ContextParseAdvice.java @DataDog/asm-java diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index b91030cdac3..3b5993df7dc 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -2117,13 +2117,15 @@ abstract class HttpServerTest extends WithHttpServer { * be closed without a complete HTTP response. */ protected Response executeIgnoringIoErrors(Request request) { + Response response = null try { - def response = client.newCall(request).execute() + response = client.newCall(request).execute() response.body().bytes() - response.close() response } catch (IOException ignored) { null + } finally { + response?.close() } } diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java index 3706cfc8a3d..3f8a1df12b0 100644 --- a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java +++ b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java @@ -146,8 +146,10 @@ private void invokeRaspCallback( if (brf.tryCommitBlockingResponse(ctx, rba)) { ctx.getTraceSegment().effectivelyBlocked(); } - throw new BlockingException("Blocked request (for LFI attempt)"); } + // Thrown even without a BlockResponseFunction: RASP must abort the LFI attempt even when + // no blocking response can be committed. + throw new BlockingException("Blocked request (for LFI attempt)"); } } catch (final BlockingException e) { // re-throw blocking exceptions diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java index 9703877cd5e..f3036e84286 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java @@ -88,8 +88,10 @@ private static void raspCallback(@Nonnull final URL url) { if (brf.tryCommitBlockingResponse(ctx, rba)) { ctx.getTraceSegment().effectivelyBlocked(); } - throw new BlockingException("Blocked request (for SSRF attempt)"); } + // Thrown even without a BlockResponseFunction: RASP must abort the SSRF attempt even when + // no blocking response can be committed. + throw new BlockingException("Blocked request (for SSRF attempt)"); } } catch (final BlockingException e) { throw e; 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 3fff9264195..eaa5f72b83a 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 @@ -246,13 +246,7 @@ private static void executeFilenamesCallback( 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) { - // play runs on netty, which commits the blocking response synchronously and calls - // TraceSegment#effectivelyBlocked() itself: never call it here - brf.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (multipart file upload)"); - } + commitBlockAndThrow(reqCtx, rba, "multipart file upload"); } } @@ -331,13 +325,7 @@ private static void executeFilesContentCallback( 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) { - // play runs on netty, which commits the blocking response synchronously and calls - // TraceSegment#effectivelyBlocked() itself: never call it here - brf.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (multipart file upload content)"); - } + commitBlockAndThrow(reqCtx, rba, "multipart file upload content"); } } @@ -368,13 +356,18 @@ private static void executeCallback( Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - // play runs on netty, which commits the blocking response synchronously and calls - // TraceSegment#effectivelyBlocked() itself: never call it here - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (for " + details + ")"); - } + commitBlockAndThrow(reqCtx, rba, "for " + details); + } + } + + private static void commitBlockAndThrow( + RequestContext reqCtx, Flow.Action.RequestBlockingAction rba, String details) { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (" + details + ")"); } } 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 001b50af18a..50a002d31a8 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 @@ -257,13 +257,7 @@ private static void executeFilenamesCallback( 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) { - // play runs on netty, which commits the blocking response synchronously and calls - // TraceSegment#effectivelyBlocked() itself: never call it here - brf.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (multipart file upload)"); - } + commitBlockAndThrow(reqCtx, rba, "multipart file upload"); } } @@ -342,13 +336,7 @@ private static void executeFilesContentCallback( 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) { - // play runs on netty, which commits the blocking response synchronously and calls - // TraceSegment#effectivelyBlocked() itself: never call it here - brf.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (multipart file upload content)"); - } + commitBlockAndThrow(reqCtx, rba, "multipart file upload content"); } } @@ -405,13 +393,18 @@ private static void executeCallback( Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - // play runs on netty, which commits the blocking response synchronously and calls - // TraceSegment#effectivelyBlocked() itself: never call it here - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (for " + details + ")"); - } + commitBlockAndThrow(reqCtx, rba, "for " + details); + } + } + + private static void commitBlockAndThrow( + RequestContext reqCtx, Flow.Action.RequestBlockingAction rba, String details) { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + // play runs on netty, which commits the blocking response synchronously and calls + // TraceSegment#effectivelyBlocked() itself: never call it here + brf.tryCommitBlockingResponse(reqCtx, rba); + throw new BlockingException("Blocked request (" + details + ")"); } } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java index af51a7ac6a5..08c6e465e09 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java @@ -19,8 +19,7 @@ public class RatpackRequestBodyGetTextCalledAdvice { static void after( @Advice.This ByteBufBackedTypedData thiz, @Advice.Return String str, - @ActiveRequestContext RequestContext reqCtx, - @Advice.Thrown(readOnly = false) Throwable throwable) { + @ActiveRequestContext RequestContext reqCtx) { Boolean bodyPublished = InstrumentationContext.get(ByteBufBackedTypedData.class, Boolean.class).get(thiz); if (bodyPublished == Boolean.TRUE) { @@ -39,9 +38,7 @@ static void after( // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - if (throwable == null) { - throwable = new BlockingException("Blocked request (for ByteBufBackedTypedData/getText)"); - } + throw new BlockingException("Blocked request (for ByteBufBackedTypedData/getText)"); } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 5cdf9841555..034b1956c35 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -240,13 +240,7 @@ public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { Flow flow = execCmdCallback.apply(ctx, cmdArray); Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction brf = ctx.getBlockResponseFunction(); - if (brf != null) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (brf.tryCommitBlockingResponse(ctx, rba)) { - ctx.getTraceSegment().effectivelyBlocked(); - } - } + commitBlockingResponse(ctx, (Flow.Action.RequestBlockingAction) action); // Thrown even without a BlockResponseFunction: RASP must abort the exec attempt even when // no blocking response can be committed. throw new BlockingException("Blocked request (for CMDI attempt)"); @@ -260,6 +254,14 @@ public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { } } + private static void commitBlockingResponse( + RequestContext ctx, Flow.Action.RequestBlockingAction rba) { + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null && brf.tryCommitBlockingResponse(ctx, rba)) { + ctx.getTraceSegment().effectivelyBlocked(); + } + } + public static void resetCheckShi() { checkShi.set(false); } @@ -295,13 +297,7 @@ public static void shiRaspCheck(@Nonnull final String cmd) { Flow flow = shellCmdCallback.apply(ctx, cmd); Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction brf = ctx.getBlockResponseFunction(); - if (brf != null) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (brf.tryCommitBlockingResponse(ctx, rba)) { - ctx.getTraceSegment().effectivelyBlocked(); - } - } + commitBlockingResponse(ctx, (Flow.Action.RequestBlockingAction) action); // Thrown even without a BlockResponseFunction: RASP must abort the shell command attempt // even when no blocking response can be committed. throw new BlockingException("Blocked request (for SHI attempt)"); From 48a528afbf9a0fbdbc56a0e6cc2182e5420b3d08 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 16:18:00 +0200 Subject: [PATCH 13/21] Exclude datadog.trace.agent.test.base from SpotBugs stale-thread-write analysis HttpServerTest$IGCallbacks$Context implementing AppSecContext made SpotBugs newly flag every primitive setter in the class as AT_STALE_THREAD_WRITE_OF_PRIMITIVE, failing CI. This is test infrastructure shared across instrumentation suites, not production code, matching the existing exclusion pattern for smoke test controllers. --- gradle/spotbugFilters/exclude.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gradle/spotbugFilters/exclude.xml b/gradle/spotbugFilters/exclude.xml index 2276ed212e0..22bde5b0ef0 100644 --- a/gradle/spotbugFilters/exclude.xml +++ b/gradle/spotbugFilters/exclude.xml @@ -4,6 +4,8 @@ + + From a39067d58ace18b086a215da9aef89d1334738da Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 10:38:15 +0200 Subject: [PATCH 14/21] Restore blocking rethrow through @Advice.Thrown and unconditional body-read abort Three OnMethodExit advices (Ratpack getText, vertx-web 3.4/4.0 getBodyAsJson) had suppress = Throwable.class, so a bare `throw new BlockingException(...)` inside the advice body was swallowed by ByteBuddy's suppression wrapper instead of propagating to the caller, silently disabling blocking on those call sites. Restored the @Advice.Thrown(readOnly = false) reassignment, the only mechanism ByteBuddy respects to rethrow despite suppress. StoredCharBody.maybeNotifyAndBlock() had moved the abort throw inside the blockResponseFunction != null guard, so a request with no response function available no longer aborted the body read at all. Moved the throw back outside the guard to always abort when the WAF requests blocking. Found by Codex review on PR #12601. --- .../ratpack/RatpackRequestBodyGetTextCalledAdvice.java | 7 +++++-- .../vertx_3_4/server/RoutingContextJsonAdvice.java | 9 +++++++-- .../vertx_4_0/server/RoutingContextJsonAdvice.java | 9 +++++++-- .../main/java/datadog/trace/api/http/StoredCharBody.java | 2 +- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java index 08c6e465e09..af51a7ac6a5 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/RatpackRequestBodyGetTextCalledAdvice.java @@ -19,7 +19,8 @@ public class RatpackRequestBodyGetTextCalledAdvice { static void after( @Advice.This ByteBufBackedTypedData thiz, @Advice.Return String str, - @ActiveRequestContext RequestContext reqCtx) { + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable throwable) { Boolean bodyPublished = InstrumentationContext.get(ByteBufBackedTypedData.class, Boolean.class).get(thiz); if (bodyPublished == Boolean.TRUE) { @@ -38,7 +39,9 @@ static void after( // effectivelyBlocked() is intentionally absent: Ratpack blocks through Netty's // BlockResponseFunction, whose BlockingResponseHandler already marks the segment. blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (for ByteBufBackedTypedData/getText)"); + if (throwable == null) { + throwable = new BlockingException("Blocked request (for ByteBufBackedTypedData/getText)"); + } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java index 44b1fa91acb..7bb3c5c6a74 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextJsonAdvice.java @@ -18,7 +18,10 @@ @RequiresRequestContext(RequestContextSlot.APPSEC) class RoutingContextJsonAdvice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - static void after(@Advice.Return Object obj_, @ActiveRequestContext RequestContext reqCtx) { + static void after( + @Advice.Return Object obj_, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable throwable) { if (obj_ == null) { return; } @@ -45,7 +48,9 @@ static void after(@Advice.Return Object obj_, @ActiveRequestContext RequestConte // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response // function, which finishes the span synchronously when the blocking response is committed. blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); + if (throwable == null) { + throwable = new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); + } } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java index 3fccc3d1a49..99c968af91d 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextJsonAdvice.java @@ -26,7 +26,10 @@ static void before() { } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - static void after(@Advice.Return Object obj_, @ActiveRequestContext RequestContext reqCtx) { + static void after( + @Advice.Return Object obj_, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable throwable) { // in newer versions of vert.x rc.getBodyAsJson() calls internally rc.body().asJsonObject() // so we need to prevent sending the body twice to the WAF @@ -60,7 +63,9 @@ static void after(@Advice.Return Object obj_, @ActiveRequestContext RequestConte // effectivelyBlocked() is intentionally absent: vertx-web shares Netty's block response // function, which finishes the span synchronously when the blocking response is committed. blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); + if (throwable == null) { + throwable = new BlockingException("Blocked request (for RoutingContextImpl/getBodyAsJson)"); + } } } } diff --git a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java index 7666d9955d3..0991cd65122 100644 --- a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java +++ b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java @@ -183,8 +183,8 @@ public synchronized void maybeNotifyAndBlock() { if (blockResponseFunction.tryCommitBlockingResponse(httpContext, rba)) { httpContext.getTraceSegment().effectivelyBlocked(); } - throw new BlockingException("Blocked request (for request body stream read)"); } + throw new BlockingException("Blocked request (for request body stream read)"); } } From e2b56563f69f0ac3bf3ac331a206d95e21726f59 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 13:04:46 +0200 Subject: [PATCH 15/21] Extract tryCommitBlockingResponseAndMarkBlocked to dedupe commit-and-mark pattern StoredCharBody and AppSecEventTracker both live in internal-api and repeated the same "commit blocking response, then mark segment on success" sequence. --- .../trace/api/appsec/AppSecEventTracker.java | 4 +--- .../api/gateway/BlockResponseFunction.java | 20 +++++++++++++++++++ .../trace/api/http/StoredCharBody.java | 4 +--- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java index fdc1be30e5e..b1d65a57d70 100644 --- a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java +++ b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java @@ -386,9 +386,7 @@ private boolean dispatch( final BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (brf.tryCommitBlockingResponse(ctx, rba)) { - ctx.getTraceSegment().effectivelyBlocked(); - } + brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); } return true; } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java b/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java index a9c11618e11..ceb47837178 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java @@ -69,4 +69,24 @@ default boolean tryCommitBlockingResponse( } return committed; } + + /** + * Commits blocking response using a RequestBlockingAction, marking {@code ctx}'s trace segment as + * effectively blocked on success. Callers in {@code internal-api} that both commit and mark the + * segment should use this instead of repeating the {@code if (tryCommitBlockingResponse(...)) { + * effectivelyBlocked(); }} pattern inline. + * + * @param ctx the request context + * @param action the blocking action containing status code, content type, headers, and security + * response ID + * @return true unless blocking could not be attempted + */ + default boolean tryCommitBlockingResponseAndMarkBlocked( + RequestContext ctx, Flow.Action.RequestBlockingAction action) { + boolean committed = tryCommitBlockingResponse(ctx, action); + if (committed) { + ctx.getTraceSegment().effectivelyBlocked(); + } + return committed; + } } diff --git a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java index 0991cd65122..178ee0e2c96 100644 --- a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java +++ b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java @@ -180,9 +180,7 @@ public synchronized void maybeNotifyAndBlock() { BlockResponseFunction blockResponseFunction = httpContext.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (blockResponseFunction.tryCommitBlockingResponse(httpContext, rba)) { - httpContext.getTraceSegment().effectivelyBlocked(); - } + blockResponseFunction.tryCommitBlockingResponseAndMarkBlocked(httpContext, rba); } throw new BlockingException("Blocked request (for request body stream read)"); } From 97a5d3fa4085861e2bbc875d3b790d2ae9dda40b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 13:17:20 +0200 Subject: [PATCH 16/21] Extend tryCommitBlockingResponseAndMarkBlocked reuse to RASP/AppSec call sites FileIORaspHelper, URLSinkCallSite, and both MessageBodyWriterInstrumentation classes (jax-rs2, jakarta3) had the same inline tryCommitBlockingResponse+effectivelyBlocked pattern already deduped in internal-api. ProcessImplInstrumentationHelpers' own private helper now delegates to the shared default method instead of duplicating the logic. --- .../trace/instrumentation/java/lang/FileIORaspHelper.java | 4 +--- .../trace/instrumentation/java/net/URLSinkCallSite.java | 4 +--- .../jakarta3/MessageBodyWriterInstrumentation.java | 4 +--- .../jaxrs2/MessageBodyWriterInstrumentation.java | 4 +--- .../api/java/lang/ProcessImplInstrumentationHelpers.java | 4 ++-- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java index 3f8a1df12b0..6785592d66c 100644 --- a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java +++ b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java @@ -143,9 +143,7 @@ private void invokeRaspCallback( BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (brf.tryCommitBlockingResponse(ctx, rba)) { - ctx.getTraceSegment().effectivelyBlocked(); - } + brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); } // Thrown even without a BlockResponseFunction: RASP must abort the LFI attempt even when // no blocking response can be committed. diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java index f3036e84286..927677d1ec2 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java @@ -85,9 +85,7 @@ private static void raspCallback(@Nonnull final URL url) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (brf.tryCommitBlockingResponse(ctx, rba)) { - ctx.getTraceSegment().effectivelyBlocked(); - } + brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); } // Thrown even without a BlockResponseFunction: RASP must abort the SSRF attempt even when // no blocking response can be committed. diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java index 52af9511e92..f55b56e7f13 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java @@ -74,9 +74,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba)) { - reqCtx.getTraceSegment().effectivelyBlocked(); - } + blockResponseFunction.tryCommitBlockingResponseAndMarkBlocked(reqCtx, rba); throw new BlockingException("Blocked request (for MessageBodyWriter)"); } diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java index 7707dd638d3..ac169aacdcb 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java @@ -79,9 +79,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - if (blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba)) { - reqCtx.getTraceSegment().effectivelyBlocked(); - } + blockResponseFunction.tryCommitBlockingResponseAndMarkBlocked(reqCtx, rba); throw new BlockingException("Blocked request (for MessageBodyWriter)"); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 034b1956c35..49e0619fed3 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -257,8 +257,8 @@ public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { private static void commitBlockingResponse( RequestContext ctx, Flow.Action.RequestBlockingAction rba) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); - if (brf != null && brf.tryCommitBlockingResponse(ctx, rba)) { - ctx.getTraceSegment().effectivelyBlocked(); + if (brf != null) { + brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); } } From a0abb67c3d78c5aee2185d70f2f16bdbf157338a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 13:22:42 +0200 Subject: [PATCH 17/21] Extract handleResponseBody helper in play26 BodyParserHelpers ResultsStatusInstrumentation and StatusHeaderInstrumentation duplicated the same callback-lookup + block-and-throw logic inline. play25's BodyParserHelpers already extracted this to handleResponseBody() in this PR; apply the same extraction here, reusing the existing executeCallback helper. --- .../play26/appsec/BodyParserHelpers.java | 14 +++++++ .../appsec/ResultsStatusInstrumentation.java | 31 +--------------- .../appsec/StatusHeaderInstrumentation.java | 37 ++++--------------- 3 files changed, 24 insertions(+), 58 deletions(-) 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 50a002d31a8..63e6e1a8863 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 @@ -397,6 +397,20 @@ private static void executeCallback( } } + public static void handleResponseBody(RequestContext reqCtx, Object body, String details) { + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + if (cbp == null) { + return; + } + BiFunction> callback = + cbp.getCallback(EVENTS.responseBody()); + if (callback == null) { + return; + } + + executeCallback(reqCtx, callback, body, details); + } + private static void commitBlockAndThrow( RequestContext reqCtx, Flow.Action.RequestBlockingAction rba, String details) { BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java index 30cc2ecfe52..ffcb98d330b 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/ResultsStatusInstrumentation.java @@ -1,24 +1,17 @@ package datadog.trace.instrumentation.play26.appsec; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static datadog.trace.api.gateway.Events.EVENTS; import static datadog.trace.instrumentation.play26.appsec.BodyParserHelpers.jsValueToJavaObject; import com.google.auto.service.AutoService; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -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.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.instrumentation.play26.MuzzleReferences; -import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; import play.api.libs.json.JsValue; @@ -69,28 +62,8 @@ static void after( return; } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - if (cbp == null) { - return; - } - BiFunction> callback = - cbp.getCallback(EVENTS.responseBody()); - if (callback == null) { - return; - } - - Flow flow = callback.apply(reqCtx, jsValueToJavaObject((JsValue) content)); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction == null) { - return; - } - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - - throw new BlockingException("Blocked request (for Results$Status/apply)"); - } + BodyParserHelpers.handleResponseBody( + reqCtx, jsValueToJavaObject((JsValue) content), "Results$Status/apply"); } } } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java index ca855a58492..7cd57f39900 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/StatusHeaderInstrumentation.java @@ -1,26 +1,19 @@ package datadog.trace.instrumentation.play26.appsec; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static datadog.trace.api.gateway.Events.EVENTS; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.fasterxml.jackson.databind.JsonNode; import com.google.auto.service.AutoService; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -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.bootstrap.CallDepthThreadLocalMap; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.instrumentation.play26.MuzzleReferences; -import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; import play.mvc.StatusHeader; @@ -47,6 +40,13 @@ public String instrumentedType() { return "play.mvc.StatusHeader"; } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".BodyParserHelpers", packageName + ".BodyParserHelpers$ScalaIteratorAdapter", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -70,28 +70,7 @@ static void before( return; } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - if (cbp == null) { - return; - } - BiFunction> callback = - cbp.getCallback(EVENTS.responseBody()); - if (callback == null) { - return; - } - - Flow flow = callback.apply(reqCtx, json); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction == null) { - return; - } - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - - throw new BlockingException("Blocked request (for StatusHeader/sendJson)"); - } + BodyParserHelpers.handleResponseBody(reqCtx, json, "StatusHeader/sendJson"); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) From 6e0c149bc58a9b8076787c8cde95d0e901fd1cf0 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 14:30:08 +0200 Subject: [PATCH 18/21] Fix commitBlockAndThrow to honor commit success and cover markAndCommit - Restore the success check in play25/play26 commitBlockAndThrow before throwing BlockingException, dropped by the earlier dedup extraction - Add unit tests for BlockResponseFunction.tryCommitBlockingResponseAndMarkBlocked covering both branches (marks effectivelyBlocked on success, skips on failure), fixing internal-api jacocoTestCoverageVerification --- .../play25/appsec/BodyParserHelpers.java | 6 +- .../play26/appsec/BodyParserHelpers.java | 6 +- .../gateway/BlockResponseFunctionTest.java | 76 ++++++++++++++++++- 3 files changed, 83 insertions(+), 5 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 eaa5f72b83a..2f30e950dee 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 @@ -366,8 +366,10 @@ private static void commitBlockAndThrow( if (brf != null) { // play runs on netty, which commits the blocking response synchronously and calls // TraceSegment#effectivelyBlocked() itself: never call it here - brf.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (" + details + ")"); + boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); + if (success) { + throw new BlockingException("Blocked request (" + details + ")"); + } } } 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 63e6e1a8863..078c9444cfd 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 @@ -417,8 +417,10 @@ private static void commitBlockAndThrow( if (brf != null) { // play runs on netty, which commits the blocking response synchronously and calls // TraceSegment#effectivelyBlocked() itself: never call it here - brf.tryCommitBlockingResponse(reqCtx, rba); - throw new BlockingException("Blocked request (" + details + ")"); + boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); + if (success) { + throw new BlockingException("Blocked request (" + details + ")"); + } } } diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java index e07f49ceb12..a696e467a1c 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java @@ -57,6 +57,75 @@ void doesNotThrowWhenAppSecSlotDoesNotHoldAnAppSecContext() { assertFalse(brf.tryCommitBlockingResponse(new TestRequestContext("not an AppSecContext"), RBA)); } + @Test + void markAndCommitMarksTraceSegmentBlockedWhenCommitSucceeds() { + CountingTraceSegment traceSegment = new CountingTraceSegment(); + TestRequestContext ctx = new TestRequestContext(new CountingAppSecContext(), traceSegment); + TestBlockResponseFunction brf = new TestBlockResponseFunction(true); + + assertTrue(brf.tryCommitBlockingResponseAndMarkBlocked(ctx, RBA)); + + assertEquals(1, traceSegment.effectivelyBlockedCalls); + } + + @Test + void markAndCommitDoesNotMarkTraceSegmentBlockedWhenCommitFails() { + CountingTraceSegment traceSegment = new CountingTraceSegment(); + TestRequestContext ctx = new TestRequestContext(new CountingAppSecContext(), traceSegment); + TestBlockResponseFunction brf = new TestBlockResponseFunction(false); + + assertFalse(brf.tryCommitBlockingResponseAndMarkBlocked(ctx, RBA)); + + assertEquals(0, traceSegment.effectivelyBlockedCalls); + } + + private static final class CountingTraceSegment implements TraceSegment { + private int effectivelyBlockedCalls; + + @Override + public void setTagTop(String key, Object value, boolean sanitize) {} + + @Override + public Object getTagTop(String key, boolean sanitize) { + return null; + } + + @Override + public void setTagCurrent(String key, Object value, boolean sanitize) {} + + @Override + public Object getTagCurrent(String key, boolean sanitize) { + return null; + } + + @Override + public void setDataTop(String key, Object value) {} + + @Override + public Object getDataTop(String key) { + return null; + } + + @Override + public void effectivelyBlocked() { + effectivelyBlockedCalls++; + } + + @Override + public void setDataCurrent(String key, Object value) {} + + @Override + public Object getDataCurrent(String key) { + return null; + } + + @Override + public void setMetaStructTop(String field, Object value) {} + + @Override + public void setMetaStructCurrent(String field, Object value) {} + } + private static final class CountingAppSecContext implements AppSecContext { private int blockFailures; @@ -97,10 +166,15 @@ public boolean tryCommitBlockingResponse( private static final class TestRequestContext implements RequestContext { private final Object appSecData; - private final TraceSegment traceSegment = TraceSegment.NoOp.INSTANCE; + private final TraceSegment traceSegment; private TestRequestContext(Object appSecData) { + this(appSecData, TraceSegment.NoOp.INSTANCE); + } + + private TestRequestContext(Object appSecData, TraceSegment traceSegment) { this.appSecData = appSecData; + this.traceSegment = traceSegment; } @SuppressWarnings("unchecked") From d68cedcefdad23c6ad73981aaa96795147851c5c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 15:35:36 +0200 Subject: [PATCH 19/21] Trigger CI rerun Empty commit to retrigger the pipeline after the transient muzzle shard [3/8] network timeout. From 589c247429002362cc7c177cd4f6537b43344f50 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 15:56:44 +0200 Subject: [PATCH 20/21] Remove premature effectivelyBlocked marking from BlockResponseFunction helper tryCommitBlockingResponseAndMarkBlocked called effectivelyBlocked() as soon as tryCommitBlockingResponse returned true, but for async implementations (Netty off-event-loop, Undertow dispatch) true only means the commit was scheduled, not that it actually happened. A later async failure would leave the trace marked as blocked with no reportBlockFailure() call. None of the migrated call sites called effectivelyBlocked() before this PR, so the helper was introducing the premature call rather than deduplicating an existing pattern. Removes the helper, restores the 7 call sites to call tryCommitBlockingResponse(ctx, rba) directly (matching pre-PR semantics), and documents on the interface javadoc that a synchronous true does not guarantee the response was already committed. Adds a BlockResponseFunctionTest case reproducing the async schedule-then-commit-later scenario from the review comment. --- .../java/lang/FileIORaspHelper.java | 2 +- .../java/net/URLSinkCallSite.java | 2 +- .../MessageBodyWriterInstrumentation.java | 2 +- .../MessageBodyWriterInstrumentation.java | 2 +- .../trace/api/appsec/AppSecEventTracker.java | 2 +- .../api/gateway/BlockResponseFunction.java | 25 ++----- .../trace/api/http/StoredCharBody.java | 2 +- .../ProcessImplInstrumentationHelpers.java | 2 +- .../gateway/BlockResponseFunctionTest.java | 72 +++++++++++++++++-- 9 files changed, 77 insertions(+), 34 deletions(-) diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java index 6785592d66c..0d242f879e9 100644 --- a/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java +++ b/dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileIORaspHelper.java @@ -143,7 +143,7 @@ private void invokeRaspCallback( BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); + brf.tryCommitBlockingResponse(ctx, rba); } // Thrown even without a BlockResponseFunction: RASP must abort the LFI attempt even when // no blocking response can be committed. diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java index 927677d1ec2..c160edf0a8b 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/main/java/datadog/trace/instrumentation/java/net/URLSinkCallSite.java @@ -85,7 +85,7 @@ private static void raspCallback(@Nonnull final URL url) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); + brf.tryCommitBlockingResponse(ctx, rba); } // Thrown even without a BlockResponseFunction: RASP must abort the SSRF attempt even when // no blocking response can be committed. diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java index f55b56e7f13..7fde1168665 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterInstrumentation.java @@ -74,7 +74,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponseAndMarkBlocked(reqCtx, rba); + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for MessageBodyWriter)"); } diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java index ac169aacdcb..30352bb22e0 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterInstrumentation.java @@ -79,7 +79,7 @@ static void before( return; } Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - blockResponseFunction.tryCommitBlockingResponseAndMarkBlocked(reqCtx, rba); + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); throw new BlockingException("Blocked request (for MessageBodyWriter)"); } diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java index b1d65a57d70..aee774ff6ab 100644 --- a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java +++ b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java @@ -386,7 +386,7 @@ private boolean dispatch( final BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); + brf.tryCommitBlockingResponse(ctx, rba); } return true; } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java b/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java index ceb47837178..742492e7e7a 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/BlockResponseFunction.java @@ -51,7 +51,10 @@ default boolean tryCommitBlockingResponse( * AppSecContext#reportBlockFailure()} if the commit fails. * *

It's responsible for calling {@link TraceSegment#effectivelyBlocked()} before the span is - * finished. + * finished. Callers must never call {@link TraceSegment#effectivelyBlocked()} themselves on the + * strength of a {@code true} return value: asynchronous implementations (Netty off the event + * loop, Undertow dispatching to an IO thread) return {@code true} as soon as the blocking + * response is scheduled, and only mark the segment once the response is actually committed. * * @param ctx the request context * @param action the blocking action containing status code, content type, headers, and security @@ -69,24 +72,4 @@ default boolean tryCommitBlockingResponse( } return committed; } - - /** - * Commits blocking response using a RequestBlockingAction, marking {@code ctx}'s trace segment as - * effectively blocked on success. Callers in {@code internal-api} that both commit and mark the - * segment should use this instead of repeating the {@code if (tryCommitBlockingResponse(...)) { - * effectivelyBlocked(); }} pattern inline. - * - * @param ctx the request context - * @param action the blocking action containing status code, content type, headers, and security - * response ID - * @return true unless blocking could not be attempted - */ - default boolean tryCommitBlockingResponseAndMarkBlocked( - RequestContext ctx, Flow.Action.RequestBlockingAction action) { - boolean committed = tryCommitBlockingResponse(ctx, action); - if (committed) { - ctx.getTraceSegment().effectivelyBlocked(); - } - return committed; - } } diff --git a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java index 178ee0e2c96..cc46a41ae3b 100644 --- a/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java +++ b/internal-api/src/main/java/datadog/trace/api/http/StoredCharBody.java @@ -180,7 +180,7 @@ public synchronized void maybeNotifyAndBlock() { BlockResponseFunction blockResponseFunction = httpContext.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponseAndMarkBlocked(httpContext, rba); + blockResponseFunction.tryCommitBlockingResponse(httpContext, rba); } throw new BlockingException("Blocked request (for request body stream read)"); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 49e0619fed3..f807f6e1daa 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -258,7 +258,7 @@ private static void commitBlockingResponse( RequestContext ctx, Flow.Action.RequestBlockingAction rba) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponseAndMarkBlocked(ctx, rba); + brf.tryCommitBlockingResponse(ctx, rba); } } diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java index a696e467a1c..bcb3dbf88aa 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java @@ -16,7 +16,8 @@ /** * Covers the {@code tryCommitBlockingResponse(RequestContext, RequestBlockingAction)} default * method, which reports a block failure to {@link AppSecContext#reportBlockFailure()} when the - * blocking response cannot be committed. + * blocking response cannot be committed, and leaves {@link TraceSegment#effectivelyBlocked()} to + * the implementation. */ class BlockResponseFunctionTest { @@ -58,25 +59,51 @@ void doesNotThrowWhenAppSecSlotDoesNotHoldAnAppSecContext() { } @Test - void markAndCommitMarksTraceSegmentBlockedWhenCommitSucceeds() { + void leavesEffectivelyBlockedToTheImplementationOnSuccess() { CountingTraceSegment traceSegment = new CountingTraceSegment(); TestRequestContext ctx = new TestRequestContext(new CountingAppSecContext(), traceSegment); TestBlockResponseFunction brf = new TestBlockResponseFunction(true); - assertTrue(brf.tryCommitBlockingResponseAndMarkBlocked(ctx, RBA)); + assertTrue(brf.tryCommitBlockingResponse(ctx, RBA)); - assertEquals(1, traceSegment.effectivelyBlockedCalls); + // the implementation, not the default method, owns effectivelyBlocked() + assertEquals(0, traceSegment.effectivelyBlockedCalls); } @Test - void markAndCommitDoesNotMarkTraceSegmentBlockedWhenCommitFails() { + void doesNotMarkTraceSegmentBlockedWhenCommitFails() { CountingTraceSegment traceSegment = new CountingTraceSegment(); TestRequestContext ctx = new TestRequestContext(new CountingAppSecContext(), traceSegment); TestBlockResponseFunction brf = new TestBlockResponseFunction(false); - assertFalse(brf.tryCommitBlockingResponseAndMarkBlocked(ctx, RBA)); + assertFalse(brf.tryCommitBlockingResponse(ctx, RBA)); + + assertEquals(0, traceSegment.effectivelyBlockedCalls); + } + + /** + * Mimics Netty off the event loop or Undertow dispatching to an IO thread: the commit is only + * scheduled, so {@code true} must not be read as "the response was committed". The trace segment + * stays unmarked until the scheduled work runs, and a scheduled commit that later fails still + * gets to report the block failure. + */ + @Test + void asynchronousImplementationOwnsMarkingAndFailureReporting() { + CountingTraceSegment traceSegment = new CountingTraceSegment(); + CountingAppSecContext appSecCtx = new CountingAppSecContext(); + TestRequestContext ctx = new TestRequestContext(appSecCtx, traceSegment); + DeferredBlockResponseFunction brf = new DeferredBlockResponseFunction(); + assertTrue(brf.tryCommitBlockingResponse(ctx, RBA)); assertEquals(0, traceSegment.effectivelyBlockedCalls); + assertEquals(0, appSecCtx.blockFailures); + + brf.runScheduled(true); + assertEquals(1, traceSegment.effectivelyBlockedCalls); + + brf.runScheduled(false); + assertEquals(1, traceSegment.effectivelyBlockedCalls); + assertEquals(1, appSecCtx.blockFailures); } private static final class CountingTraceSegment implements TraceSegment { @@ -164,6 +191,39 @@ public boolean tryCommitBlockingResponse( } } + /** + * A {@link BlockResponseFunction} that only schedules the blocking response, the way Netty does + * when called off the event loop. {@link #runScheduled(boolean)} plays the scheduled work back. + */ + private static final class DeferredBlockResponseFunction implements BlockResponseFunction { + private RequestContext scheduledCtx; + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + throw new UnsupportedOperationException("the RequestContext overload is scheduled instead"); + } + + @Override + public boolean tryCommitBlockingResponse( + RequestContext ctx, Flow.Action.RequestBlockingAction action) { + this.scheduledCtx = ctx; + return true; + } + + private void runScheduled(boolean committed) { + if (committed) { + scheduledCtx.getTraceSegment().effectivelyBlocked(); + } else { + ((AppSecContext) scheduledCtx.getData(RequestContextSlot.APPSEC)).reportBlockFailure(); + } + } + } + private static final class TestRequestContext implements RequestContext { private final Object appSecData; private final TraceSegment traceSegment; From c6ae6d9b2609a5208eab76c7f8b2afed270e942d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 16:15:28 +0200 Subject: [PATCH 21/21] Add unit test coverage for PathExtractionHelpers fail-open contract PathExtractionHelpers had zero test coverage. Its behavior when BlockResponseFunction is unavailable (fail-open: return null instead of throwing BlockingException) is an intentional design decision already discussed on PR review (a WAF block cannot be committed consistently without a BlockResponseFunction, unlike the RASP case which fails closed to prevent an actual command/file/network operation), but nothing guarded it against an accidental flip in a future refactor. Adds PathExtractionHelpersTest covering: empty/null params, missing callback, non-blocking actions, the fail-open branch, the commit branch (including a failed commit still throwing, since the boolean result is intentionally ignored), and callback exceptions being swallowed. Adds the mockito test dependency the module was missing. --- .../play/play-appsec-common/build.gradle | 4 + .../appsec/PathExtractionHelpersTest.java | 159 ++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java diff --git a/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle b/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle index 3d48cbcff41..804fba845f1 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle +++ b/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle @@ -1,3 +1,7 @@ plugins { id 'dd-trace-java.module.instrumentation' } + +dependencies { + testImplementation libs.bundles.mockito +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java b/dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java new file mode 100644 index 00000000000..babded1f26a --- /dev/null +++ b/dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java @@ -0,0 +1,159 @@ +package datadog.trace.instrumentation.play.appsec; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +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.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; +import java.util.Collections; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class PathExtractionHelpersTest { + + private static final String ORIGIN = "test.origin"; + + private TracerAPI originalTracer; + private CallbackProvider callbackProvider; + private BiFunction, Flow> callback; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + originalTracer = AgentTracer.get(); + + callbackProvider = mock(CallbackProvider.class); + callback = mock(BiFunction.class); + when(callbackProvider.getCallback(EVENTS.requestPathParams())).thenReturn(callback); + + TracerAPI tracer = mock(TracerAPI.class); + when(tracer.getCallbackProvider(RequestContextSlot.APPSEC)).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @Test + void nullParams_returnsNullWithoutCallingCallback() { + RequestContext reqCtx = mock(RequestContext.class); + + assertNull(PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, null, ORIGIN)); + + verify(callback, never()).apply(any(), any()); + } + + @Test + void emptyParams_returnsNullWithoutCallingCallback() { + RequestContext reqCtx = mock(RequestContext.class); + + assertNull( + PathExtractionHelpers.callRequestPathParamsCallback( + reqCtx, Collections.emptyMap(), ORIGIN)); + + verify(callback, never()).apply(any(), any()); + } + + @Test + void noCallbackRegistered_returnsNull() { + when(callbackProvider.getCallback(EVENTS.requestPathParams())).thenReturn(null); + RequestContext reqCtx = mock(RequestContext.class); + + assertNull(PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params(), ORIGIN)); + + verify(reqCtx, never()).getBlockResponseFunction(); + } + + @Test + void nonBlockingAction_returnsNull() { + RequestContext reqCtx = mock(RequestContext.class); + stubCallbackAction(Flow.Action.Noop.INSTANCE); + + assertNull(PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params(), ORIGIN)); + + verify(reqCtx, never()).getBlockResponseFunction(); + } + + /** + * Pins the intentional fail-open contract: without a {@link BlockResponseFunction} nothing can + * commit a blocking response, so a refactor flipping this to fail-closed must fail here. + */ + @Test + void blockingActionWithoutBlockResponseFunction_failsOpen() { + RequestContext reqCtx = mock(RequestContext.class); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + stubCallbackAction(blockingAction()); + + assertNull(PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params(), ORIGIN)); + + verify(reqCtx).getBlockResponseFunction(); + } + + @Test + void blockingActionWithBlockResponseFunction_commitsAndThrows() { + RequestContext reqCtx = mock(RequestContext.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + Flow.Action.RequestBlockingAction rba = blockingAction(); + stubCallbackAction(rba); + + BlockingException exception = + PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params(), ORIGIN); + + assertNotNull(exception); + verify(brf).tryCommitBlockingResponse(reqCtx, rba); + } + + @Test + void blockingActionWithFailedCommit_stillThrows() { + RequestContext reqCtx = mock(RequestContext.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + Flow.Action.RequestBlockingAction rba = blockingAction(); + when(brf.tryCommitBlockingResponse(reqCtx, rba)).thenReturn(false); + stubCallbackAction(rba); + + assertNotNull(PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params(), ORIGIN)); + } + + @Test + void callbackThrows_exceptionIsSwallowed() { + RequestContext reqCtx = mock(RequestContext.class); + when(callback.apply(any(), any())).thenThrow(new RuntimeException("boom")); + + assertNull(PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params(), ORIGIN)); + } + + private void stubCallbackAction(Flow.Action action) { + @SuppressWarnings("unchecked") + Flow flow = mock(Flow.class); + when(flow.getAction()).thenReturn(action); + when(callback.apply(any(), any())).thenReturn(flow); + } + + private static Flow.Action.RequestBlockingAction blockingAction() { + return new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + } + + private static Map params() { + return Collections.singletonMap("id", "1"); + } +}