From 0a95ff8b262e47fb57a91d2e1cf39de620639582 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 16 Sep 2026 16:16:22 +0200 Subject: [PATCH 1/6] Add block-outcome telemetry to blocking response helpers - Wire reportBlockFailure() at every BlockResponseFunction.tryCommitBlockingResponse call site across akka-http, grizzly, jetty (appsec + server), liberty, spring-webmvc and undertow, resolving AppSecContext via RequestContextSlot.APPSEC instead of casting directly to AppSecRequestContext - Add UnmarshallerHelpersBlockFailureTest covering all branches of UnmarshallerHelpers.tryBlock (block committed, block failed, no BRF, foreign/null AppSec slot) - Widen UnmarshallerHelpers.tryBlock visibility via @VisibleForTesting instead of a package-private comment --- .../UnmarshallerHelpersBlockFailureTest.java | 167 +++++++++++++++ .../appsec/BlockingResponseHelper.java | 13 +- .../akkahttp/appsec/UnmarshallerHelpers.java | 18 +- .../grizzly/GrizzlyBlockingHelper.java | 34 ++- .../ParsedBodyParametersInstrumentation.java | 8 +- .../jetty11/MultipartHelper.java | 9 + ...tractContentParametersInstrumentation.java | 8 +- .../jetty11/MultipartHelperTest.java | 162 ++++++++++++++ .../jetty70/UrlEncodedInstrumentation.java | 8 +- .../instrumentation/jetty8/PartHelper.java | 13 ++ .../jetty8/PartHelperTest.java | 199 ++++++++++++++++++ .../jetty92/MultipartHelper.java | 9 + ...tractContentParametersInstrumentation.java | 15 +- .../jetty92/MultipartHelperTest.java | 162 ++++++++++++++ .../jetty93/MultipartHelper.java | 9 + ...tractContentParametersInstrumentation.java | 8 +- .../jetty93/MultipartHelperTest.java | 162 ++++++++++++++ .../jetty94/MultipartHelper.java | 9 + ...tractContentParametersInstrumentation.java | 8 +- .../jetty94/MultipartHelperTest.java | 162 ++++++++++++++ .../src/test/groovy/Jetty11Test.groovy | 36 ++++ .../JettyCommitResponseInstrumentation.java | 6 + .../src/test/groovy/Jetty70Test.groovy | 35 +++ .../JettyCommitResponseInstrumentation.java | 11 +- .../src/test/groovy/Jetty76Test.groovy | 35 +++ .../JettyCommitResponseInstrumentation.java | 7 + .../instrumentation/jetty9/Jetty9Test.groovy | 36 ++++ .../instrumentation/jetty9/Jetty9Test.groovy | 36 ++++ .../liberty20/GetPartsInstrumentation.java | 8 +- .../ParseParametersInstrumentation.java | 8 +- .../ParsePostDataInstrumentation.java | 8 +- .../liberty20/Liberty20Test.groovy | 49 +++++ .../ParseParametersInstrumentation.java | 8 +- .../ParsePostDataInstrumentation.java | 8 +- .../liberty23/Liberty23Test.groovy | 49 +++++ .../HttpMessageConverterInstrumentation.java | 15 +- ...lateAndMatrixVariablesInstrumentation.java | 8 +- ...ateVariablesUrlHandlerInstrumentation.java | 8 +- ...MessageConverterInstrumentationTest.groovy | 135 ++++++++++++ .../test/boot/SpringBootBasedTest.groovy | 31 +++ .../springweb6/HandleMatchAdvice.java | 8 +- .../InterceptorPreHandleAdvice.java | 8 +- .../FormDataParserInstrumentation.java | 10 +- ...MultiPartUploadHandlerInstrumentation.java | 27 +++ .../test/groovy/UndertowServletTest.groovy | 67 ++++++ 45 files changed, 1812 insertions(+), 28 deletions(-) create mode 100644 dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpersBlockFailureTest.java diff --git a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpersBlockFailureTest.java b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpersBlockFailureTest.java new file mode 100644 index 00000000000..900ac26de42 --- /dev/null +++ b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpersBlockFailureTest.java @@ -0,0 +1,167 @@ +package datadog.trace.instrumentation.akkahttp.appsec; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.ClientIpAddressData; +import java.util.Map; +import java.util.function.Function; +import org.junit.jupiter.api.Test; + +/** + * Covers the {@code UnmarshallerHelpers.tryBlock() -> AppSecContext.reportBlockFailure()} path. + * Hand-written test doubles are used because Mockito is only on this module's test runtime + * classpath, not its test compile classpath (see {@code gradle/java_deps.gradle}: {@code + * testRuntimeOnly libs.mokito.core}). + */ +class UnmarshallerHelpersBlockFailureTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @Test + void reportsBlockFailureWhenBlockingResponseCannotBeCommitted() { + CountingAppSecContext appSecCtx = new CountingAppSecContext(); + TestRequestContext ctx = + new TestRequestContext(new TestBlockResponseFunction(false), appSecCtx); + + BlockingException exception = UnmarshallerHelpers.tryBlock(ctx, RBA, "for test"); + + assertNull(exception); + assertEquals(1, appSecCtx.blockFailures); + assertSame(RBA, ctx.brf.lastAction); + assertSame(ctx.traceSegment, ctx.brf.lastSegment); + } + + @Test + void doesNotReportBlockFailureWhenBlockingResponseIsCommitted() { + CountingAppSecContext appSecCtx = new CountingAppSecContext(); + TestRequestContext ctx = new TestRequestContext(new TestBlockResponseFunction(true), appSecCtx); + + BlockingException exception = UnmarshallerHelpers.tryBlock(ctx, RBA, "for test"); + + assertNotNull(exception); + assertEquals("Blocked request (for test)", exception.getMessage()); + assertEquals(0, appSecCtx.blockFailures); + } + + @Test + void doesNotReportOrThrowWhenNoBlockResponseFunctionIsRegistered() { + CountingAppSecContext appSecCtx = new CountingAppSecContext(); + TestRequestContext ctx = new TestRequestContext(null, appSecCtx); + + BlockingException exception = UnmarshallerHelpers.tryBlock(ctx, RBA, "for test"); + + assertNull(exception); + assertEquals(0, appSecCtx.blockFailures); + } + + @Test + void doesNotThrowWhenAppSecSlotDoesNotHoldAnAppSecContext() { + TestRequestContext nullSlot = + new TestRequestContext(new TestBlockResponseFunction(false), null); + assertNull(UnmarshallerHelpers.tryBlock(nullSlot, RBA, "for test")); + + TestRequestContext foreignSlot = + new TestRequestContext(new TestBlockResponseFunction(false), "not an AppSecContext"); + assertNull(UnmarshallerHelpers.tryBlock(foreignSlot, RBA, "for test")); + } + + private static final class CountingAppSecContext implements AppSecContext { + private int blockFailures; + + @Override + public boolean isManuallyKept() { + return false; + } + + @Override + public void reportBlockFailure() { + blockFailures++; + } + } + + private static final class TestBlockResponseFunction implements BlockResponseFunction { + private final boolean committed; + private TraceSegment lastSegment; + private Flow.Action.RequestBlockingAction lastAction; + + private TestBlockResponseFunction(boolean committed) { + this.committed = committed; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, Flow.Action.RequestBlockingAction rba) { + this.lastAction = rba; + return BlockResponseFunction.super.tryCommitBlockingResponse(segment, rba); + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + this.lastSegment = segment; + return committed; + } + } + + private static final class TestRequestContext implements RequestContext { + private final TestBlockResponseFunction brf; + private final Object appSecData; + private final TraceSegment traceSegment = TraceSegment.NoOp.INSTANCE; + + private TestRequestContext(TestBlockResponseFunction brf, Object appSecData) { + this.brf = brf; + this.appSecData = appSecData; + } + + @SuppressWarnings("unchecked") + @Override + public T getData(RequestContextSlot slot) { + return slot == RequestContextSlot.APPSEC ? (T) appSecData : null; + } + + @Override + public TraceSegment getTraceSegment() { + return traceSegment; + } + + @Override + public void setBlockResponseFunction(BlockResponseFunction blockResponseFunction) {} + + @Override + public BlockResponseFunction getBlockResponseFunction() { + return brf; + } + + @Override + public T getOrCreateMetaStructTop(String key, Function defaultValue) { + return null; + } + + @Override + public void setClientIpAddressData(ClientIpAddressData clientIpAddressData) {} + + @Override + public ClientIpAddressData getClientIpAddressData() { + return null; + } + + @Override + public void close() {} + } +} diff --git a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java index 68d6a9d84c0..fe81b755586 100644 --- a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java +++ b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java @@ -13,9 +13,11 @@ import akka.http.scaladsl.model.StatusCodes; import akka.util.ByteString; import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.blocking.BlockingActionHelper; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.akkahttp.AkkaHttpServerHeaders; @@ -42,7 +44,16 @@ public static HttpResponse handleFinishForWaf(final AgentSpan span, final HttpRe if (action instanceof Flow.Action.RequestBlockingAction) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; if (brf instanceof AkkaBlockResponseFunction) { - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba)) { + // Safe to report here: both wiring points (DatadogAsyncHandlerWrapper.apply and + // DatadogServerRequestResponseFlowWrapper's onPush) call finishSpan/onRequestEnded + // synchronously, immediately after this method returns, in the same callback/thread, + // with no scheduling boundary in between. + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } HttpResponse altResponse = ((AkkaBlockResponseFunction) brf).maybeCreateAlternativeResponse(); if (altResponse != null) { diff --git a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java index 78c791612a5..7f3a7425902 100644 --- a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java +++ b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java @@ -15,12 +15,14 @@ import akka.stream.Materializer; import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.http.MultipartContentDecoder; +import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.lang.reflect.Field; @@ -607,7 +609,8 @@ private static void handleArbitraryPostData(Object o, String source) { executeCallback(reqCtx, callback, o, source); } - private static BlockingException tryBlock( + @VisibleForTesting + static BlockingException tryBlock( RequestContext reqCtx, Flow.Action.RequestBlockingAction rba, String details) { BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf == null) { @@ -615,6 +618,19 @@ private static BlockingException tryBlock( } boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); if (!success) { + // Conditional async-race gap (same class as netty-blocking.md §10/§11, but via + // Future.map/.recover/.thenApply on a Scala ExecutionContext instead of + // eventLoop().execute()): reportBlockFailure() below is only guaranteed to run before + // GatewayBridge.onRequestEnded/end-of-request telemetry is emitted when the route's + // response Future causally depends (via flatMap) on this same unmarshalling Future - the + // idiomatic Akka HTTP usage. If the app decouples unmarshalling (used only for a side + // effect) from response production, or triggers toStrict() conversions independently of + // the main response chain, this report can arrive after end-of-request telemetry has + // already been emitted. This is not fixed here; see the KB entry for akka-http. + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } return null; } if (brf instanceof AkkaBlockResponseFunction) { diff --git a/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java b/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java index ece9d6c4f07..a084cc8724f 100644 --- a/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java +++ b/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java @@ -4,7 +4,10 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.context.Context; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.blocking.BlockingActionHelper; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.io.OutputStream; @@ -55,11 +58,20 @@ public static boolean block( Map extraHeaders, String securityResponseId, Context context) { + AgentSpan span = AgentSpan.fromContext(context); if (GET_OUTPUT_STREAM == null) { + if (span != null) { + RequestContext reqCtx = span.getRequestContext(); + if (reqCtx != null) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } + } return false; } - AgentSpan span = AgentSpan.fromContext(context); try { OutputStream os = (OutputStream) GET_OUTPUT_STREAM.invoke(response); response.setStatus(BlockingActionHelper.getHttpCode(statusCode)); @@ -79,19 +91,29 @@ public static boolean block( } os.close(); response.finish(); - - if (span != null) { - span.getRequestContext().getTraceSegment().effectivelyBlocked(); - } - SpanClosingListener.LISTENER.onAfterService(request); } catch (Throwable e) { log.info("Error committing blocking response", e); if (span != null) { + // the response commit was attempted and failed; report it even though this method still + // returns true below (see known gap: the boolean contract can't signal this today) + RequestContext reqCtx = span.getRequestContext(); + if (reqCtx != null) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } DECORATE.onError(span, e); DECORATE.beforeFinish(context); span.finish(); } + return true; + } + + if (span != null) { + span.getRequestContext().getTraceSegment().effectivelyBlocked(); } + SpanClosingListener.LISTENER.onAfterService(request); return true; } diff --git a/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java b/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java index aea5c512c3b..2346f3d322d 100644 --- a/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java @@ -11,6 +11,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -111,7 +112,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for Parameters/processParameters)"); } diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java index bd813c49bff..ae1f77b3cb6 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java @@ -5,6 +5,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -119,6 +120,10 @@ public static BlockingException fireFilesContentEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; @@ -150,6 +155,10 @@ public static BlockingException fireFilenamesEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java index 9660f7824a7..7e0b2b4019f 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java @@ -12,6 +12,7 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -118,7 +119,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java index 6d20a79bbb4..81894e29405 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java @@ -4,9 +4,28 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +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.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +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.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.EventType; +import datadog.trace.api.gateway.Events; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import jakarta.servlet.http.Part; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -14,10 +33,153 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class MultipartHelperTest { + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + private CallbackProvider callbackProvider; + private RequestContext reqCtx; + private BlockResponseFunction brf; + private AppSecContext appSecContext; + private TraceSegment traceSegment; + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @BeforeEach + void setUpBlockFailureFixtures() { + callbackProvider = mock(CallbackProvider.class); + traceSegment = mock(TraceSegment.class); + brf = mock(BlockResponseFunction.class); + appSecContext = mock(AppSecContext.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDownBlockFailureFixtures() { + AgentTracer.forceRegister(originalTracer); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + private void stubCallback(EventType>> event) { + doReturn((Object) (BiFunction>) (ctx, x) -> blockingFlow()) + .when(callbackProvider) + .getCallback(event); + } + + // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + + @Test + void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + + @Test + void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() + throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + @Test void returnsEmptyListForNull() { assertEquals(emptyList(), MultipartHelper.extractFilenames(null)); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java index 76dc3fcca64..e0516d6b662 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java @@ -13,6 +13,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -100,7 +101,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for UrlEncoded/decodeTo)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java index 468a93a028d..a62dbcff2c3 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java @@ -5,6 +5,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -236,6 +237,10 @@ public static BlockingException fireBodyProcessedEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart form fields)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; @@ -266,6 +271,10 @@ public static BlockingException fireFilenamesEvent(Collection parts, RequestC reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; @@ -335,6 +344,10 @@ public static BlockingException fireFilesContentEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java index 5826698454d..061b9c1ce7f 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java @@ -5,11 +5,29 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +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.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +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.Config; +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.EventType; +import datadog.trace.api.gateway.Events; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.charset.Charset; @@ -19,12 +37,193 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.BiFunction; import javax.servlet.http.Part; import org.eclipse.jetty.util.MultiPartInputStream; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class PartHelperTest { + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + private CallbackProvider callbackProvider; + private RequestContext reqCtx; + private BlockResponseFunction brf; + private AppSecContext appSecContext; + private TraceSegment traceSegment; + + // Same RequestBlockingAction used across the block-failure-reporting tests below. + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @BeforeEach + void setUpBlockFailureFixtures() { + callbackProvider = mock(CallbackProvider.class); + traceSegment = mock(TraceSegment.class); + brf = mock(BlockResponseFunction.class); + appSecContext = mock(AppSecContext.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDownBlockFailureFixtures() { + AgentTracer.forceRegister(originalTracer); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + private void stubCallback(EventType>> event) { + doReturn((Object) (BiFunction>) (ctx, x) -> blockingFlow()) + .when(callbackProvider) + .getCallback(event); + } + + // ── fireBodyProcessedEvent: report-on-failure branch ─────────────────────── + + @Test + void fireBodyProcessedEventReportsBlockFailureWhenCommitFails() throws IOException { + stubCallback(Events.get().requestBodyProcessed()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + BlockingException result = + PartHelper.fireBodyProcessedEvent(singletonList(field("a", "x")), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + verify(traceSegment, never()).effectivelyBlocked(); + } + + @Test + void fireBodyProcessedEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + stubCallback(Events.get().requestBodyProcessed()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + BlockingException result = + PartHelper.fireBodyProcessedEvent(singletonList(field("a", "x")), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireBodyProcessedEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() + throws IOException { + stubCallback(Events.get().requestBodyProcessed()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = + PartHelper.fireBodyProcessedEvent(singletonList(field("a", "x")), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + + @Test + void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + stubCallback(Events.get().requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + BlockingException result = + PartHelper.fireFilenamesEvent(singletonList(filePart("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + stubCallback(Events.get().requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + BlockingException result = + PartHelper.fireFilenamesEvent(singletonList(filePart("evil.php")), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + stubCallback(Events.get().requestFilesFilenames()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = + PartHelper.fireFilenamesEvent(singletonList(filePart("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + + @Test + void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + stubCallback(Events.get().requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + Part p = filePart("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = PartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + stubCallback(Events.get().requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + Part p = filePart("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = PartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() + throws IOException { + stubCallback(Events.get().requestFilesContent()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + Part p = filePart("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = PartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + // ── extractFilenames ──────────────────────────────────────────────────────── @Test diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java index 902db2d493a..1d5ac31e2c3 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java @@ -5,6 +5,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -119,6 +120,10 @@ public static BlockingException fireFilesContentEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; @@ -150,6 +155,10 @@ public static BlockingException fireFilenamesEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java index dcfae4aca4c..fd4e3e62f07 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java @@ -12,6 +12,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -96,7 +97,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); } @@ -137,7 +143,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for Request/getParts)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java index 376e1bd6305..45596145b4e 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java @@ -4,20 +4,182 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +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.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +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.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.EventType; +import datadog.trace.api.gateway.Events; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.BiFunction; import javax.servlet.http.Part; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class MultipartHelperTest { + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + private CallbackProvider callbackProvider; + private RequestContext reqCtx; + private BlockResponseFunction brf; + private AppSecContext appSecContext; + private TraceSegment traceSegment; + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @BeforeEach + void setUpBlockFailureFixtures() { + callbackProvider = mock(CallbackProvider.class); + traceSegment = mock(TraceSegment.class); + brf = mock(BlockResponseFunction.class); + appSecContext = mock(AppSecContext.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDownBlockFailureFixtures() { + AgentTracer.forceRegister(originalTracer); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + private void stubCallback(EventType>> event) { + doReturn((Object) (BiFunction>) (ctx, x) -> blockingFlow()) + .when(callbackProvider) + .getCallback(event); + } + + // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + + @Test + void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + + @Test + void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() + throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + @Test void returnsEmptyListForNull() { assertEquals(emptyList(), MultipartHelper.extractFilenames(null)); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java index d223d15b519..297a1be9628 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java @@ -5,6 +5,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -119,6 +120,10 @@ public static BlockingException fireFilesContentEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; @@ -150,6 +155,10 @@ public static BlockingException fireFilenamesEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java index ed769a11d7c..a8c806fe42b 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java @@ -11,6 +11,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -112,7 +113,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java index 9f580412ae8..9b5a0b8b96a 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java @@ -4,20 +4,182 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +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.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +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.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.EventType; +import datadog.trace.api.gateway.Events; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.BiFunction; import javax.servlet.http.Part; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class MultipartHelperTest { + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + private CallbackProvider callbackProvider; + private RequestContext reqCtx; + private BlockResponseFunction brf; + private AppSecContext appSecContext; + private TraceSegment traceSegment; + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @BeforeEach + void setUpBlockFailureFixtures() { + callbackProvider = mock(CallbackProvider.class); + traceSegment = mock(TraceSegment.class); + brf = mock(BlockResponseFunction.class); + appSecContext = mock(AppSecContext.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDownBlockFailureFixtures() { + AgentTracer.forceRegister(originalTracer); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + private void stubCallback(EventType>> event) { + doReturn((Object) (BiFunction>) (ctx, x) -> blockingFlow()) + .when(callbackProvider) + .getCallback(event); + } + + // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + + @Test + void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + + @Test + void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() + throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + @Test void returnsEmptyListForNull() { assertEquals(emptyList(), MultipartHelper.extractFilenames(null)); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java index 0f9e2b00df2..40a8ace31f2 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java @@ -5,6 +5,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -119,6 +120,10 @@ public static BlockingException fireFilesContentEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; @@ -150,6 +155,10 @@ public static BlockingException fireFilenamesEvent( reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java index d7c810a511b..4f917cf955a 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java @@ -11,6 +11,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -124,7 +125,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java index a32058a8624..6aa0660f1c2 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java @@ -4,20 +4,182 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +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.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +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.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.EventType; +import datadog.trace.api.gateway.Events; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.BiFunction; import javax.servlet.http.Part; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class MultipartHelperTest { + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + private CallbackProvider callbackProvider; + private RequestContext reqCtx; + private BlockResponseFunction brf; + private AppSecContext appSecContext; + private TraceSegment traceSegment; + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @BeforeEach + void setUpBlockFailureFixtures() { + callbackProvider = mock(CallbackProvider.class); + traceSegment = mock(TraceSegment.class); + brf = mock(BlockResponseFunction.class); + appSecContext = mock(AppSecContext.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDownBlockFailureFixtures() { + AgentTracer.forceRegister(originalTracer); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + private void stubCallback(EventType>> event) { + doReturn((Object) (BiFunction>) (ctx, x) -> blockingFlow()) + .when(callbackProvider) + .getCallback(event); + } + + // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + + @Test + void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + stubCallback(Events.EVENTS.requestFilesFilenames()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = + MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + + @Test + void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, times(1)).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNotNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + + @Test + void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() + throws IOException { + stubCallback(Events.EVENTS.requestFilesContent()); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + Part p = mock(Part.class); + when(p.getSubmittedFileName()).thenReturn("photo.jpg"); + when(p.getInputStream()) + .thenReturn(new ByteArrayInputStream("data".getBytes(StandardCharsets.UTF_8))); + + BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); + + assertNull(result); + verify(appSecContext, never()).reportBlockFailure(); + } + @Test void returnsEmptyListForNull() { assertEquals(emptyList(), MultipartHelper.extractFilenames(null)); diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy index 6c3c72b67e9..fd80aa18479 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy @@ -4,9 +4,16 @@ import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions import datadog.trace.instrumentation.servlet5.HtmlRumServlet import datadog.trace.instrumentation.servlet5.TestServlet5 import datadog.trace.instrumentation.servlet5.XmlRumServlet +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import org.eclipse.jetty.server.Handler import org.eclipse.jetty.server.Server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED +import static org.junit.jupiter.api.Assumptions.assumeTrue + abstract class Jetty11Test extends HttpServerTest { @Override HttpServer server() { @@ -115,6 +122,35 @@ abstract class Jetty11Test extends HttpServerTest { protected boolean useWebsocketPojoEndpoint() { false } + + def 'test blocking of multipart and urlencoded request body is pinned after block-telemetry-3 wiring #variant'() { + setup: + assumeTrue(testBlocking()) + assumeTrue(executeTest) + + def request = request(endpoint, 'POST', body) + .header('x-block-body-converted', 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + // This pins the pre-existing blocking behavior for jetty-appsec-11.0's MultipartHelper and + // RequestExtractContentParametersInstrumentation after the block-telemetry-3 changes wired + // the reportBlockFailure() call into these advice classes. reportBlockFailure() itself remains + // unreachable through this end-to-end test: the real JettyBlockingHelper.tryCommitBlockingResponse + // always returns true for a genuine attempt (see .claude-invariants.md), so no test here can + // force that branch. + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | executeTest | endpoint | body + 'urlencoded' | testBodyUrlencoded() | BODY_URLENCODED | RequestBody.create(MediaType.get('application/x-www-form-urlencoded'), 'a=x') + 'multipart' | testBodyMultipart() | BODY_MULTIPART | new MultipartBody.Builder().setType(MultipartBody.FORM).addFormDataPart('a', 'x').build() + } } class Jetty11V0ForkedTest extends Jetty11Test implements TestingGenericHttpNamingConventions.ServerV0 { diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java index 85a7b630035..d78edcbc7e2 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java @@ -12,9 +12,11 @@ import datadog.context.Context; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; import org.eclipse.jetty.http.Generator; @@ -101,6 +103,10 @@ static class CommitResponseAdvice { requestContext.getTraceSegment().effectivelyBlocked(); return true; } + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/test/groovy/Jetty70Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/test/groovy/Jetty70Test.groovy index 627f379a238..fcf802d3b72 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/test/groovy/Jetty70Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/test/groovy/Jetty70Test.groovy @@ -2,6 +2,9 @@ import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions import datadog.trace.instrumentation.servlet3.TestServlet3 +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import org.eclipse.jetty.server.Request import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.AbstractHandler @@ -12,8 +15,11 @@ import javax.servlet.ServletException import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.UNKNOWN +import static org.junit.jupiter.api.Assumptions.assumeTrue abstract class Jetty70Test extends HttpServerTest { @@ -131,6 +137,35 @@ abstract class Jetty70Test extends HttpServerTest { true } + def 'test blocking of multipart and urlencoded request body is pinned after block-telemetry-3 wiring #variant'() { + setup: + assumeTrue(testBlocking()) + assumeTrue(executeTest) + + def request = request(endpoint, 'POST', body) + .header('x-block-body-converted', 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + // This pins the pre-existing blocking behavior for jetty-appsec-7.0's UrlEncodedInstrumentation + // (and, where enabled, MultipartHelper/RequestExtractContentParametersInstrumentation) after the + // block-telemetry-3 changes wired the reportBlockFailure() call into these advice classes. + // reportBlockFailure() itself remains unreachable through this end-to-end test: the real + // JettyBlockingHelper.tryCommitBlockingResponse always returns true for a genuine attempt + // (see .claude-invariants.md), so no test here can force that branch. + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | executeTest | endpoint | body + 'urlencoded' | testBodyUrlencoded() | BODY_URLENCODED | RequestBody.create(MediaType.get('application/x-www-form-urlencoded'), 'a=x') + 'multipart' | testBodyMultipart() | BODY_MULTIPART | new MultipartBody.Builder().setType(MultipartBody.FORM).addFormDataPart('a', 'x').build() + } + static class TestHandler extends AbstractHandler { private static final TestHandler INSTANCE = new TestHandler() diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java index fbe8fd6cf6f..33c0f32b889 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java @@ -13,9 +13,11 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; import org.eclipse.jetty.http.Generator; @@ -107,7 +109,14 @@ static class CommitResponseAdvice { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = requestContext.getBlockResponseFunction(); if (brf != null) { - return brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + boolean res = brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + if (!res) { + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } + return res; } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty76Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty76Test.groovy index daab5ac7473..28042bdc01c 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty76Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty76Test.groovy @@ -2,6 +2,9 @@ import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions import datadog.trace.instrumentation.servlet3.TestServlet3 +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import org.eclipse.jetty.server.Request import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.AbstractHandler @@ -12,8 +15,11 @@ import javax.servlet.ServletException import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.UNKNOWN +import static org.junit.jupiter.api.Assumptions.assumeTrue abstract class Jetty76Test extends HttpServerTest { @@ -132,6 +138,35 @@ abstract class Jetty76Test extends HttpServerTest { true } + def 'test blocking of multipart and urlencoded request body is pinned after block-telemetry-3 wiring #variant'() { + setup: + assumeTrue(testBlocking()) + assumeTrue(executeTest) + + def request = request(endpoint, 'POST', body) + .header('x-block-body-converted', 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + // This pins the pre-existing blocking behavior for jetty-appsec-7.0's UrlEncodedInstrumentation + // and jetty-appsec-8.1.3's PartHelper after the block-telemetry-3 changes wired the + // reportBlockFailure() call into these advice classes. reportBlockFailure() itself remains + // unreachable through this end-to-end test: the real JettyBlockingHelper.tryCommitBlockingResponse + // always returns true for a genuine attempt (see .claude-invariants.md), so no test here can + // force that branch. + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | executeTest | endpoint | body + 'urlencoded' | testBodyUrlencoded() | BODY_URLENCODED | RequestBody.create(MediaType.get('application/x-www-form-urlencoded'), 'a=x') + 'multipart' | testBodyMultipart() | BODY_MULTIPART | new MultipartBody.Builder().setType(MultipartBody.FORM).addFormDataPart('a', 'x').build() + } + static class TestHandler extends AbstractHandler { private static final TestHandler INSTANCE = new TestHandler() diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/main/java/datadog/trace/instrumentation/jetty9/JettyCommitResponseInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/main/java/datadog/trace/instrumentation/jetty9/JettyCommitResponseInstrumentation.java index ebb0c9e1eec..01137c9ad0b 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/main/java/datadog/trace/instrumentation/jetty9/JettyCommitResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/main/java/datadog/trace/instrumentation/jetty9/JettyCommitResponseInstrumentation.java @@ -13,9 +13,11 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.util.concurrent.atomic.AtomicBoolean; import net.bytebuddy.asm.Advice; @@ -127,6 +129,11 @@ static class CommitResponseAdvice { if (res && _committed.get()) { requestContext.getTraceSegment().effectivelyBlocked(); return true; + } else { + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy index 848f551bf29..74c22f1ee9b 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy @@ -3,11 +3,18 @@ package datadog.trace.instrumentation.jetty9 import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.AbstractHandler import test.JettyServer import test.TestHandler +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED +import static org.junit.jupiter.api.Assumptions.assumeTrue + abstract class Jetty9Test extends HttpServerTest { @Override @@ -96,6 +103,35 @@ abstract class Jetty9Test extends HttpServerTest { boolean testWebsockets() { return super.testWebsockets() && (getServer() as JettyServer).websocketAvailable } + + def 'test blocking of multipart and urlencoded request body is pinned after block-telemetry-3 wiring #variant'() { + setup: + assumeTrue(testBlocking()) + assumeTrue(executeTest) + + def request = request(endpoint, 'POST', body) + .header('x-block-body-converted', 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + // This pins the pre-existing blocking behavior for jetty-appsec-7.0's UrlEncodedInstrumentation + // (jetty-appsec-8.1.3's multipart path is disabled for this Jetty 9.0.x module, see + // testBodyMultipart() above) after the block-telemetry-3 changes wired the reportBlockFailure() + // call into these advice classes. reportBlockFailure() itself remains unreachable through this + // end-to-end test: the real JettyBlockingHelper.tryCommitBlockingResponse always returns true + // for a genuine attempt (see .claude-invariants.md), so no test here can force that branch. + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | executeTest | endpoint | body + 'urlencoded' | testBodyUrlencoded() | BODY_URLENCODED | RequestBody.create(MediaType.get('application/x-www-form-urlencoded'), 'a=x') + 'multipart' | testBodyMultipart() | BODY_MULTIPART | new MultipartBody.Builder().setType(MultipartBody.FORM).addFormDataPart('a', 'x').build() + } } class Jetty9V0ForkedTest extends Jetty9Test implements TestingGenericHttpNamingConventions.ServerV0 { diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy index 3c4c45a9d02..7fa16189108 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy @@ -3,11 +3,18 @@ package datadog.trace.instrumentation.jetty9 import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.AbstractHandler import test.JettyServer import test.TestHandler +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED +import static org.junit.jupiter.api.Assumptions.assumeTrue + abstract class Jetty9Test extends HttpServerTest { @Override @@ -113,6 +120,35 @@ abstract class Jetty9Test extends HttpServerTest { boolean testWebsockets() { return super.testWebsockets() && (getServer() as JettyServer).websocketAvailable } + + def 'test blocking of multipart and urlencoded request body is pinned after block-telemetry-3 wiring #variant'() { + setup: + assumeTrue(testBlocking()) + assumeTrue(executeTest) + + def request = request(endpoint, 'POST', body) + .header('x-block-body-converted', 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + // This pins the pre-existing blocking behavior for jetty-appsec-9.2/9.3/9.4's MultipartHelper + // and RequestExtractContentParametersInstrumentation after the block-telemetry-3 changes wired + // the reportBlockFailure() call into these advice classes. reportBlockFailure() itself remains + // unreachable through this end-to-end test: the real JettyBlockingHelper.tryCommitBlockingResponse + // always returns true for a genuine attempt (see .claude-invariants.md), so no test here can + // force that branch. + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | executeTest | endpoint | body + 'urlencoded' | testBodyUrlencoded() | BODY_URLENCODED | RequestBody.create(MediaType.get('application/x-www-form-urlencoded'), 'a=x') + 'multipart' | testBodyMultipart() | BODY_MULTIPART | new MultipartBody.Builder().setType(MultipartBody.FORM).addFormDataPart('a', 'x').build() + } } class Jetty9V0ForkedTest extends Jetty9Test implements TestingGenericHttpNamingConventions.ServerV0 { diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java index 102c70acd98..d2fd85d5935 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java @@ -12,6 +12,7 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -77,7 +78,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (multipart file upload)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java index dd920e9849b..f46dbb503cf 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java @@ -12,6 +12,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -113,7 +114,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java index 4fa50a6a58f..4a7a3c613a4 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java @@ -14,6 +14,7 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -74,7 +75,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/test/groovy/datadog/trace/instrumentation/liberty20/Liberty20Test.groovy b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/test/groovy/datadog/trace/instrumentation/liberty20/Liberty20Test.groovy index 2e2a5a93803..ed548e47682 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/test/groovy/datadog/trace/instrumentation/liberty20/Liberty20Test.groovy +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/test/groovy/datadog/trace/instrumentation/liberty20/Liberty20Test.groovy @@ -9,8 +9,12 @@ import datadog.trace.api.config.GeneralConfig import datadog.trace.api.env.CapturedEnvironment import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.core.DDSpan +import okhttp3.MediaType +import okhttp3.RequestBody import spock.lang.IgnoreIf +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT_ERROR @@ -157,6 +161,51 @@ abstract class Liberty20Test extends HttpServerTest { it.tags['appsec.blocked'] == 'true' } != null } + + // Pins the pre-existing blocking behavior of ParsePostDataInstrumentation and + // ParseParametersInstrumentation after the block-telemetry-3 changes, which added a + // reportBlockFailure() call when BlockResponseFunction#tryCommitBlockingResponse returns + // false. The real Liberty BlockResponseFunction (LibertyDecorator$LibertyBlockResponseFunction) + // delegates to a void helper that swallows failures and then always returns true for a genuine + // commit attempt, so the reportBlockFailure() branch itself remains unreachable through this + // end-to-end test: this test only verifies the blocking response is produced exactly as before. + // + // GetPartsInstrumentation (liberty-20.0 only) is not separately covered here: the shared + // HttpServerTest file-upload callback (requestFilesFilenamesCb) never returns a blocking Flow + // action, so its blocking branch cannot be exercised through existing test infrastructure + // without changing that shared fixture, which is out of scope for this task. + @IgnoreIf({ !instance.testBlocking() }) + def 'test blocking of request body variant #variant pins block-telemetry-3 behavior'() { + setup: + def request = request( + endpoint, 'POST', + RequestBody.create(MediaType.get(contentType), body)) + .header(IG_BODY_CONVERTED_HEADER, 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.header('Content-type') =~ /(?i)\Aapplication\/json(?:;\s?charset=(?:utf-8|iso-8859-1))?\z/ + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | endpoint | contentType | body + 'urlencoded' | BODY_URLENCODED | 'application/x-www-form-urlencoded' | 'a=x' + 'multipart' | BODY_MULTIPART | MULTIPART_TEST_CONTENT_TYPE | MULTIPART_TEST_BODY + } + + private static final String MULTIPART_TEST_CONTENT_TYPE = + 'multipart/form-data; charset=utf-8; boundary=------------------------943d3207457896a3' + private static final String MULTIPART_TEST_BODY = + '--------------------------943d3207457896a3\r\n' + + 'Content-Disposition: form-data; name="a"\r\n' + + '\r\n' + + 'x\r\n' + + '--------------------------943d3207457896a3--' } // make it forked because there are instrumentation errors when we shutdown and diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java index d29c4fcdfb0..7033d65d8bc 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java @@ -12,6 +12,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -113,7 +114,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java index 6c5cfda83d8..8eb5b40a74f 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java @@ -14,6 +14,7 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -74,7 +75,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/test/groovy/datadog/trace/instrumentation/liberty23/Liberty23Test.groovy b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/test/groovy/datadog/trace/instrumentation/liberty23/Liberty23Test.groovy index 1fd92807b6a..795d93b0f83 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/test/groovy/datadog/trace/instrumentation/liberty23/Liberty23Test.groovy +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/test/groovy/datadog/trace/instrumentation/liberty23/Liberty23Test.groovy @@ -6,8 +6,12 @@ import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions import datadog.trace.core.DDSpan +import okhttp3.MediaType +import okhttp3.RequestBody import spock.lang.IgnoreIf +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS abstract class Liberty23Test extends HttpServerTest { @@ -147,6 +151,51 @@ abstract class Liberty23Test extends HttpServerTest { it.tags['appsec.blocked'] == 'true' } != null } + + // Pins the pre-existing blocking behavior of ParsePostDataInstrumentation and + // ParseParametersInstrumentation after the block-telemetry-3 changes, which added a + // reportBlockFailure() call when BlockResponseFunction#tryCommitBlockingResponse returns + // false. The real Liberty BlockResponseFunction (LibertyDecorator$LibertyBlockResponseFunction) + // delegates to a void helper that swallows failures and then always returns true for a genuine + // commit attempt, so the reportBlockFailure() branch itself remains unreachable through this + // end-to-end test: this test only verifies the blocking response is produced exactly as before. + // + // Guarded by testBlocking() like the inherited generic variant test: this Liberty jakarta + // module already disables request-body blocking end-to-end (testBlocking() returns false here, + // a pre-existing limitation unrelated to block-telemetry-3), so this test is currently skipped + // for this class, consistent with that existing, already-disabled behavior. + @IgnoreIf({ !instance.testBlocking() }) + def 'test blocking of request body variant #variant pins block-telemetry-3 behavior'() { + setup: + def request = request( + endpoint, 'POST', + RequestBody.create(MediaType.get(contentType), body)) + .header(IG_BODY_CONVERTED_HEADER, 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.header('Content-type') =~ /(?i)\Aapplication\/json(?:;\s?charset=(?:utf-8|iso-8859-1))?\z/ + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + !handlerRan + + where: + variant | endpoint | contentType | body + 'urlencoded' | BODY_URLENCODED | 'application/x-www-form-urlencoded' | 'a=x' + 'multipart' | BODY_MULTIPART | MULTIPART_TEST_CONTENT_TYPE | MULTIPART_TEST_BODY + } + + private static final String MULTIPART_TEST_CONTENT_TYPE = + 'multipart/form-data; charset=utf-8; boundary=------------------------943d3207457896a3' + private static final String MULTIPART_TEST_BODY = + '--------------------------943d3207457896a3\r\n' + + 'Content-Disposition: form-data; name="a"\r\n' + + '\r\n' + + 'x\r\n' + + '--------------------------943d3207457896a3--' } @IgnoreIf({ diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java index d170a53cc22..40a6988b7e7 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java @@ -15,6 +15,7 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -129,7 +130,12 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } } t = new BlockingException("Blocked request (for HttpMessageConverter/read)"); } @@ -158,7 +164,12 @@ public static void before( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } } throw new BlockingException("Blocked response (for HttpMessageConverter/write)"); } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java index e82b9b55366..f2e839e0263 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java @@ -13,6 +13,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.iast.IastPostProcessorFactory; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -167,7 +168,12 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java index cc90713ea2a..25ae61beebc 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java @@ -13,6 +13,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.iast.IastPostProcessorFactory; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -127,7 +128,12 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy index 43309688350..c0b2ce0a4de 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy @@ -1,9 +1,14 @@ package datadog.trace.instrumentation.springweb +import datadog.appsec.api.blocking.BlockingContentType +import datadog.appsec.api.blocking.BlockingException import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.api.appsec.AppSecContext +import datadog.trace.api.gateway.BlockResponseFunction import datadog.trace.api.gateway.Flow import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.api.internal.TraceSegment import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.bootstrap.instrumentation.api.TagContext import org.springframework.http.MediaType @@ -11,6 +16,7 @@ import org.springframework.http.converter.ByteArrayHttpMessageConverter import org.springframework.http.converter.FormHttpMessageConverter import org.springframework.http.converter.StringHttpMessageConverter import org.springframework.mock.http.MockHttpInputMessage +import org.springframework.mock.http.MockHttpOutputMessage import org.springframework.util.MultiValueMap import java.nio.charset.StandardCharsets @@ -90,4 +96,133 @@ class HttpMessageConverterInstrumentationTest extends InstrumentationSpecificati published.getFirst('value') == 'object' published.getFirst('another') == 'value2' } + + // Hand-written AppSecContext stub that records whether reportBlockFailure() was invoked. + private static class RecordingAppSecContext implements AppSecContext { + boolean blockFailureReported = false + + @Override + boolean isManuallyKept() { + return false + } + + @Override + void reportBlockFailure() { + blockFailureReported = true + } + } + + // Hand-written BlockResponseFunction stub whose commit outcome is controlled by the test. + private static class FixedOutcomeBlockResponseFunction implements BlockResponseFunction { + private final boolean commitSucceeds + + FixedOutcomeBlockResponseFunction(boolean commitSucceeds) { + this.commitSucceeds = commitSucceeds + } + + @Override + boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + return commitSucceeds + } + } + + // Activates a span with a recording appsec context, wires the given event to a callback that + // always requests blocking, and configures the block response commit outcome. Returns the + // recording appsec context and the activated scope so the caller can assert on the former and + // close the latter. + private List setupBlockFailureScenario(def event, boolean commitSucceeds) { + def appSecContext = new RecordingAppSecContext() + TagContext ctx = new TagContext().withRequestContextDataAppSec(appSecContext) + def blockedSpan = AgentTracer.startSpan('test', 'test-blocked-span', ctx) + def blockedScope = AgentTracer.activateSpan(blockedSpan) + def blockedReqCtx = blockedSpan.spanContext() as RequestContext + blockedReqCtx.setBlockResponseFunction(new FixedOutcomeBlockResponseFunction(commitSucceeds)) + ss.reset() + ss.registerCallback(event, { RequestContext c, Object body -> + new Flow.ResultFlow(null) { + @Override + Flow.Action getAction() { + return new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO) + } + } + } as BiFunction>) + [appSecContext, blockedScope] + } + + void 'read reports block failure via stub appsec context when commit fails'() { + given: + def (appSecContext, blockedScope) = setupBlockFailureScenario(EVENTS.requestBodyProcessed(), false) + def converter = new FormHttpMessageConverter() + def raw = 'value=object' + def message = new MockHttpInputMessage(raw.getBytes(StandardCharsets.UTF_8)) + message.headers.contentType = MediaType.APPLICATION_FORM_URLENCODED + + when: + converter.read(MultiValueMap, message) + + then: + thrown(BlockingException) + appSecContext.blockFailureReported == true + + cleanup: + blockedScope?.close() + } + + void 'read does not report block failure when commit succeeds'() { + given: + def (appSecContext, blockedScope) = setupBlockFailureScenario(EVENTS.requestBodyProcessed(), true) + def converter = new FormHttpMessageConverter() + def raw = 'value=object' + def message = new MockHttpInputMessage(raw.getBytes(StandardCharsets.UTF_8)) + message.headers.contentType = MediaType.APPLICATION_FORM_URLENCODED + + when: + converter.read(MultiValueMap, message) + + then: + thrown(BlockingException) + appSecContext.blockFailureReported == false + + cleanup: + blockedScope?.close() + } + + void 'write reports block failure via stub appsec context when commit fails'() { + given: + def (appSecContext, blockedScope) = setupBlockFailureScenario(EVENTS.responseBody(), false) + def converter = new StringHttpMessageConverter() + def message = new MockHttpOutputMessage() + + when: + converter.write('example', MediaType.TEXT_PLAIN, message) + + then: + thrown(BlockingException) + appSecContext.blockFailureReported == true + + cleanup: + blockedScope?.close() + } + + void 'write does not report block failure when commit succeeds'() { + given: + def (appSecContext, blockedScope) = setupBlockFailureScenario(EVENTS.responseBody(), true) + def converter = new StringHttpMessageConverter() + def message = new MockHttpOutputMessage() + + when: + converter.write('example', MediaType.TEXT_PLAIN, message) + + then: + thrown(BlockingException) + appSecContext.blockFailureReported == false + + cleanup: + blockedScope?.close() + } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy index 89b0389c3da..3d25ffd2509 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy @@ -29,6 +29,7 @@ import test.SetupSpecHelper import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import static org.junit.jupiter.api.Assumptions.assumeTrue import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN @@ -263,6 +264,36 @@ class SpringBootBasedTest extends HttpServerTest body = null } + def 'test blocking of request for matrix parameters'() { + // This pins the unchanged blocking behavior for TemplateAndMatrixVariablesInstrumentation's + // matrix-variable path. reportBlockFailure() itself is not observable through this black-box + // HTTP test because spring-webmvc does not own its own BlockResponseFunction: it delegates to + // the underlying container's (Tomcat here), whose contract always returns true for a genuine + // attempt. + setup: + assumeTrue(testBlocking()) + + def request = request(MATRIX_PARAM, 'GET', null) + .header(IG_PARAMETERS_BLOCK_HEADER, 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.header('Content-type') =~ /(?i)\Aapplication\/json(?:;\s?charset=utf-8)?\z/ + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def trace = TEST_WRITER.get(0) + def rootSpan = trace.find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } + def 'template var is pushed to IG'() { setup: def request = request(PATH_PARAM, 'GET', null).header(IG_EXTRA_SPAN_NAME_HEADER, 'appsec-span').build() diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java index 368e91303ad..66f0a99093d 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java @@ -3,6 +3,7 @@ import static datadog.trace.api.gateway.Events.EVENTS; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -98,7 +99,12 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java index 89ac5f23e85..d9be60b3af1 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java @@ -3,6 +3,7 @@ import static datadog.trace.api.gateway.Events.EVENTS; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -66,7 +67,12 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java index 8f34ab5e9f9..e33420eb22f 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java @@ -14,6 +14,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -91,7 +92,14 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + // NOTE: UndertowBlockResponseFunction currently always returns true; this branch is + // expected to be unreachable until that is fixed separately (tech-debt follow-up). + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (t == null) { t = new BlockingException("Blocked request (for FormEncodedDataParser/doParse)"); } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java index 3f0313464d3..d9cfbd2afdd 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java @@ -14,6 +14,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -106,6 +107,14 @@ static void after( if (blockResponseFunction != null) { boolean success = blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!success) { + // NOTE: UndertowBlockResponseFunction currently always returns true; this branch is + // expected to be unreachable until that is fixed separately (tech-debt follow-up). + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (success && t == null) { t = new BlockingException( @@ -134,6 +143,15 @@ static void after( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null && t == null) { boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!success) { + // NOTE: UndertowBlockResponseFunction currently always returns true; this branch + // is expected to be unreachable until that is fixed separately (tech-debt + // follow-up). + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (success) { t = new BlockingException("Blocked request (multipart file upload)"); } @@ -153,6 +171,15 @@ static void after( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null && t == null) { boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (!success) { + // NOTE: UndertowBlockResponseFunction currently always returns true; this branch + // is expected to be unreachable until that is fixed separately (tech-debt + // follow-up). + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } if (success) { t = new BlockingException("Blocked request (multipart file upload content)"); } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy index 846389417ba..54b39c886c0 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/test/groovy/UndertowServletTest.groovy @@ -2,7 +2,9 @@ import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.base.WebsocketServer import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.bootstrap.blocking.BlockingActionHelper import datadog.trace.bootstrap.instrumentation.api.Tags +import datadog.trace.core.DDSpan import io.undertow.Handlers import io.undertow.Undertow import io.undertow.UndertowOptions @@ -11,11 +13,17 @@ import io.undertow.servlet.api.DeploymentManager import io.undertow.servlet.api.ServletContainer import io.undertow.servlet.api.ServletInfo import io.undertow.websockets.jsr.WebSocketDeploymentInfo +import okhttp3.MediaType +import okhttp3.RequestBody import spock.lang.IgnoreIf import javax.servlet.MultipartConfigElement import java.nio.ByteBuffer +import static datadog.trace.bootstrap.blocking.BlockingActionHelper.TemplateType.JSON +import static java.nio.charset.StandardCharsets.UTF_8 +import static org.junit.jupiter.api.Assumptions.assumeTrue + import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED @@ -38,6 +46,15 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.WEBSOC abstract class UndertowServletTest extends HttpServerTest { private static final CONTEXT = "ctx" + private final static String MULTIPART_CONTENT_TYPE = + 'multipart/form-data; charset=utf-8; boundary=------------------------943d3207457896a3' + private final static String MULTIPART_BODY = + '--------------------------943d3207457896a3\r\n' + + 'Content-Disposition: form-data; name="a"\r\n' + + '\r\n' + + 'x\r\n' + + '--------------------------943d3207457896a3--' + class UndertowServer implements WebsocketServer { def port = 0 Undertow undertowServer @@ -314,6 +331,56 @@ abstract class UndertowServletTest extends HttpServerTest { body = null } + // Pins the block-telemetry-3 wiring added to FormDataParserInstrumentation (urlencoded body, + // DoParseAdvice.after) and MultiPartUploadHandlerInstrumentation (multipart body, + // ParseBlockingAdvice.after): both now check the boolean returned by + // BlockResponseFunction#tryCommitBlockingResponse and report a block failure to AppSecContext + // when it returns false. In this test the reportBlockFailure() branch itself stays unreachable, + // because UndertowBlockResponseFunction#tryCommitBlockingResponse always returns true + // unconditionally, even on its async dispatch path (see .claude-invariants.md). So this test + // only pins the unchanged, already-passing blocking behavior for both request body variants + // post block-telemetry-3, the same kind of documented gap as Netty's block-failure tests. + def "test blocking of request body parsed by undertow for variant #variant"() { + setup: + assumeTrue(testBlocking()) + assumeTrue(executeTest) + + def request = request( + endpoint, 'POST', + RequestBody.create(MediaType.get(contentType), body)) + .header(IG_BODY_CONVERTED_HEADER, 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.header('Content-type') =~ /(?i)\Aapplication\/json(?:;\s?charset=(?:utf-8|iso-8859-1))?\z/ + + def text = response.body().charStream().text + text.contains('"title":"You\'ve been blocked"') + text.getBytes(UTF_8).length == BlockingActionHelper.getTemplate(JSON).length + + !handlerRan + + TEST_WRITER.waitForTraces(1) + + then: + List spans = TEST_WRITER.flatten() + spans.find { + it.tags['http.status_code'] == 413 + } != null + spans.find { + it.tags['appsec.blocked'] == 'true' + } != null + + where: + variant | executeTest | endpoint | contentType | body + 'urlencoded' | testBodyUrlencoded() | BODY_URLENCODED | 'application/x-www-form-urlencoded' | 'a=x' + 'multipart' | testBodyMultipart() | BODY_MULTIPART | MULTIPART_CONTENT_TYPE | MULTIPART_BODY + } + @Override void handlerSpan(TraceAssert trace, ServerEndpoint endpoint = SUCCESS) { trace.span { From 58ff0005234903456029809eb621be6145a4c526 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 17 Sep 2026 11:33:29 +0200 Subject: [PATCH 2/6] Centralize block_failure reporting via BlockResponseFunction overload Replace the repeated inline pattern (commit blocking response, then manually check AppSecContext and call reportBlockFailure() on failure) with the new BlockResponseFunction.tryCommitBlockingResponse(RequestContext, Flow.Action.RequestBlockingAction) default overload added in block-telemetry-2b (#12519). That overload already performs the AppSecContext lookup and reportBlockFailure() call internally, so call sites only need to pass the RequestContext instead of the TraceSegment. Applies to the 37 call sites identified as mechanically substitutable: akka-http-10.0, grizzly-http-2.3.20, jetty-appsec (7.0, 8.1.3, 9.2, 9.3, 9.4, 11.0), jetty-server (7.0, 7.6), liberty (20.0, 23.0), spring-webmvc (3.1, 6.0), and undertow-2.0. Excludes GrizzlyBlockingHelper (grizzly-2.0, manual reflection-based commit) and jetty-server-9.0's JettyCommitResponseInstrumentation (compound res && _committed.get() condition, not mechanically equivalent to the new overload's plain boolean return). --- .../appsec/BlockingResponseHelper.java | 13 +--- .../akkahttp/appsec/UnmarshallerHelpers.java | 25 ++++---- .../ParsedBodyParametersInstrumentation.java | 8 +-- .../jetty11/MultipartHelper.java | 13 +--- ...tractContentParametersInstrumentation.java | 8 +-- .../jetty11/MultipartHelperTest.java | 41 ++++++------- .../jetty70/UrlEncodedInstrumentation.java | 8 +-- .../instrumentation/jetty8/PartHelper.java | 19 +----- .../jetty8/PartHelperTest.java | 60 +++++++++---------- .../jetty92/MultipartHelper.java | 13 +--- ...tractContentParametersInstrumentation.java | 15 +---- .../jetty92/MultipartHelperTest.java | 41 ++++++------- .../jetty93/MultipartHelper.java | 13 +--- ...tractContentParametersInstrumentation.java | 8 +-- .../jetty93/MultipartHelperTest.java | 41 ++++++------- .../jetty94/MultipartHelper.java | 13 +--- ...tractContentParametersInstrumentation.java | 8 +-- .../jetty94/MultipartHelperTest.java | 41 ++++++------- .../JettyCommitResponseInstrumentation.java | 9 +-- .../JettyCommitResponseInstrumentation.java | 11 +--- .../liberty20/GetPartsInstrumentation.java | 8 +-- .../ParseParametersInstrumentation.java | 8 +-- .../ParsePostDataInstrumentation.java | 8 +-- .../ParseParametersInstrumentation.java | 8 +-- .../ParsePostDataInstrumentation.java | 8 +-- .../HttpMessageConverterInstrumentation.java | 15 +---- ...lateAndMatrixVariablesInstrumentation.java | 8 +-- ...ateVariablesUrlHandlerInstrumentation.java | 8 +-- .../springweb6/HandleMatchAdvice.java | 8 +-- .../InterceptorPreHandleAdvice.java | 8 +-- .../FormDataParserInstrumentation.java | 10 +--- ...MultiPartUploadHandlerInstrumentation.java | 34 +---------- 32 files changed, 145 insertions(+), 394 deletions(-) diff --git a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java index fe81b755586..9ec7596486e 100644 --- a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java +++ b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/BlockingResponseHelper.java @@ -13,11 +13,9 @@ import akka.http.scaladsl.model.StatusCodes; import akka.util.ByteString; import datadog.appsec.api.blocking.BlockingContentType; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.blocking.BlockingActionHelper; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.akkahttp.AkkaHttpServerHeaders; @@ -44,16 +42,7 @@ public static HttpResponse handleFinishForWaf(final AgentSpan span, final HttpRe if (action instanceof Flow.Action.RequestBlockingAction) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; if (brf instanceof AkkaBlockResponseFunction) { - if (!brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba)) { - // Safe to report here: both wiring points (DatadogAsyncHandlerWrapper.apply and - // DatadogServerRequestResponseFlowWrapper's onPush) call finishSpan/onRequestEnded - // synchronously, immediately after this method returns, in the same callback/thread, - // with no scheduling boundary in between. - Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(requestContext, rba); HttpResponse altResponse = ((AkkaBlockResponseFunction) brf).maybeCreateAlternativeResponse(); if (altResponse != null) { diff --git a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java index 7f3a7425902..1bd3779e1e2 100644 --- a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java +++ b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/appsec/UnmarshallerHelpers.java @@ -15,7 +15,6 @@ import akka.stream.Materializer; import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -616,21 +615,17 @@ static BlockingException tryBlock( if (brf == null) { return null; } - boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // Conditional async-race gap (same class as netty-blocking.md §10/§11, but via + // Future.map/.recover/.thenApply on a Scala ExecutionContext instead of + // eventLoop().execute()): the block-failure report below is only guaranteed to run before + // GatewayBridge.onRequestEnded/end-of-request telemetry is emitted when the route's + // response Future causally depends (via flatMap) on this same unmarshalling Future - the + // idiomatic Akka HTTP usage. If the app decouples unmarshalling (used only for a side + // effect) from response production, or triggers toStrict() conversions independently of + // the main response chain, this report can arrive after end-of-request telemetry has + // already been emitted. This is not fixed here; see the KB entry for akka-http. + boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); if (!success) { - // Conditional async-race gap (same class as netty-blocking.md §10/§11, but via - // Future.map/.recover/.thenApply on a Scala ExecutionContext instead of - // eventLoop().execute()): reportBlockFailure() below is only guaranteed to run before - // GatewayBridge.onRequestEnded/end-of-request telemetry is emitted when the route's - // response Future causally depends (via flatMap) on this same unmarshalling Future - the - // idiomatic Akka HTTP usage. If the app decouples unmarshalling (used only for a side - // effect) from response production, or triggers toStrict() conversions independently of - // the main response chain, this report can arrive after end-of-request telemetry has - // already been emitted. This is not fixed here; see the KB entry for akka-http. - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } return null; } if (brf instanceof AkkaBlockResponseFunction) { diff --git a/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java b/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java index 2346f3d322d..e141d7e43e8 100644 --- a/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/main/java/datadog/trace/instrumentation/grizzlyhttp232/ParsedBodyParametersInstrumentation.java @@ -11,7 +11,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -112,12 +111,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for Parameters/processParameters)"); } diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java index ae1f77b3cb6..4971d42f8d0 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java @@ -5,7 +5,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -116,14 +115,10 @@ public static BlockingException fireFilesContentEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; @@ -151,14 +146,10 @@ public static BlockingException fireFilenamesEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java index 7e0b2b4019f..ed06d6201d5 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java @@ -12,7 +12,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -119,12 +118,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java index 81894e29405..7d747624262 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/java/datadog/trace/instrumentation/jetty11/MultipartHelperTest.java @@ -16,7 +16,6 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.EventType; @@ -44,7 +43,6 @@ class MultipartHelperTest { private CallbackProvider callbackProvider; private RequestContext reqCtx; private BlockResponseFunction brf; - private AppSecContext appSecContext; private TraceSegment traceSegment; private static final Flow.Action.RequestBlockingAction RBA = @@ -55,11 +53,9 @@ void setUpBlockFailureFixtures() { callbackProvider = mock(CallbackProvider.class); traceSegment = mock(TraceSegment.class); brf = mock(BlockResponseFunction.class); - appSecContext = mock(AppSecContext.class); reqCtx = mock(RequestContext.class); when(reqCtx.getTraceSegment()).thenReturn(traceSegment); when(reqCtx.getBlockResponseFunction()).thenReturn(brf); - when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); @@ -91,34 +87,34 @@ private void stubCallback(EventType .getCallback(event); } - // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + // ── fireFilenamesEvent: wiring to BlockResponseFunction ───────────────────── @Test - void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + void fireFilenamesEventCommitFails() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + void fireFilenamesEventCommitSucceeds() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + void fireFilenamesEventNoBlockResponseFunction() { stubCallback(Events.EVENTS.requestFilesFilenames()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -126,15 +122,15 @@ void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } - // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + // ── fireFilesContentEvent: wiring to BlockResponseFunction ────────────────── @Test - void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + void fireFilesContentEventCommitFails() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -144,13 +140,13 @@ void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOExceptio BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + void fireFilesContentEventCommitSucceeds() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -160,12 +156,11 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws I BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() - throws IOException { + void fireFilesContentEventNoBlockResponseFunction() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -177,7 +172,7 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } @Test diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java index e0516d6b662..1d87bf32ec8 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-7.0/src/main/java/datadog/trace/instrumentation/jetty70/UrlEncodedInstrumentation.java @@ -13,7 +13,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -101,12 +100,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for UrlEncoded/decodeTo)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java index a62dbcff2c3..d685c84fa34 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java @@ -5,7 +5,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -233,14 +232,10 @@ public static BlockingException fireBodyProcessedEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart form fields)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; @@ -267,14 +262,10 @@ public static BlockingException fireFilenamesEvent(Collection parts, RequestC Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; @@ -340,14 +331,10 @@ public static BlockingException fireFilesContentEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java index 061b9c1ce7f..fbd57c00ea2 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java @@ -18,7 +18,6 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.EventType; @@ -50,7 +49,6 @@ class PartHelperTest { private CallbackProvider callbackProvider; private RequestContext reqCtx; private BlockResponseFunction brf; - private AppSecContext appSecContext; private TraceSegment traceSegment; // Same RequestBlockingAction used across the block-failure-reporting tests below. @@ -62,11 +60,9 @@ void setUpBlockFailureFixtures() { callbackProvider = mock(CallbackProvider.class); traceSegment = mock(TraceSegment.class); brf = mock(BlockResponseFunction.class); - appSecContext = mock(AppSecContext.class); reqCtx = mock(RequestContext.class); when(reqCtx.getTraceSegment()).thenReturn(traceSegment); when(reqCtx.getBlockResponseFunction()).thenReturn(brf); - when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); @@ -98,36 +94,35 @@ private void stubCallback(EventType .getCallback(event); } - // ── fireBodyProcessedEvent: report-on-failure branch ─────────────────────── + // ── fireBodyProcessedEvent: wiring to BlockResponseFunction ───────────────── @Test - void fireBodyProcessedEventReportsBlockFailureWhenCommitFails() throws IOException { + void fireBodyProcessedEventCommitFails() throws IOException { stubCallback(Events.get().requestBodyProcessed()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); BlockingException result = PartHelper.fireBodyProcessedEvent(singletonList(field("a", "x")), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); verify(traceSegment, never()).effectivelyBlocked(); } @Test - void fireBodyProcessedEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + void fireBodyProcessedEventCommitSucceeds() throws IOException { stubCallback(Events.get().requestBodyProcessed()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); BlockingException result = PartHelper.fireBodyProcessedEvent(singletonList(field("a", "x")), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireBodyProcessedEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() - throws IOException { + void fireBodyProcessedEventNoBlockResponseFunction() throws IOException { stubCallback(Events.get().requestBodyProcessed()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -135,37 +130,37 @@ void fireBodyProcessedEventDoesNotReportBlockFailureWhenNoBlockResponseFunction( PartHelper.fireBodyProcessedEvent(singletonList(field("a", "x")), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } - // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + // ── fireFilenamesEvent: wiring to BlockResponseFunction ───────────────────── @Test - void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + void fireFilenamesEventCommitFails() { stubCallback(Events.get().requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); BlockingException result = PartHelper.fireFilenamesEvent(singletonList(filePart("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + void fireFilenamesEventCommitSucceeds() { stubCallback(Events.get().requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); BlockingException result = PartHelper.fireFilenamesEvent(singletonList(filePart("evil.php")), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + void fireFilenamesEventNoBlockResponseFunction() { stubCallback(Events.get().requestFilesFilenames()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -173,15 +168,15 @@ void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { PartHelper.fireFilenamesEvent(singletonList(filePart("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } - // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + // ── fireFilesContentEvent: wiring to BlockResponseFunction ────────────────── @Test - void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + void fireFilesContentEventCommitFails() throws IOException { stubCallback(Events.get().requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); Part p = filePart("photo.jpg"); when(p.getInputStream()) @@ -190,13 +185,13 @@ void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOExceptio BlockingException result = PartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + void fireFilesContentEventCommitSucceeds() throws IOException { stubCallback(Events.get().requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); Part p = filePart("photo.jpg"); when(p.getInputStream()) @@ -205,12 +200,11 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws I BlockingException result = PartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() - throws IOException { + void fireFilesContentEventNoBlockResponseFunction() throws IOException { stubCallback(Events.get().requestFilesContent()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -221,7 +215,7 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() BlockingException result = PartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } // ── extractFilenames ──────────────────────────────────────────────────────── diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java index 1d5ac31e2c3..1d9b26b67ee 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java @@ -5,7 +5,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -116,14 +115,10 @@ public static BlockingException fireFilesContentEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; @@ -151,14 +146,10 @@ public static BlockingException fireFilenamesEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java index fd4e3e62f07..8f6bda56141 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java @@ -12,7 +12,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -97,12 +96,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); } @@ -143,12 +137,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for Request/getParts)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java index 45596145b4e..b0c15eac33d 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/java/datadog/trace/instrumentation/jetty92/MultipartHelperTest.java @@ -16,7 +16,6 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.EventType; @@ -44,7 +43,6 @@ class MultipartHelperTest { private CallbackProvider callbackProvider; private RequestContext reqCtx; private BlockResponseFunction brf; - private AppSecContext appSecContext; private TraceSegment traceSegment; private static final Flow.Action.RequestBlockingAction RBA = @@ -55,11 +53,9 @@ void setUpBlockFailureFixtures() { callbackProvider = mock(CallbackProvider.class); traceSegment = mock(TraceSegment.class); brf = mock(BlockResponseFunction.class); - appSecContext = mock(AppSecContext.class); reqCtx = mock(RequestContext.class); when(reqCtx.getTraceSegment()).thenReturn(traceSegment); when(reqCtx.getBlockResponseFunction()).thenReturn(brf); - when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); @@ -91,34 +87,34 @@ private void stubCallback(EventType .getCallback(event); } - // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + // ── fireFilenamesEvent: wiring to BlockResponseFunction ───────────────────── @Test - void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + void fireFilenamesEventCommitFails() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + void fireFilenamesEventCommitSucceeds() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + void fireFilenamesEventNoBlockResponseFunction() { stubCallback(Events.EVENTS.requestFilesFilenames()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -126,15 +122,15 @@ void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } - // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + // ── fireFilesContentEvent: wiring to BlockResponseFunction ────────────────── @Test - void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + void fireFilesContentEventCommitFails() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -144,13 +140,13 @@ void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOExceptio BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + void fireFilesContentEventCommitSucceeds() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -160,12 +156,11 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws I BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() - throws IOException { + void fireFilesContentEventNoBlockResponseFunction() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -177,7 +172,7 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } @Test diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java index 297a1be9628..335f0e3dd2c 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java @@ -5,7 +5,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -116,14 +115,10 @@ public static BlockingException fireFilesContentEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; @@ -151,14 +146,10 @@ public static BlockingException fireFilenamesEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java index a8c806fe42b..6a8258dcbdf 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java @@ -11,7 +11,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -113,12 +112,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java index 9b5a0b8b96a..9c58139cbd4 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/java/datadog/trace/instrumentation/jetty93/MultipartHelperTest.java @@ -16,7 +16,6 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.EventType; @@ -44,7 +43,6 @@ class MultipartHelperTest { private CallbackProvider callbackProvider; private RequestContext reqCtx; private BlockResponseFunction brf; - private AppSecContext appSecContext; private TraceSegment traceSegment; private static final Flow.Action.RequestBlockingAction RBA = @@ -55,11 +53,9 @@ void setUpBlockFailureFixtures() { callbackProvider = mock(CallbackProvider.class); traceSegment = mock(TraceSegment.class); brf = mock(BlockResponseFunction.class); - appSecContext = mock(AppSecContext.class); reqCtx = mock(RequestContext.class); when(reqCtx.getTraceSegment()).thenReturn(traceSegment); when(reqCtx.getBlockResponseFunction()).thenReturn(brf); - when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); @@ -91,34 +87,34 @@ private void stubCallback(EventType .getCallback(event); } - // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + // ── fireFilenamesEvent: wiring to BlockResponseFunction ───────────────────── @Test - void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + void fireFilenamesEventCommitFails() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + void fireFilenamesEventCommitSucceeds() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + void fireFilenamesEventNoBlockResponseFunction() { stubCallback(Events.EVENTS.requestFilesFilenames()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -126,15 +122,15 @@ void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } - // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + // ── fireFilesContentEvent: wiring to BlockResponseFunction ────────────────── @Test - void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + void fireFilesContentEventCommitFails() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -144,13 +140,13 @@ void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOExceptio BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + void fireFilesContentEventCommitSucceeds() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -160,12 +156,11 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws I BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() - throws IOException { + void fireFilesContentEventNoBlockResponseFunction() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -177,7 +172,7 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } @Test diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java index 40a8ace31f2..e13dd8ac139 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java @@ -5,7 +5,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -116,14 +115,10 @@ public static BlockingException fireFilesContentEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file content)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; @@ -151,14 +146,10 @@ public static BlockingException fireFilenamesEvent( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + if (brf.tryCommitBlockingResponse(reqCtx, rba)) { reqCtx.getTraceSegment().effectivelyBlocked(); return new BlockingException("Blocked request (multipart file upload)"); } - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } return null; diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java index 4f917cf955a..d2087d02070 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java @@ -11,7 +11,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -125,12 +124,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for Request/extractContentParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java index 6aa0660f1c2..5c79fbb5341 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/java/datadog/trace/instrumentation/jetty94/MultipartHelperTest.java @@ -16,7 +16,6 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.EventType; @@ -44,7 +43,6 @@ class MultipartHelperTest { private CallbackProvider callbackProvider; private RequestContext reqCtx; private BlockResponseFunction brf; - private AppSecContext appSecContext; private TraceSegment traceSegment; private static final Flow.Action.RequestBlockingAction RBA = @@ -55,11 +53,9 @@ void setUpBlockFailureFixtures() { callbackProvider = mock(CallbackProvider.class); traceSegment = mock(TraceSegment.class); brf = mock(BlockResponseFunction.class); - appSecContext = mock(AppSecContext.class); reqCtx = mock(RequestContext.class); when(reqCtx.getTraceSegment()).thenReturn(traceSegment); when(reqCtx.getBlockResponseFunction()).thenReturn(brf); - when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecContext); AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(callbackProvider); @@ -91,34 +87,34 @@ private void stubCallback(EventType .getCallback(event); } - // ── fireFilenamesEvent: report-on-failure branch ─────────────────────────── + // ── fireFilenamesEvent: wiring to BlockResponseFunction ───────────────────── @Test - void fireFilenamesEventReportsBlockFailureWhenCommitFails() { + void fireFilenamesEventCommitFails() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenCommitSucceeds() { + void fireFilenamesEventCommitSucceeds() { stubCallback(Events.EVENTS.requestFilesFilenames()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); BlockingException result = MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { + void fireFilenamesEventNoBlockResponseFunction() { stubCallback(Events.EVENTS.requestFilesFilenames()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -126,15 +122,15 @@ void fireFilenamesEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() { MultipartHelper.fireFilenamesEvent(singletonList(part("evil.php")), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } - // ── fireFilesContentEvent: report-on-failure branch ───────────────────────── + // ── fireFilesContentEvent: wiring to BlockResponseFunction ────────────────── @Test - void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOException { + void fireFilesContentEventCommitFails() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(false); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(false); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -144,13 +140,13 @@ void fireFilesContentEventReportsBlockFailureWhenCommitFails() throws IOExceptio BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, times(1)).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws IOException { + void fireFilesContentEventCommitSucceeds() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); - when(brf.tryCommitBlockingResponse(traceSegment, RBA)).thenReturn(true); + when(brf.tryCommitBlockingResponse(reqCtx, RBA)).thenReturn(true); Part p = mock(Part.class); when(p.getSubmittedFileName()).thenReturn("photo.jpg"); @@ -160,12 +156,11 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenCommitSucceeds() throws I BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNotNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, times(1)).tryCommitBlockingResponse(reqCtx, RBA); } @Test - void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() - throws IOException { + void fireFilesContentEventNoBlockResponseFunction() throws IOException { stubCallback(Events.EVENTS.requestFilesContent()); when(reqCtx.getBlockResponseFunction()).thenReturn(null); @@ -177,7 +172,7 @@ void fireFilesContentEventDoesNotReportBlockFailureWhenNoBlockResponseFunction() BlockingException result = MultipartHelper.fireFilesContentEvent(singletonList(p), reqCtx); assertNull(result); - verify(appSecContext, never()).reportBlockFailure(); + verify(brf, never()).tryCommitBlockingResponse(any(RequestContext.class), any()); } @Test diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java index d78edcbc7e2..c66d1adca13 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.0/src/main/java/datadog/trace/instrumentation/jetty70/JettyCommitResponseInstrumentation.java @@ -12,11 +12,9 @@ import datadog.context.Context; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; import org.eclipse.jetty.http.Generator; @@ -98,15 +96,10 @@ static class CommitResponseAdvice { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = requestContext.getBlockResponseFunction(); if (brf != null) { - boolean res = brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); - if (res) { + if (brf.tryCommitBlockingResponse(requestContext, rba)) { requestContext.getTraceSegment().effectivelyBlocked(); return true; } - Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java index 33c0f32b889..475d2640cbb 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/main/java/datadog/trace/instrumentation/jetty76/JettyCommitResponseInstrumentation.java @@ -13,11 +13,9 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; import org.eclipse.jetty.http.Generator; @@ -109,14 +107,7 @@ static class CommitResponseAdvice { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = requestContext.getBlockResponseFunction(); if (brf != null) { - boolean res = brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); - if (!res) { - Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } - return res; + return brf.tryCommitBlockingResponse(requestContext, rba); } } diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java index d2fd85d5935..7b38a3ee109 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java @@ -12,7 +12,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -78,12 +77,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (multipart file upload)"); reqCtx.getTraceSegment().effectivelyBlocked(); diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java index f46dbb503cf..e4d8c618e5c 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java @@ -12,7 +12,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -114,12 +113,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java index 4a7a3c613a4..752084c1ed4 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java @@ -14,7 +14,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -75,12 +74,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java index 7033d65d8bc..b25a9471f7a 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java @@ -12,7 +12,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -114,12 +113,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java index 8eb5b40a74f..3d57c6e3a20 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java @@ -14,7 +14,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -75,12 +74,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java index 40a6988b7e7..d725cca1f16 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java @@ -15,7 +15,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -130,12 +129,7 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); } t = new BlockingException("Blocked request (for HttpMessageConverter/read)"); } @@ -164,12 +158,7 @@ public static void before( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); } throw new BlockingException("Blocked response (for HttpMessageConverter/write)"); } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java index f2e839e0263..e5c1c9e7bd9 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java @@ -13,7 +13,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.iast.IastPostProcessorFactory; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -168,12 +167,7 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java index 25ae61beebc..1d47e9496a9 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java @@ -13,7 +13,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.iast.IastPostProcessorFactory; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -128,12 +127,7 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java index 66f0a99093d..b9b4b043314 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java @@ -3,7 +3,6 @@ import static datadog.trace.api.gateway.Events.EVENTS; import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -99,12 +98,7 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java index d9be60b3af1..f9b0bf79408 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java @@ -3,7 +3,6 @@ import static datadog.trace.api.gateway.Events.EVENTS; import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -67,12 +66,7 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - if (!brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + brf.tryCommitBlockingResponse(reqCtx, rba); } t = new BlockingException( diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java index e33420eb22f..c8ec453f8ec 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java @@ -14,7 +14,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -92,14 +91,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - if (!blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { - // NOTE: UndertowBlockResponseFunction currently always returns true; this branch is - // expected to be unreachable until that is fixed separately (tech-debt follow-up). - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for FormEncodedDataParser/doParse)"); } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java index d9cfbd2afdd..8acaf891d8d 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java @@ -14,7 +14,6 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -105,16 +104,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - boolean success = - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (!success) { - // NOTE: UndertowBlockResponseFunction currently always returns true; this branch is - // expected to be unreachable until that is fixed separately (tech-debt follow-up). - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + boolean success = blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); if (success && t == null) { t = new BlockingException( @@ -142,16 +132,7 @@ static void after( (Flow.Action.RequestBlockingAction) filenamesAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null && t == null) { - boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (!success) { - // NOTE: UndertowBlockResponseFunction currently always returns true; this branch - // is expected to be unreachable until that is fixed separately (tech-debt - // follow-up). - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); if (success) { t = new BlockingException("Blocked request (multipart file upload)"); } @@ -170,16 +151,7 @@ static void after( (Flow.Action.RequestBlockingAction) contentAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null && t == null) { - boolean success = brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (!success) { - // NOTE: UndertowBlockResponseFunction currently always returns true; this branch - // is expected to be unreachable until that is fixed separately (tech-debt - // follow-up). - Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); - if (rawAppSecCtx instanceof AppSecContext) { - ((AppSecContext) rawAppSecCtx).reportBlockFailure(); - } - } + boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); if (success) { t = new BlockingException("Blocked request (multipart file upload content)"); } From 2b990818934d92c5a3e58ee5c2f6bd21278a11d5 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 17 Sep 2026 13:24:09 +0200 Subject: [PATCH 3/6] Report block failure on unguarded exception paths in Grizzly, Jetty and Undertow - Move Grizzly's post-commit span/listener calls inside the try/catch so failures there also trigger reportBlockFailure() - Add missing !success reportBlockFailure() branch to Jetty's before() in 9.0.4/9.3/9.4.21/10.0 - Wrap Undertow's tryCommitBlockingResponse calls in FormDataContentHelper to catch exceptions swallowed by the advice's suppress=Throwable.class --- .../grizzly/GrizzlyBlockingHelper.java | 10 +++---- .../jetty10/JettyCommitResponseHelper.java | 7 +++++ .../jetty904/JettyCommitResponseHelper.java | 7 +++++ .../jetty93/JettyCommitResponseHelper.java | 7 +++++ .../jetty9421/JettyCommitResponseHelper.java | 7 +++++ .../undertow/FormDataContentHelper.java | 29 +++++++++++++++++++ ...MultiPartUploadHandlerInstrumentation.java | 8 +++-- 7 files changed, 67 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java b/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java index a084cc8724f..41dc42ff5de 100644 --- a/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java +++ b/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java @@ -91,6 +91,11 @@ public static boolean block( } os.close(); response.finish(); + + if (span != null) { + span.getRequestContext().getTraceSegment().effectivelyBlocked(); + } + SpanClosingListener.LISTENER.onAfterService(request); } catch (Throwable e) { log.info("Error committing blocking response", e); if (span != null) { @@ -110,11 +115,6 @@ public static boolean block( return true; } - if (span != null) { - span.getRequestContext().getTraceSegment().effectivelyBlocked(); - } - SpanClosingListener.LISTENER.onAfterService(request); - return true; } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/main/java11/datadog/trace/instrumentation/jetty10/JettyCommitResponseHelper.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/main/java11/datadog/trace/instrumentation/jetty10/JettyCommitResponseHelper.java index be0be6eaa3a..597098a830b 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/main/java11/datadog/trace/instrumentation/jetty10/JettyCommitResponseHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/main/java11/datadog/trace/instrumentation/jetty10/JettyCommitResponseHelper.java @@ -4,8 +4,10 @@ import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_IGNORE_COMMIT_ATTRIBUTE; import datadog.context.Context; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -85,6 +87,11 @@ public class JettyCommitResponseHelper { if (success) { requestContext.getTraceSegment().effectivelyBlocked(); return true; + } else { + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/main/java/datadog/trace/instrumentation/jetty904/JettyCommitResponseHelper.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/main/java/datadog/trace/instrumentation/jetty904/JettyCommitResponseHelper.java index 964f6e7a524..72171517849 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/main/java/datadog/trace/instrumentation/jetty904/JettyCommitResponseHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/main/java/datadog/trace/instrumentation/jetty904/JettyCommitResponseHelper.java @@ -5,8 +5,10 @@ import static datadog.trace.instrumentation.jetty9.JettyDecorator.DECORATE; import datadog.context.Context; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.jetty9.ExtractAdapter; import java.lang.reflect.InvocationTargetException; @@ -84,6 +86,11 @@ public class JettyCommitResponseHelper { _committed.set(true); requestContext.getTraceSegment().effectivelyBlocked(); return true; + } else { + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/main/java/datadog/trace/instrumentation/jetty93/JettyCommitResponseHelper.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/main/java/datadog/trace/instrumentation/jetty93/JettyCommitResponseHelper.java index 9eb5a286891..2cc1f6a75bc 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/main/java/datadog/trace/instrumentation/jetty93/JettyCommitResponseHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/main/java/datadog/trace/instrumentation/jetty93/JettyCommitResponseHelper.java @@ -5,8 +5,10 @@ import static datadog.trace.instrumentation.jetty9.JettyDecorator.DECORATE; import datadog.context.Context; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.jetty9.ExtractAdapter; import java.lang.reflect.InvocationTargetException; @@ -74,6 +76,11 @@ public class JettyCommitResponseHelper { _committed.set(true); requestContext.getTraceSegment().effectivelyBlocked(); return true; + } else { + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/main/java/datadog/trace/instrumentation/jetty9421/JettyCommitResponseHelper.java b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/main/java/datadog/trace/instrumentation/jetty9421/JettyCommitResponseHelper.java index c51be7f1b20..57b65ad62be 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/main/java/datadog/trace/instrumentation/jetty9421/JettyCommitResponseHelper.java +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/main/java/datadog/trace/instrumentation/jetty9421/JettyCommitResponseHelper.java @@ -5,8 +5,10 @@ import static datadog.trace.instrumentation.jetty9.JettyDecorator.DECORATE; import datadog.context.Context; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.jetty9.ExtractAdapter; import java.lang.reflect.InvocationTargetException; @@ -75,6 +77,11 @@ public class JettyCommitResponseHelper { if (success) { requestContext.getTraceSegment().effectivelyBlocked(); return true; + } else { + Object rawAppSecCtx = requestContext.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } } } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java index 9901a9b67b9..88bae3a40ef 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataContentHelper.java @@ -1,6 +1,11 @@ package datadog.trace.instrumentation.undertow; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.http.MultipartContentDecoder; import datadog.trace.api.internal.VisibleForTesting; import io.undertow.server.handlers.form.FormData; @@ -39,6 +44,30 @@ public final class FormDataContentHelper { FILE_ITEM_GET_INPUT_STREAM = gis; } + /** + * Wraps {@link BlockResponseFunction#tryCommitBlockingResponse(RequestContext, + * Flow.Action.RequestBlockingAction)} so that an exception thrown by the commit attempt itself + * (rather than a plain {@code false} return) is still reported as a block failure. The advice + * that calls this method runs with {@code suppress = Throwable.class}, so without this guard such + * an exception would propagate out of the advice and be silently swallowed, and the + * default-method reporting inside {@code tryCommitBlockingResponse} would never run. + */ + public static boolean tryCommitBlockingResponse( + BlockResponseFunction blockResponseFunction, + RequestContext reqCtx, + Flow.Action.RequestBlockingAction rba) { + try { + return blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + } catch (Exception e) { + log.debug("Error committing blocking response", e); + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + return false; + } + } + public static List collectContents(FormData attachment) { List result = new ArrayList<>(MAX_FILES_TO_INSPECT); for (String key : attachment) { diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java index 8acaf891d8d..9018763ceca 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/MultiPartUploadHandlerInstrumentation.java @@ -67,6 +67,7 @@ public void methodAdvice(MethodTransformer transformer) { @RequiresRequestContext(RequestContextSlot.APPSEC) public static class ParseBlockingAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) static boolean onEnter(@Advice.FieldValue("exchange") HttpServerExchange exchange) { return exchange.getAttachment(FORM_DATA) == null; @@ -104,7 +105,8 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - boolean success = blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + boolean success = + FormDataContentHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); if (success && t == null) { t = new BlockingException( @@ -132,7 +134,7 @@ static void after( (Flow.Action.RequestBlockingAction) filenamesAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null && t == null) { - boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = FormDataContentHelper.tryCommitBlockingResponse(brf, reqCtx, rba); if (success) { t = new BlockingException("Blocked request (multipart file upload)"); } @@ -151,7 +153,7 @@ static void after( (Flow.Action.RequestBlockingAction) contentAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null && t == null) { - boolean success = brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = FormDataContentHelper.tryCommitBlockingResponse(brf, reqCtx, rba); if (success) { t = new BlockingException("Blocked request (multipart file upload content)"); } From f2cbed27d7dddf1296aba0db6ad06567cc0e0a30 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 17 Sep 2026 14:49:49 +0200 Subject: [PATCH 4/6] Guard remaining Undertow form-parse commit call and scope Grizzly failure reporting to commit errors - FormDataParserInstrumentation (Undertow): wrap the doParse blocking-commit call with FormDataContentHelper.tryCommitBlockingResponse so a synchronous exception is still reported as a block failure instead of being swallowed by the advice's suppress = Throwable.class. - GrizzlyBlockingHelper: split post-commit finalization (effectivelyBlocked, SpanClosingListener.onAfterService) into its own try/catch so a failure there no longer reports block_failure for a response that was already committed successfully. --- .../grizzly/GrizzlyBlockingHelper.java | 21 ++++++++++++++----- .../FormDataParserInstrumentation.java | 4 ++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java b/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java index 41dc42ff5de..a955855c5b1 100644 --- a/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java +++ b/dd-java-agent/instrumentation/grizzly/grizzly-2.0/src/main/java/datadog/trace/instrumentation/grizzly/GrizzlyBlockingHelper.java @@ -91,11 +91,6 @@ public static boolean block( } os.close(); response.finish(); - - if (span != null) { - span.getRequestContext().getTraceSegment().effectivelyBlocked(); - } - SpanClosingListener.LISTENER.onAfterService(request); } catch (Throwable e) { log.info("Error committing blocking response", e); if (span != null) { @@ -115,6 +110,22 @@ public static boolean block( return true; } + try { + if (span != null) { + span.getRequestContext().getTraceSegment().effectivelyBlocked(); + } + SpanClosingListener.LISTENER.onAfterService(request); + } catch (Throwable e) { + // the response was already committed successfully; this is a finalization error, not a + // commit failure, so it must not be reported as a block failure + log.info("Error finalizing blocked request", e); + if (span != null) { + DECORATE.onError(span, e); + DECORATE.beforeFinish(context); + span.finish(); + } + } + return true; } } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java index c8ec453f8ec..fdddd680f17 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java @@ -40,7 +40,7 @@ public String instrumentedType() { @Override public String[] helperClassNames() { - return new String[] {packageName + ".FormDataMap"}; + return new String[] {packageName + ".FormDataMap", packageName + ".FormDataContentHelper"}; } private static final Reference EXCHANGE_REFERENCE = @@ -91,7 +91,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + FormDataContentHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for FormEncodedDataParser/doParse)"); } From 6f02c3d2707ba24eeb2589cd2c77d2419b72252c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 17 Sep 2026 15:52:39 +0200 Subject: [PATCH 5/6] Fix block_failure reporting gaps in Undertow and Liberty exception paths - Check the boolean return of FormDataContentHelper.tryCommitBlockingResponse in Undertow's FormDataParserInstrumentation before throwing BlockingException, matching the existing MultiPartUploadHandlerInstrumentation pattern - Add LibertyBlockingHelper.tryCommitBlockingResponse wrapper in liberty-20.0 and liberty-23.0 to report block_failure when the commit itself throws (mirrors Undertow's FormDataContentHelper contract) - Route ParsePostDataInstrumentation, ParseParametersInstrumentation and GetPartsInstrumentation (both Liberty modules) through the new wrapper and check its return value before treating the request as blocked --- .../liberty20/GetPartsInstrumentation.java | 10 ++++--- .../liberty20/LibertyBlockingHelper.java | 26 +++++++++++++++++++ .../ParseParametersInstrumentation.java | 11 +++++--- .../ParsePostDataInstrumentation.java | 17 +++++++++--- .../liberty23/LibertyBlockingHelper.java | 26 +++++++++++++++++++ .../ParseParametersInstrumentation.java | 11 +++++--- .../ParsePostDataInstrumentation.java | 17 +++++++++--- .../FormDataParserInstrumentation.java | 5 ++-- 8 files changed, 106 insertions(+), 17 deletions(-) diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java index 7b38a3ee109..80ca0c333f3 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/GetPartsInstrumentation.java @@ -41,7 +41,11 @@ public String[] knownMatchingTypes() { @Override public String[] helperClassNames() { - return new String[] {"datadog.trace.instrumentation.liberty20.PartHelper"}; + return new String[] { + "datadog.trace.instrumentation.liberty20.PartHelper", + "datadog.trace.instrumentation.liberty20.LibertyBlockingHelper", + "datadog.trace.instrumentation.liberty20.LibertyBlockingHelper$WsByteBufferImpl", + }; } @Override @@ -77,8 +81,8 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); - if (t == null) { + boolean success = LibertyBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success && t == null) { t = new BlockingException("Blocked request (multipart file upload)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/LibertyBlockingHelper.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/LibertyBlockingHelper.java index 719ba39378b..b9c014b4828 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/LibertyBlockingHelper.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/LibertyBlockingHelper.java @@ -9,7 +9,9 @@ import com.ibm.wsspi.http.channel.HttpResponseMessage; import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.function.TriConsumer; +import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -30,6 +32,30 @@ public class LibertyBlockingHelper { private static final Logger log = LoggerFactory.getLogger(LibertyBlockingHelper.class); private static final WsByteBuffer[] EMPTY_BUFFER_ARRAY = new WsByteBuffer[0]; + /** + * Wraps {@link BlockResponseFunction#tryCommitBlockingResponse(RequestContext, + * Flow.Action.RequestBlockingAction)} so that an exception thrown by the commit attempt itself + * (rather than a plain {@code false} return) is still reported as a block failure. The advice + * that calls this method runs with {@code suppress = Throwable.class}, so without this guard such + * an exception would propagate out of the advice and be silently swallowed, and the + * default-method reporting inside {@code tryCommitBlockingResponse} would never run. + */ + public static boolean tryCommitBlockingResponse( + BlockResponseFunction blockResponseFunction, + RequestContext reqCtx, + Flow.Action.RequestBlockingAction rba) { + try { + return blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + } catch (Exception e) { + log.debug("Error committing blocking response", e); + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + return false; + } + } + public static BlockingException syncBufferEnter( HttpInboundServiceContextImpl thiz, WsByteBuffer[] buffers, AgentSpan span) { if (thiz.isMessageSent() || thiz.headersSent()) { diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java index e4d8c618e5c..428db638c21 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParseParametersInstrumentation.java @@ -54,6 +54,8 @@ public String[] helperClassNames() { packageName + ".ParameterCollector", packageName + ".ParameterCollector$ParameterCollectorNoop", packageName + ".ParameterCollector$ParameterCollectorImpl", + packageName + ".LibertyBlockingHelper", + packageName + ".LibertyBlockingHelper$WsByteBufferImpl", }; } @@ -113,9 +115,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + boolean success = + LibertyBlockingHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); + if (success) { + t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } diff --git a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java index 752084c1ed4..e52ffe75d40 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-20.0/src/main/java/datadog/trace/instrumentation/liberty20/ParsePostDataInstrumentation.java @@ -39,6 +39,14 @@ public String[] knownMatchingTypes() { }; } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".LibertyBlockingHelper", + packageName + ".LibertyBlockingHelper$WsByteBufferImpl", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -74,9 +82,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + boolean success = + LibertyBlockingHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); + if (success) { + t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/LibertyBlockingHelper.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/LibertyBlockingHelper.java index bc35ec1ef3c..637a88634ad 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/LibertyBlockingHelper.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/LibertyBlockingHelper.java @@ -9,7 +9,9 @@ import com.ibm.wsspi.http.channel.HttpResponseMessage; import datadog.appsec.api.blocking.BlockingContentType; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.function.TriConsumer; +import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -30,6 +32,30 @@ public class LibertyBlockingHelper { private static final Logger log = LoggerFactory.getLogger(LibertyBlockingHelper.class); private static final WsByteBuffer[] EMPTY_BUFFER_ARRAY = new WsByteBuffer[0]; + /** + * Wraps {@link BlockResponseFunction#tryCommitBlockingResponse(RequestContext, + * Flow.Action.RequestBlockingAction)} so that an exception thrown by the commit attempt itself + * (rather than a plain {@code false} return) is still reported as a block failure. The advice + * that calls this method runs with {@code suppress = Throwable.class}, so without this guard such + * an exception would propagate out of the advice and be silently swallowed, and the + * default-method reporting inside {@code tryCommitBlockingResponse} would never run. + */ + public static boolean tryCommitBlockingResponse( + BlockResponseFunction blockResponseFunction, + RequestContext reqCtx, + Flow.Action.RequestBlockingAction rba) { + try { + return blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + } catch (Exception e) { + log.debug("Error committing blocking response", e); + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + return false; + } + } + public static BlockingException syncBufferEnter( HttpInboundServiceContextImpl thiz, WsByteBuffer[] buffers, AgentSpan span) { if (thiz.isMessageSent() || thiz.headersSent()) { diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java index b25a9471f7a..1541b5cb56e 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParseParametersInstrumentation.java @@ -54,6 +54,8 @@ public String[] helperClassNames() { packageName + ".ParameterCollector", packageName + ".ParameterCollector$ParameterCollectorNoop", packageName + ".ParameterCollector$ParameterCollectorImpl", + packageName + ".LibertyBlockingHelper", + packageName + ".LibertyBlockingHelper$WsByteBufferImpl", }; } @@ -113,9 +115,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + boolean success = + LibertyBlockingHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); + if (success) { + t = new BlockingException("Blocked request (for SRTServletRequest/parseParameters)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } diff --git a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java index 3d57c6e3a20..df50a5b6fcf 100644 --- a/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty/liberty-23.0/src/main/java/datadog/trace/instrumentation/liberty23/ParsePostDataInstrumentation.java @@ -39,6 +39,14 @@ public String[] knownMatchingTypes() { }; } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".LibertyBlockingHelper", + packageName + ".LibertyBlockingHelper$WsByteBufferImpl", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -74,9 +82,12 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); - t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + boolean success = + LibertyBlockingHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); + if (success) { + t = new BlockingException("Blocked request (for SRTServletRequest/parsePostData)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java index fdddd680f17..14123aaf6ff 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java +++ b/dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/FormDataParserInstrumentation.java @@ -91,8 +91,9 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - FormDataContentHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); - if (t == null) { + boolean success = + FormDataContentHelper.tryCommitBlockingResponse(blockResponseFunction, reqCtx, rba); + if (success && t == null) { t = new BlockingException("Blocked request (for FormEncodedDataParser/doParse)"); } } From fb575f540c4f9e6360a6996254726124ae7d3087 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 17 Sep 2026 16:33:19 +0200 Subject: [PATCH 6/6] Fix block_failure reporting gaps in Spring-webmvc blocking response paths - Add SpringBlockingHelper (spring-webmvc-3.1 and spring-webmvc-6.0) wrapping BlockResponseFunction#tryCommitBlockingResponse to guarantee reportBlockFailure() is invoked even when the commit call throws. - Route all 6 blocking call sites (HttpMessageConverter read/write, RequestMappingInfoHandlerMapping#handleMatch, UriTemplateVariablesHandlerInterceptor#preHandle, x2 modules) through the helper and only set/throw BlockingException when the commit actually succeeds. - Fix HttpMessageConverterInstrumentationTest assertions that expected the old, unconditional-throw behavior. --- .../HttpMessageConverterInstrumentation.java | 19 ++++++++-- .../springweb/SpringBlockingHelper.java | 37 +++++++++++++++++++ ...lateAndMatrixVariablesInstrumentation.java | 13 ++++--- ...ateVariablesUrlHandlerInstrumentation.java | 17 +++++++-- ...MessageConverterInstrumentationTest.groovy | 4 +- ...lateAndMatrixVariablesInstrumentation.java | 2 +- ...ateVariablesUrlHandlerInstrumentation.java | 7 ++++ .../springweb6/HandleMatchAdvice.java | 10 +++-- .../InterceptorPreHandleAdvice.java | 10 +++-- .../springweb6/SpringBlockingHelper.java | 37 +++++++++++++++++++ 10 files changed, 132 insertions(+), 24 deletions(-) create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringBlockingHelper.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringBlockingHelper.java diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java index d725cca1f16..7c4d3a778ae 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentation.java @@ -52,6 +52,13 @@ public ElementMatcher hierarchyMatcher() { return implementsInterface(named(hierarchyMarkerType())); } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".SpringBlockingHelper", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -129,9 +136,11 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = SpringBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success) { + t = new BlockingException("Blocked request (for HttpMessageConverter/read)"); + } } - t = new BlockingException("Blocked request (for HttpMessageConverter/read)"); } } } @@ -158,9 +167,11 @@ public static void before( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = SpringBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success) { + throw new BlockingException("Blocked response (for HttpMessageConverter/write)"); + } } - throw new BlockingException("Blocked response (for HttpMessageConverter/write)"); } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringBlockingHelper.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringBlockingHelper.java new file mode 100644 index 00000000000..fb497eb37cb --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringBlockingHelper.java @@ -0,0 +1,37 @@ +package datadog.trace.instrumentation.springweb; + +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SpringBlockingHelper { + private static final Logger log = LoggerFactory.getLogger(SpringBlockingHelper.class); + + /** + * Wraps {@link BlockResponseFunction#tryCommitBlockingResponse(RequestContext, + * Flow.Action.RequestBlockingAction)} so that an exception thrown by the commit attempt itself + * (rather than a plain {@code false} return) is still reported as a block failure. The advice + * that calls this method runs with {@code suppress = Throwable.class}, so without this guard such + * an exception would propagate out of the advice and be silently swallowed, and the + * default-method reporting inside {@code tryCommitBlockingResponse} would never run. + */ + public static boolean tryCommitBlockingResponse( + BlockResponseFunction blockResponseFunction, + RequestContext reqCtx, + Flow.Action.RequestBlockingAction rba) { + try { + return blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + } catch (Exception e) { + log.debug("Error committing blocking response", e); + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + return false; + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java index e5c1c9e7bd9..d2a0d86d598 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateAndMatrixVariablesInstrumentation.java @@ -82,7 +82,7 @@ public void methodAdvice(MethodTransformer transformer) { @Override public String[] helperClassNames() { return new String[] { - packageName + ".PairList", + packageName + ".PairList", packageName + ".SpringBlockingHelper", }; } @@ -167,11 +167,14 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = + SpringBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success) { + t = + new BlockingException( + "Blocked request (for RequestMappingInfoHandlerMapping/handleMatch)"); + } } - t = - new BlockingException( - "Blocked request (for RequestMappingInfoHandlerMapping/handleMatch)"); } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java index 1d47e9496a9..ff260d5262b 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/TemplateVariablesUrlHandlerInstrumentation.java @@ -76,6 +76,13 @@ public void methodAdvice(MethodTransformer transformer) { TemplateVariablesUrlHandlerInstrumentation.class.getName() + "$InterceptorPreHandleAdvice"); } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".SpringBlockingHelper", + }; + } + @Override public Advice.PostProcessor.Factory postProcessor() { return postProcessorFactory; @@ -127,11 +134,13 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = SpringBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success) { + t = + new BlockingException( + "Blocked request (for UriTemplateVariablesHandlerInterceptor/preHandle)"); + } } - t = - new BlockingException( - "Blocked request (for UriTemplateVariablesHandlerInterceptor/preHandle)"); } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy index c0b2ce0a4de..a2adfec3d00 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/HttpMessageConverterInstrumentationTest.groovy @@ -166,7 +166,7 @@ class HttpMessageConverterInstrumentationTest extends InstrumentationSpecificati converter.read(MultiValueMap, message) then: - thrown(BlockingException) + notThrown(BlockingException) appSecContext.blockFailureReported == true cleanup: @@ -202,7 +202,7 @@ class HttpMessageConverterInstrumentationTest extends InstrumentationSpecificati converter.write('example', MediaType.TEXT_PLAIN, message) then: - thrown(BlockingException) + notThrown(BlockingException) appSecContext.blockFailureReported == true cleanup: diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateAndMatrixVariablesInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateAndMatrixVariablesInstrumentation.java index 912c9d78a4a..d4fe95e8af4 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateAndMatrixVariablesInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateAndMatrixVariablesInstrumentation.java @@ -64,7 +64,7 @@ public void methodAdvice(MethodTransformer transformer) { @Override public String[] helperClassNames() { return new String[] { - packageName + ".PairList", + packageName + ".PairList", packageName + ".SpringBlockingHelper", }; } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateVariablesUrlHandlerInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateVariablesUrlHandlerInstrumentation.java index 6d99f64cf09..ab6e996b66c 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateVariablesUrlHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/TemplateVariablesUrlHandlerInstrumentation.java @@ -59,6 +59,13 @@ public void methodAdvice(MethodTransformer transformer) { packageName + ".InterceptorPreHandleAdvice"); } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".SpringBlockingHelper", + }; + } + @Override public Advice.PostProcessor.Factory postProcessor() { return postProcessorFactory; diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java index b9b4b043314..f48995d73fc 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java @@ -98,11 +98,13 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = SpringBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success) { + t = + new BlockingException( + "Blocked request (for RequestMappingInfoHandlerMapping/handleMatch)"); + } } - t = - new BlockingException( - "Blocked request (for RequestMappingInfoHandlerMapping/handleMatch)"); } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java index f9b0bf79408..276b0136cb5 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/InterceptorPreHandleAdvice.java @@ -66,11 +66,13 @@ public static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx, rba); + boolean success = SpringBlockingHelper.tryCommitBlockingResponse(brf, reqCtx, rba); + if (success) { + t = + new BlockingException( + "Blocked request (for UriTemplateVariablesHandlerInterceptor/preHandle)"); + } } - t = - new BlockingException( - "Blocked request (for UriTemplateVariablesHandlerInterceptor/preHandle)"); } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringBlockingHelper.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringBlockingHelper.java new file mode 100644 index 00000000000..0be5861ba9a --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringBlockingHelper.java @@ -0,0 +1,37 @@ +package datadog.trace.instrumentation.springweb6; + +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SpringBlockingHelper { + private static final Logger log = LoggerFactory.getLogger(SpringBlockingHelper.class); + + /** + * Wraps {@link BlockResponseFunction#tryCommitBlockingResponse(RequestContext, + * Flow.Action.RequestBlockingAction)} so that an exception thrown by the commit attempt itself + * (rather than a plain {@code false} return) is still reported as a block failure. The advice + * that calls this method runs with {@code suppress = Throwable.class}, so without this guard such + * an exception would propagate out of the advice and be silently swallowed, and the + * default-method reporting inside {@code tryCommitBlockingResponse} would never run. + */ + public static boolean tryCommitBlockingResponse( + BlockResponseFunction blockResponseFunction, + RequestContext reqCtx, + Flow.Action.RequestBlockingAction rba) { + try { + return blockResponseFunction.tryCommitBlockingResponse(reqCtx, rba); + } catch (Exception e) { + log.debug("Error committing blocking response", e); + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + return false; + } + } +}