diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/test/SetupSpecHelper.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/test/SetupSpecHelper.groovy index 04912808d9b..639da44f3dc 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/test/SetupSpecHelper.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/test/SetupSpecHelper.groovy @@ -31,14 +31,24 @@ class SetupSpecHelper { ss.registerCallback(uriEvent, new TriFunction>() { @Override Flow apply(RequestContext requestContext, String s, URIDataAdapter uriDataAdapter) { - requestContext.setBlockResponseFunction(TestSpringBlockResponseFunction.INSTANCE) + BlockResponseFunction original = requestContext.getBlockResponseFunction() + requestContext.setBlockResponseFunction(new TestSpringBlockResponseFunction(original)) origUriCallback.apply(requestContext, s, uriDataAdapter) } }) } - enum TestSpringBlockResponseFunction implements BlockResponseFunction { - INSTANCE + /** + * Commits the blocking response through Spring's request attributes when they are available, + * falling back to the block response function the server instrumentation had already registered + * (e.g. Tomcat's) for block points that happen before Spring populates RequestContextHolder. + */ + static class TestSpringBlockResponseFunction implements BlockResponseFunction { + private final BlockResponseFunction original + + TestSpringBlockResponseFunction(BlockResponseFunction original) { + this.original = original + } @Override boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, BlockingContentType templateType, Map extraHeaders, String securityResponseId) { @@ -46,8 +56,9 @@ class SetupSpecHelper { if (attributes) { ServletBlockingHelper .commitBlockingResponse(segment, attributes.request, attributes.response, statusCode, templateType, extraHeaders, securityResponseId) + return true } - true + original != null && original.tryCommitBlockingResponse(segment, statusCode, templateType, extraHeaders, securityResponseId) } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/SetupSpecHelper.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/SetupSpecHelper.groovy index 04912808d9b..639da44f3dc 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/SetupSpecHelper.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/SetupSpecHelper.groovy @@ -31,14 +31,24 @@ class SetupSpecHelper { ss.registerCallback(uriEvent, new TriFunction>() { @Override Flow apply(RequestContext requestContext, String s, URIDataAdapter uriDataAdapter) { - requestContext.setBlockResponseFunction(TestSpringBlockResponseFunction.INSTANCE) + BlockResponseFunction original = requestContext.getBlockResponseFunction() + requestContext.setBlockResponseFunction(new TestSpringBlockResponseFunction(original)) origUriCallback.apply(requestContext, s, uriDataAdapter) } }) } - enum TestSpringBlockResponseFunction implements BlockResponseFunction { - INSTANCE + /** + * Commits the blocking response through Spring's request attributes when they are available, + * falling back to the block response function the server instrumentation had already registered + * (e.g. Tomcat's) for block points that happen before Spring populates RequestContextHolder. + */ + static class TestSpringBlockResponseFunction implements BlockResponseFunction { + private final BlockResponseFunction original + + TestSpringBlockResponseFunction(BlockResponseFunction original) { + this.original = original + } @Override boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, BlockingContentType templateType, Map extraHeaders, String securityResponseId) { @@ -46,8 +56,9 @@ class SetupSpecHelper { if (attributes) { ServletBlockingHelper .commitBlockingResponse(segment, attributes.request, attributes.response, statusCode, templateType, extraHeaders, securityResponseId) + return true } - true + original != null && original.tryCommitBlockingResponse(segment, statusCode, templateType, extraHeaders, securityResponseId) } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SetupSpecHelper.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SetupSpecHelper.groovy index e409483dc6b..34f006b185e 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SetupSpecHelper.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SetupSpecHelper.groovy @@ -31,14 +31,24 @@ class SetupSpecHelper { ss.registerCallback(uriEvent, new TriFunction>() { @Override Flow apply(RequestContext requestContext, String s, URIDataAdapter uriDataAdapter) { - requestContext.setBlockResponseFunction(TestSpringBlockResponseFunction.INSTANCE) + BlockResponseFunction original = requestContext.getBlockResponseFunction() + requestContext.setBlockResponseFunction(new TestSpringBlockResponseFunction(original)) origUriCallback.apply(requestContext, s, uriDataAdapter) } }) } - enum TestSpringBlockResponseFunction implements BlockResponseFunction { - INSTANCE + /** + * Commits the blocking response through Spring's request attributes when they are available, + * falling back to the block response function the server instrumentation had already registered + * (e.g. Tomcat's) for block points that happen before Spring populates RequestContextHolder. + */ + static class TestSpringBlockResponseFunction implements BlockResponseFunction { + private final BlockResponseFunction original + + TestSpringBlockResponseFunction(BlockResponseFunction original) { + this.original = original + } @Override boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, BlockingContentType templateType, Map extraHeaders, String securityResponseId) { @@ -46,8 +56,9 @@ class SetupSpecHelper { if (attributes) { JakartaServletBlockingHelper .commitBlockingResponse(segment, attributes.request, attributes.response, statusCode, templateType, extraHeaders, securityResponseId) + return true } - true + original != null && original.tryCommitBlockingResponse(segment, statusCode, templateType, extraHeaders, securityResponseId) } } } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/RequestInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/RequestInstrumentation.java index 9265724a132..781965a1207 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/RequestInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/RequestInstrumentation.java @@ -54,6 +54,7 @@ public String[] helperClassNames() { packageName + ".TomcatDecorator", packageName + ".TomcatDecorator$TomcatBlockResponseFunction", packageName + ".TomcatBlockingHelper", + packageName + ".BlockFailureReporter", packageName + ".RequestURIDataAdapter", }; } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/ResponseInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/ResponseInstrumentation.java index d9acec40e32..df1f7f5d5d1 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/ResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/ResponseInstrumentation.java @@ -38,6 +38,7 @@ public String[] helperClassNames() { packageName + ".TomcatDecorator", packageName + ".TomcatDecorator$TomcatBlockResponseFunction", packageName + ".TomcatBlockingHelper", + packageName + ".BlockFailureReporter", packageName + ".RequestURIDataAdapter", }; } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java index 14fb92e8fae..4235863358b 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java @@ -56,6 +56,7 @@ public String[] helperClassNames() { packageName + ".TomcatDecorator$TomcatBlockResponseFunction", packageName + ".RequestURIDataAdapter", packageName + ".TomcatBlockingHelper", + packageName + ".BlockFailureReporter", }; } @@ -186,9 +187,7 @@ public static class PostParseAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void afterParse( - @Advice.Argument(1) Request req, - @Advice.Argument(3) Response resp, - @Advice.Return(readOnly = false) Boolean ret) { + @Advice.Argument(1) Request req, @Advice.Return(readOnly = false) Boolean ret) { Object contextObj = req.getAttribute(DD_CONTEXT_ATTRIBUTE); if (contextObj instanceof Context) { Context context = (Context) contextObj; @@ -202,8 +201,7 @@ public static void afterParse( DECORATE.onRequest(span, req, req, parentContext); Flow.Action.RequestBlockingAction rba = span.getRequestBlockingAction(); if (rba != null) { - TomcatBlockingHelper.commitBlockingResponse( - span.getRequestContext().getTraceSegment(), req, resp, rba); + BlockFailureReporter.tryCommitAndReport(span.getRequestContext(), rba); ret = false; // skip pipeline } } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/CommitActionInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/CommitActionInstrumentation.java index b5e74a8751b..983b40b924f 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/CommitActionInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/CommitActionInstrumentation.java @@ -14,6 +14,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; +import datadog.trace.instrumentation.tomcat.BlockFailureReporter; import datadog.trace.instrumentation.tomcat.ExtractAdapter; import datadog.trace.instrumentation.tomcat.TomcatDecorator; import net.bytebuddy.asm.Advice; @@ -75,6 +76,7 @@ public String[] helperClassNames() { pkg + ".TomcatDecorator", pkg + ".TomcatDecorator$TomcatBlockResponseFunction", pkg + ".TomcatBlockingHelper", + pkg + ".BlockFailureReporter", pkg + ".RequestURIDataAdapter", }; } @@ -114,7 +116,7 @@ static class ProcessCommitActionAdvice { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = requestContext.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + BlockFailureReporter.tryCommitAndReport(requestContext, rba); thiz.action(ActionCode.ACTION_CLOSE, null); return true; } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/ParsedBodyParametersInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/ParsedBodyParametersInstrumentation.java index be98fe79c12..13dd6962148 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/ParsedBodyParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-5.5/src/main/java/datadog/trace/instrumentation/tomcat55/ParsedBodyParametersInstrumentation.java @@ -20,6 +20,7 @@ import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.instrumentation.tomcat.BlockFailureReporter; import java.util.Hashtable; import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; @@ -65,6 +66,11 @@ public Reference[] additionalMuzzleReferences() { return new Reference[] {PARAM_HASH_STRING_ARRAY_REFERENCE}; } + @Override + public String[] helperClassNames() { + return new String[] {"datadog.trace.instrumentation.tomcat.BlockFailureReporter"}; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -143,7 +149,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + BlockFailureReporter.tryCommitAndReport(reqCtx, rba); if (t == null) { t = new BlockingException("Blocked request (for processParameters)"); } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/build.gradle index 67cb1a7ea40..fa2240c2dcf 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/build.gradle @@ -25,6 +25,7 @@ muzzle { dependencies { compileOnly group: 'org.apache.tomcat', name: 'coyote', version: '6.0.53' + implementation project(':dd-java-agent:instrumentation:tomcat:tomcat-common') } // testing happens in tomcat-5.5 module diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/src/main/java/datadog/trace/instrumentation/tomcat6/ParsedBodyParametersInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/src/main/java/datadog/trace/instrumentation/tomcat6/ParsedBodyParametersInstrumentation.java index 6a6aebeec7b..7d536179f64 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/src/main/java/datadog/trace/instrumentation/tomcat6/ParsedBodyParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/src/main/java/datadog/trace/instrumentation/tomcat6/ParsedBodyParametersInstrumentation.java @@ -12,13 +12,13 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.instrumentation.tomcat.BlockFailureReporter; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; @@ -53,6 +53,11 @@ public Reference[] additionalMuzzleReferences() { return new Reference[] {PARAM_HASH_VALUES_MAP_REFERENCE}; } + @Override + public String[] helperClassNames() { + return new String[] {"datadog.trace.instrumentation.tomcat.BlockFailureReporter"}; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -129,15 +134,8 @@ static void after( Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - boolean committedBlockingResponse = - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (committedBlockingResponse) { - if (t == null) { - t = new BlockingException("Blocked request (for processParameters)"); - } - } + if (BlockFailureReporter.tryCommitAndReport(reqCtx, rba) && t == null) { + t = new BlockingException("Blocked request (for processParameters)"); } } } finally { diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/CommitActionInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/CommitActionInstrumentation.java index 330c40a68d2..f36a55dff07 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/CommitActionInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/CommitActionInstrumentation.java @@ -15,6 +15,7 @@ import datadog.trace.api.gateway.RequestContext; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; +import datadog.trace.instrumentation.tomcat.BlockFailureReporter; import datadog.trace.instrumentation.tomcat.ExtractAdapter; import datadog.trace.instrumentation.tomcat.TomcatDecorator; import net.bytebuddy.asm.Advice; @@ -76,6 +77,7 @@ public String[] helperClassNames() { pkg + ".TomcatDecorator", pkg + ".TomcatDecorator$TomcatBlockResponseFunction", pkg + ".TomcatBlockingHelper", + pkg + ".BlockFailureReporter", pkg + ".RequestURIDataAdapter", }; } @@ -121,7 +123,7 @@ static class ProcessCommitActionAdvice { Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = requestContext.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + BlockFailureReporter.tryCommitAndReport(requestContext, rba); thiz.action(ActionCode.CLOSE, null); return true; } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java index 78a06148a71..f8acb4857ab 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java @@ -7,6 +7,7 @@ import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.http.MultipartContentDecoder; import datadog.trace.bootstrap.blocking.BlockingActionHelper; +import datadog.trace.instrumentation.tomcat.BlockFailureReporter; import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; @@ -16,9 +17,13 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.Part; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class GlassFishBlockingHelper { + private static final Logger log = LoggerFactory.getLogger(GlassFishBlockingHelper.class); + public static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount(); public static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); @@ -38,12 +43,21 @@ public static boolean tryBlock( try { BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + // tryCommitAndReport already reports the block failure when the commit fails + if (!BlockFailureReporter.tryCommitAndReport(reqCtx, rba)) { + return false; + } } else if (!commitBlocking(fallbackReq, fallbackResp, rba)) { + if (fallbackResp != null) { + // a commit was genuinely attempted (there was a response to write to) and failed + BlockFailureReporter.reportBlockFailure(reqCtx); + } return false; } - } catch (Exception ignored) { - // commit failed — response not sent, cannot block this request + } catch (Exception e) { + // commit failed - response not sent, cannot block this request + log.debug("Error committing blocking response", e); + BlockFailureReporter.reportBlockFailure(reqCtx); return false; } // Response was committed — mark as blocked on a best-effort basis. diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java index 74dde6d6b38..27d3d4376cc 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java @@ -50,6 +50,8 @@ public String instrumentedType() { @Override public String[] helperClassNames() { return new String[] { + // referenced by GlassFishBlockingHelper.tryBlock() to commit the blocking response + "datadog.trace.instrumentation.tomcat.BlockFailureReporter", "datadog.trace.instrumentation.tomcat7.GlassFishBlockingHelper", }; } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java index 1799a9cbaa9..f884816cafb 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java @@ -17,6 +17,7 @@ import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.instrumentation.tomcat.BlockFailureReporter; import java.util.List; import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; @@ -56,6 +57,8 @@ public String instrumentedType() { @Override public String[] helperClassNames() { return new String[] { + // referenced by the inlined advice below to commit the blocking response + "datadog.trace.instrumentation.tomcat.BlockFailureReporter", "datadog.trace.instrumentation.tomcat7.ParameterCollector", "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorNoop", "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorImpl", @@ -122,7 +125,7 @@ static void after( Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + BlockFailureReporter.tryCommitAndReport(reqCtx, rba); t = new BlockingException("Blocked request (for Request/parseParts)"); reqCtx.getTraceSegment().effectivelyBlocked(); } @@ -142,7 +145,7 @@ static void after( (Flow.Action.RequestBlockingAction) filenamesAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + BlockFailureReporter.tryCommitAndReport(reqCtx, rba); t = new BlockingException("Blocked request (multipart file upload)"); reqCtx.getTraceSegment().effectivelyBlocked(); } @@ -163,7 +166,7 @@ static void after( (Flow.Action.RequestBlockingAction) contentAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + BlockFailureReporter.tryCommitAndReport(reqCtx, rba); t = new BlockingException("Blocked request (multipart file upload content)"); reqCtx.getTraceSegment().effectivelyBlocked(); } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelperTest.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelperTest.java index e4c4a953c17..a231ec9e111 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelperTest.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelperTest.java @@ -5,6 +5,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -12,9 +13,11 @@ import static org.mockito.Mockito.when; 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.api.internal.TraceSegment; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -120,7 +123,7 @@ void commitBlocking_ioException_returnsFalse() throws IOException { @Test void tryBlock_withBrf_commitsViaFunctionAndReturnsTrue() throws Exception { TraceSegment segment = mock(TraceSegment.class); - BlockResponseFunction brf = mock(BlockResponseFunction.class); + BlockResponseFunction brf = mockCommittingBrf(); RequestContext reqCtx = mockReqCtx(brf, segment); Flow.Action.RequestBlockingAction action = rba(403); @@ -168,13 +171,96 @@ void tryBlock_brfThrows_returnsFalse() throws Exception { @Test void tryBlock_effectivelyBlockedThrows_stillReturnsTrue() throws Exception { TraceSegment segment = mock(TraceSegment.class); - BlockResponseFunction brf = mock(BlockResponseFunction.class); + BlockResponseFunction brf = mockCommittingBrf(); RequestContext reqCtx = mockReqCtx(brf, segment); doThrow(new RuntimeException("span already finished")).when(segment).effectivelyBlocked(); assertTrue(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); } + // ------- tryBlock() block_failure telemetry ------- + + @Test + void tryBlock_brfCommitFails_reportsBlockFailure() { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); // tryCommit... returns false + RequestContext reqCtx = mockReqCtx(brf, segment); + AppSecContext appSecCtx = mockAppSecCtx(reqCtx); + + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + + verify(appSecCtx).reportBlockFailure(); + } + + @Test + void tryBlock_brfCommitFails_doesNotMarkEffectivelyBlocked() { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); // tryCommit... returns false + RequestContext reqCtx = mockReqCtx(brf, segment); + mockAppSecCtx(reqCtx); + + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + + verify(segment, never()).effectivelyBlocked(); + } + + @Test + void tryBlock_noBrf_fallbackCommitFails_reportsBlockFailure() { + TraceSegment segment = mock(TraceSegment.class); + RequestContext reqCtx = mockReqCtx(null, segment); + AppSecContext appSecCtx = mockAppSecCtx(reqCtx); + // an already committed response makes commitBlocking() return false + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(true); + + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, resp, rba(403))); + + verify(appSecCtx).reportBlockFailure(); + verify(segment, never()).effectivelyBlocked(); + } + + @Test + void tryBlock_noBrf_nullFallbackResponse_doesNotReportBlockFailure() { + TraceSegment segment = mock(TraceSegment.class); + RequestContext reqCtx = mockReqCtx(null, segment); + AppSecContext appSecCtx = mockAppSecCtx(reqCtx); + + // brf == null and fallbackResp == null: no response was ever available to commit through, + // so nothing was genuinely attempted and no block_failure should be reported. + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + + verify(appSecCtx, never()).reportBlockFailure(); + } + + @Test + void tryBlock_commitThrows_reportsBlockFailure() throws Exception { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); + RequestContext reqCtx = mockReqCtx(brf, segment); + AppSecContext appSecCtx = mockAppSecCtx(reqCtx); + doThrow(new RuntimeException("commit failed")) + .when(brf) + .tryCommitBlockingResponse(any(), any(Flow.Action.RequestBlockingAction.class)); + + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + + verify(appSecCtx).reportBlockFailure(); + verify(segment, never()).effectivelyBlocked(); + } + + @Test + void tryBlock_success_doesNotReportBlockFailure() { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mockCommittingBrf(); + RequestContext reqCtx = mockReqCtx(brf, segment); + AppSecContext appSecCtx = mockAppSecCtx(reqCtx); + + assertTrue(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + + verify(appSecCtx, never()).reportBlockFailure(); + verify(segment).effectivelyBlocked(); + } + // ------- processPartsAndBlock() ------- @Test @@ -271,7 +357,7 @@ void processPartsAndBlock_getInputStreamThrows_emptyStringFallback() throws Exce void processPartsAndBlock_filenamesCbBlocks_contentCbNotFired() throws Exception { Part filePart = mockPart("evil.exe", "application/octet-stream", "content".getBytes()); TraceSegment segment = mock(TraceSegment.class); - BlockResponseFunction brf = mock(BlockResponseFunction.class); + BlockResponseFunction brf = mockCommittingBrf(); RequestContext reqCtx = mockReqCtx(brf, segment); BiFunction, Flow> filenamesCb = mockBlockingCb(403); BiFunction, Flow> contentCb = mockPassThroughCb(); @@ -287,7 +373,7 @@ void processPartsAndBlock_filenamesCbBlocks_contentCbNotFired() throws Exception void processPartsAndBlock_contentCbBlocks_returnsTrue() throws Exception { Part filePart = mockPart("upload.bin", "application/octet-stream", "payload".getBytes()); TraceSegment segment = mock(TraceSegment.class); - BlockResponseFunction brf = mock(BlockResponseFunction.class); + BlockResponseFunction brf = mockCommittingBrf(); RequestContext reqCtx = mockReqCtx(brf, segment); BiFunction, Flow> filenamesCb = mockPassThroughCb(); BiFunction, Flow> contentCb = mockBlockingCb(403); @@ -322,6 +408,21 @@ private static RequestContext mockReqCtx(BlockResponseFunction brf, TraceSegment return reqCtx; } + /** A {@link BlockResponseFunction} whose commit attempt succeeds. */ + private static BlockResponseFunction mockCommittingBrf() { + BlockResponseFunction brf = mock(BlockResponseFunction.class); + when(brf.tryCommitBlockingResponse(any(), any(Flow.Action.RequestBlockingAction.class))) + .thenReturn(true); + return brf; + } + + /** Binds an {@link AppSecContext} mock to the APPSEC slot of the given request context. */ + private static AppSecContext mockAppSecCtx(RequestContext reqCtx) { + AppSecContext appSecCtx = mock(AppSecContext.class); + doReturn(appSecCtx).when(reqCtx).getData(RequestContextSlot.APPSEC); + return appSecCtx; + } + private static Part mockPart(String submittedFilename, String contentType, byte[] content) throws Exception { Part part = mock(Part.class); diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-common/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-common/build.gradle index e6f844fb59f..100d8738bdb 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-common/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-common/build.gradle @@ -18,4 +18,11 @@ dependencies { // Version that corresponds with Tomcat 5.5 // https://tomcat.apache.org/whichversion.html compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.4' + + testImplementation group: 'tomcat', name: 'catalina', version: tomcatVersion + testImplementation group: 'tomcat', name: 'tomcat-coyote', version: tomcatVersion + testImplementation group: 'tomcat', name: 'tomcat-util', version: tomcatVersion + testImplementation group: 'javax.servlet', name: 'servlet-api', version: '2.4' + testImplementation libs.bundles.junit5 + testImplementation libs.bundles.mockito } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-common/gradle.lockfile b/dd-java-agent/instrumentation/tomcat/tomcat-common/gradle.lockfile index c5a0975ed81..65ca4b24631 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-common/gradle.lockfile +++ b/dd-java-agent/instrumentation/tomcat/tomcat-common/gradle.lockfile @@ -53,7 +53,7 @@ de.thetaphi:forbiddenapis:3.10=compileClasspath,testCompileClasspath,testRuntime io.leangen.geantyref:geantyref:1.3.16=testRuntimeClasspath io.sqreen:libsqreen:17.5.0=testRuntimeClasspath javax.servlet:javax.servlet-api:3.1.0=testCompileClasspath,testRuntimeClasspath -javax.servlet:servlet-api:2.4=compileClasspath +javax.servlet:servlet-api:2.4=compileClasspath,testCompileClasspath,testRuntimeClasspath jaxen:jaxen:2.0.6=spotbugs junit:junit:4.13.2=testRuntimeClasspath net.bytebuddy:byte-buddy-agent:1.18.10=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath @@ -99,7 +99,8 @@ org.junit.platform:junit-platform-suite-api:1.14.1=testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=testRuntimeClasspath org.junit:junit-bom:5.14.1=testCompileClasspath,testRuntimeClasspath org.junit:junit-bom:6.1.2=spotbugs -org.mockito:mockito-core:4.4.0=testRuntimeClasspath +org.mockito:mockito-core:4.4.0=testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs @@ -123,7 +124,7 @@ org.spockframework:spock-core:2.4-groovy-3.0=testCompileClasspath,testRuntimeCla org.tabletest:tabletest-junit:1.2.2=testCompileClasspath,testRuntimeClasspath org.tabletest:tabletest-parser:1.2.1=testCompileClasspath,testRuntimeClasspath org.xmlresolver:xmlresolver:5.3.3=spotbugs -tomcat:catalina:5.5.12=compileClasspath -tomcat:tomcat-coyote:5.5.12=compileClasspath -tomcat:tomcat-util:5.5.12=compileClasspath +tomcat:catalina:5.5.12=compileClasspath,testCompileClasspath,testRuntimeClasspath +tomcat:tomcat-coyote:5.5.12=compileClasspath,testCompileClasspath,testRuntimeClasspath +tomcat:tomcat-util:5.5.12=compileClasspath,testCompileClasspath,testRuntimeClasspath empty=spotbugsPlugins diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/BlockFailureReporter.java b/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/BlockFailureReporter.java new file mode 100644 index 00000000000..8c9538feaf8 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/BlockFailureReporter.java @@ -0,0 +1,65 @@ +package datadog.trace.instrumentation.tomcat; + +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; + +/** + * Catalina-independent counterpart of {@link TomcatBlockingHelper}: commits a blocking response + * through the {@link BlockResponseFunction} registered on a request context and reports {@code + * block_failure} telemetry when the commit fails. + * + *

Kept separate from {@link TomcatBlockingHelper} (which also carries the Servlet/Catalina + * fallback commit path) so that instrumentations whose target library version range does not + * guarantee {@code org.apache.catalina.connector.Request}/{@code Response} presence can list this + * class in {@code helperClassNames()} without pulling in {@link TomcatBlockingHelper}'s + * catalina-typed methods and static initializer, which muzzle would otherwise validate against + * every version in that range. + */ +public class BlockFailureReporter { + + /** + * Commits a blocking response through the {@link BlockResponseFunction} registered on the given + * request context and, if the commit fails, reports the failure via {@link + * AppSecContext#reportBlockFailure()}. + * + *

This is the single choke point shared by all Tomcat blocking call sites so the {@code + * block_failure} telemetry is not duplicated inline. + * + *

Exceptions thrown by the commit attempt are deliberately propagated instead of being + * converted into a {@code false} return: the calling advice methods declare {@code suppress = + * Throwable.class} and rely on the whole advice being aborted so that their blocking success-path + * side effects (closing the connection, injecting a {@link + * datadog.appsec.api.blocking.BlockingException}, marking the trace segment effectively blocked) + * are skipped when no response was committed. Such exception-based commit failures are therefore + * not reported to the {@code block_failure} telemetry, which is the same known gap as the Netty + * implementation this mirrors. + * + * @return {@code true} if the blocking response was committed, {@code false} otherwise (including + * when no {@link BlockResponseFunction} is registered, in which case nothing was attempted + * and no failure is reported). + */ + public static boolean tryCommitAndReport( + RequestContext reqCtx, Flow.Action.RequestBlockingAction rba) { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf == null) { + // nothing was attempted, so this is not a block failure + return false; + } + if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) { + return true; + } + reportBlockFailure(reqCtx); + return false; + } + + /** Reports a block failure on the AppSec context bound to the given request context, if any. */ + public static void reportBlockFailure(RequestContext reqCtx) { + Object rawAppSecCtx = reqCtx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } +} diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelper.java b/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelper.java index aab5ea6f0b0..86fca266e5c 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelper.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelper.java @@ -1,10 +1,13 @@ package datadog.trace.instrumentation.tomcat; import datadog.appsec.api.blocking.BlockingContentType; +import datadog.context.Context; import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.internal.TraceSegment; import datadog.trace.bootstrap.blocking.BlockingActionHelper; import datadog.trace.bootstrap.blocking.BlockingActionHelper.TemplateType; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; import java.io.IOException; import java.io.OutputStream; @@ -34,6 +37,25 @@ public class TomcatBlockingHelper { GET_OUTPUT_STREAM = mh; } + /** + * Reports a block failure on the AppSec context bound to the given Tomcat request, if the request + * still carries a datadog context with an active span. + */ + private static void reportBlockFailure(Request request) { + Object contextObj = request.getAttribute(HttpServerDecorator.DD_CONTEXT_ATTRIBUTE); + if (!(contextObj instanceof Context)) { + return; + } + AgentSpan span = AgentSpan.fromContext((Context) contextObj); + if (span == null) { + return; + } + RequestContext reqCtx = span.getRequestContext(); + if (reqCtx != null) { + BlockFailureReporter.reportBlockFailure(reqCtx); + } + } + public static void commitBlockingResponse( TraceSegment segment, Request request, Response resp, Flow.Action.RequestBlockingAction rba) { commitBlockingResponse( @@ -55,6 +77,8 @@ public static boolean commitBlockingResponse( Map extraHeaders, String securityResponseId) { if (GET_OUTPUT_STREAM == null) { + // a commit was genuinely attempted, but the JVM-wide reflection lookup failed at class init + reportBlockFailure(request); return false; } int httpCode = BlockingActionHelper.getHttpCode(statusCode); diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-common/src/test/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelperTest.java b/dd-java-agent/instrumentation/tomcat/tomcat-common/src/test/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelperTest.java new file mode 100644 index 00000000000..66ff3d6cf38 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat/tomcat-common/src/test/java/datadog/trace/instrumentation/tomcat/TomcatBlockingHelperTest.java @@ -0,0 +1,181 @@ +package datadog.trace.instrumentation.tomcat; + +import static java.util.Collections.emptyMap; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.context.Context; +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; +import java.lang.invoke.MethodHandle; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import org.apache.catalina.connector.Request; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * Tests the {@code GET_OUTPUT_STREAM == null} degraded path of {@link TomcatBlockingHelper}, i.e. + * the case where the JVM-wide reflective lookup of {@code Response#getOutputStream()} failed at + * class initialization: a blocking commit was genuinely attempted, so a {@code block_failure} must + * be reported on the AppSec context reachable from the Tomcat request. + * + *

On a real classpath {@code Response#getOutputStream()} always exists, so the field is never + * {@code null} and the branch is unreachable by normal means. There is no production test hook (and + * adding one is out of scope), so the tests temporarily overwrite the {@code private static final} + * field and restore it afterwards. {@code Field#set} refuses static finals even after {@code + * setAccessible(true)}, and {@code Lookup#unreflectVarHandle} refuses write access to them as well, + * so the write goes through {@code sun.misc.Unsafe}, accessed purely reflectively (the same style + * already used in {@code dd-smoke-tests/crashtracking}). + */ +class TomcatBlockingHelperTest { + + private static final Field FIELD; + private static final MethodHandle ORIGINAL_VALUE; + private static final Object UNSAFE; + private static final Method PUT_OBJECT; + private static final Object STATIC_BASE; + private static final long STATIC_OFFSET; + + static { + try { + FIELD = TomcatBlockingHelper.class.getDeclaredField("GET_OUTPUT_STREAM"); + FIELD.setAccessible(true); + // also forces class initialization, so the static initializer cannot overwrite our writes + ORIGINAL_VALUE = (MethodHandle) FIELD.get(null); + + Class unsafeClass = Class.forName("sun.misc.Unsafe"); + Field theUnsafe = unsafeClass.getDeclaredField("theUnsafe"); + theUnsafe.setAccessible(true); + UNSAFE = theUnsafe.get(null); + STATIC_BASE = unsafeClass.getMethod("staticFieldBase", Field.class).invoke(UNSAFE, FIELD); + STATIC_OFFSET = + (Long) unsafeClass.getMethod("staticFieldOffset", Field.class).invoke(UNSAFE, FIELD); + PUT_OBJECT = unsafeClass.getMethod("putObject", Object.class, long.class, Object.class); + } catch (Exception e) { + throw new ExceptionInInitializerError(e); + } + } + + private static void setLookup(MethodHandle value) { + try { + PUT_OBJECT.invoke(UNSAFE, STATIC_BASE, STATIC_OFFSET, value); + } catch (Exception e) { + throw new IllegalStateException("Unable to overwrite GET_OUTPUT_STREAM", e); + } + } + + @AfterEach + void restoreLookup() { + setLookup(ORIGINAL_VALUE); + } + + @Test + void sanityCheck_lookupSucceedsOnARealClasspath() { + assertNotNull(ORIGINAL_VALUE); + } + + @Test + void commitBlockingResponse_lookupFailed_reportsBlockFailureAndReturnsFalse() { + setLookup(null); + + AppSecContext appSecCtx = mock(AppSecContext.class); + RequestContext reqCtx = mock(RequestContext.class); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(appSecCtx); + Request request = mockRequestWithSpan(mockSpan(reqCtx)); + TraceSegment segment = mock(TraceSegment.class); + + assertFalse(commitBlockingResponse(segment, request)); + + verify(appSecCtx).reportBlockFailure(); + verifyNoInteractions(segment); + } + + @Test + void commitBlockingResponse_lookupFailed_noDatadogContextOnRequest_doesNotReport() { + setLookup(null); + + Request request = mock(Request.class); + when(request.getAttribute(HttpServerDecorator.DD_CONTEXT_ATTRIBUTE)).thenReturn(null); + TraceSegment segment = mock(TraceSegment.class); + + assertFalse(commitBlockingResponse(segment, request)); + + verifyNoInteractions(segment); + } + + @Test + void commitBlockingResponse_lookupFailed_noSpanInContext_doesNotReport() { + setLookup(null); + + Request request = mockRequestWithSpan(null); + + assertFalse(commitBlockingResponse(mock(TraceSegment.class), request)); + } + + @Test + void commitBlockingResponse_lookupFailed_noRequestContextOnSpan_doesNotReport() { + setLookup(null); + + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(null); + + assertFalse(commitBlockingResponse(mock(TraceSegment.class), mockRequestWithSpan(span))); + } + + @Test + void commitBlockingResponse_lookupFailed_appsecDataIsNotAnAppSecContext_doesNotReport() { + setLookup(null); + + RequestContext reqCtx = mock(RequestContext.class); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn("not an AppSecContext"); + Request request = mockRequestWithSpan(mockSpan(reqCtx)); + + assertFalse(commitBlockingResponse(mock(TraceSegment.class), request)); + + verify(reqCtx).getData(RequestContextSlot.APPSEC); + } + + @Test + void commitBlockingResponse_lookupFailed_doesNotTouchTheResponse() { + setLookup(null); + + Request request = mock(Request.class); + + assertFalse(commitBlockingResponse(mock(TraceSegment.class), request)); + + // the early return happens before any status/header/attribute mutation + verify(request, never()).setAttribute(any(), any()); + } + + private static boolean commitBlockingResponse(TraceSegment segment, Request request) { + // the response is never touched on this path, hence null + return TomcatBlockingHelper.commitBlockingResponse( + segment, request, null, 403, BlockingContentType.AUTO, emptyMap(), null); + } + + private static AgentSpan mockSpan(RequestContext reqCtx) { + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + return span; + } + + private static Request mockRequestWithSpan(AgentSpan span) { + Context context = mock(Context.class); + when(context.get(any())).thenReturn(span); + Request request = mock(Request.class); + when(request.getAttribute(HttpServerDecorator.DD_CONTEXT_ATTRIBUTE)).thenReturn(context); + return request; + } +}