You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add block_failure telemetry for Tomcat blocking enforcement failures - #12493
Adds BlockFailureReporter.tryCommitAndReport(RequestContext, Flow.Action.RequestBlockingAction) in tomcat-common as a shared choke point: commits the blocking response via the registered BlockResponseFunction and, if the commit fails (returns false), reports failure through AppSecContext.reportBlockFailure() (resolved from RequestContext.getData(RequestContextSlot.APPSEC)). Exceptions thrown by the commit attempt are deliberately propagated (not converted into a false return), so the calling advice's suppress = Throwable.class aborts the whole advice and skips its blocking success-path side effects; such exception-based failures are a known telemetry gap shared with the Netty implementation.
Wires all Tomcat/GlassFish blocking-commit call sites (5.5/6.0/7.0) through this shared helper: CommitActionInstrumentation (5.5, 7.0), ParsedBodyParametersInstrumentation (5.5, 6.0), ParsePartsInstrumentation (7.0), GlassFishBlockingHelper/GlassFishMultipartInstrumentation (7.0), and TomcatServerInstrumentation (5.5 base).
Wires all three of TomcatBlockingHelper.commitBlockingResponse()'s reachable failure paths to report block_failure: GET_OUTPUT_STREAM == null, the response already committed by something other than us, and the write to the client throwing. The "already committed" / "write threw" branches keep their existing true return value.
Guards GlassFishBlockingHelper.tryBlock()'s Servlet API fallback branch so reportBlockFailure() only fires when a response was actually available to commit through (i.e. a commit was genuinely attempted), not on the brf == null / fallbackResp == null case where nothing was attempted at all.
Adds a volatile boolean blockingResponseInitiated guard to TomcatDecorator.TomcatBlockResponseFunction, mirroring the existing pattern in Netty's NettyBlockResponseFunction. Tomcat can evaluate blocking more than once per request (e.g. multipart parsing), so once a block has committed successfully, later calls for the same request short-circuit instead of re-evaluating the commit - this is what makes it safe to report block_failure on the "already committed" branch without false positives for a legitimate repeat block.
Split of TomcatBlockingHelper
Extracted tryCommitAndReport()/reportBlockFailure(RequestContext) into a new class, BlockFailureReporter (tomcat-common), that has no dependency on org.apache.catalina.* types.
TomcatBlockingHelper keeps only the catalina-typed methods (commitBlockingResponse(), start(), tryWriteWithOutputStream/Writer, the private reportBlockFailure(Request)).
Reason: helperClassNames() injects a helper's entire class bytecode, and Gradle's muzzle task validates every reference in that bytecode against each declared library version group. Before the split, instrumentations whose target library range does not guarantee org.apache.catalina.connector.Request/Response presence (e.g. some tomcat-appsec-5.5/6.0/7.0 groups) failed muzzle just from listing TomcatBlockingHelper, even though their advice only ever called the catalina-independent tryCommitAndReport/reportBlockFailure(RequestContext) methods.
RequestInstrumentation/ResponseInstrumentation (tomcat-5.5) also needed BlockFailureReporter added to helperClassNames(), since they list TomcatBlockingHelper transitively (via TomcatDecorator) and TomcatBlockingHelper's own reportBlockFailure(Request) still delegates to BlockFailureReporter.reportBlockFailure(reqCtx).
Motivation
APPSEC blocking-outcome telemetry currently only exists for Netty (#12316). This extends coverage to Tomcat so that a WAF-decided block that silently fails to actually block the request (e.g. response already committed, BlockResponseFunction throwing) is observable via the block_failure tag on appsec.waf.requests, instead of looking like a successful block.
Additional Notes
tomcat-common's new test dependencies (tomcat:catalina/tomcat-coyote/tomcat-util, javax.servlet:servlet-api:2.4, JUnit 5, Mockito) required regenerating gradle.lockfile for that module (tomcat-appsec-6.0's lockfile needs no change: its new implementation project(...) dependency adds no external dependency to that module's classpath).
Fixed a pre-existing bug in the spring-webmvc test fixtures (TestSpringBlockResponseFunction): it was unconditionally returning true/false instead of delegating to the previously registered BlockResponseFunction when RequestContextHolder is not yet populated by Spring, which clobbered Tomcat's real BlockResponseFunction in latestDepTest runs and surfaced as 10 unrelated test failures once tryCommitAndReport started being exercised on that path.
The "already committed" / "write threw" branches were initially left out of scope; extended after review feedback (discussion) once the blockingResponseInitiated guard made it safe to report on them without false positives.
What Does This Do
block_failureon theappsec.waf.requestsmetric when Tomcat's blocking-commit path fails to enforce a decided block, mirroring the Netty implementation (Add block-outcome telemetry for Netty blocking enforcement failures #12316).BlockFailureReporter.tryCommitAndReport(RequestContext, Flow.Action.RequestBlockingAction)intomcat-commonas a shared choke point: commits the blocking response via the registeredBlockResponseFunctionand, if the commit fails (returnsfalse), reports failure throughAppSecContext.reportBlockFailure()(resolved fromRequestContext.getData(RequestContextSlot.APPSEC)). Exceptions thrown by the commit attempt are deliberately propagated (not converted into afalsereturn), so the calling advice'ssuppress = Throwable.classaborts the whole advice and skips its blocking success-path side effects; such exception-based failures are a known telemetry gap shared with the Netty implementation.CommitActionInstrumentation(5.5, 7.0),ParsedBodyParametersInstrumentation(5.5, 6.0),ParsePartsInstrumentation(7.0),GlassFishBlockingHelper/GlassFishMultipartInstrumentation(7.0), andTomcatServerInstrumentation(5.5 base).TomcatBlockingHelper.commitBlockingResponse()'s reachable failure paths to reportblock_failure:GET_OUTPUT_STREAM == null, the response already committed by something other than us, and the write to the client throwing. The "already committed" / "write threw" branches keep their existingtruereturn value.GlassFishBlockingHelper.tryBlock()'s Servlet API fallback branch soreportBlockFailure()only fires when a response was actually available to commit through (i.e. a commit was genuinely attempted), not on thebrf == null/fallbackResp == nullcase where nothing was attempted at all.volatile boolean blockingResponseInitiatedguard toTomcatDecorator.TomcatBlockResponseFunction, mirroring the existing pattern in Netty'sNettyBlockResponseFunction. Tomcat can evaluate blocking more than once per request (e.g. multipart parsing), so once a block has committed successfully, later calls for the same request short-circuit instead of re-evaluating the commit - this is what makes it safe to reportblock_failureon the "already committed" branch without false positives for a legitimate repeat block.Split of
TomcatBlockingHelpertryCommitAndReport()/reportBlockFailure(RequestContext)into a new class,BlockFailureReporter(tomcat-common), that has no dependency onorg.apache.catalina.*types.TomcatBlockingHelperkeeps only the catalina-typed methods (commitBlockingResponse(),start(),tryWriteWithOutputStream/Writer, the privatereportBlockFailure(Request)).helperClassNames()injects a helper's entire class bytecode, and Gradle'smuzzletask validates every reference in that bytecode against each declared library version group. Before the split, instrumentations whose target library range does not guaranteeorg.apache.catalina.connector.Request/Responsepresence (e.g. sometomcat-appsec-5.5/6.0/7.0groups) failed muzzle just from listingTomcatBlockingHelper, even though their advice only ever called the catalina-independenttryCommitAndReport/reportBlockFailure(RequestContext)methods.RequestInstrumentation/ResponseInstrumentation(tomcat-5.5) also neededBlockFailureReporteradded tohelperClassNames(), since they listTomcatBlockingHelpertransitively (viaTomcatDecorator) andTomcatBlockingHelper's ownreportBlockFailure(Request)still delegates toBlockFailureReporter.reportBlockFailure(reqCtx).Motivation
APPSEC blocking-outcome telemetry currently only exists for Netty (#12316). This extends coverage to Tomcat so that a WAF-decided block that silently fails to actually block the request (e.g. response already committed,
BlockResponseFunctionthrowing) is observable via theblock_failuretag onappsec.waf.requests, instead of looking like a successful block.Additional Notes
tomcat-common's new test dependencies (tomcat:catalina/tomcat-coyote/tomcat-util,javax.servlet:servlet-api:2.4, JUnit 5, Mockito) required regeneratinggradle.lockfilefor that module (tomcat-appsec-6.0's lockfile needs no change: its newimplementation project(...)dependency adds no external dependency to that module's classpath).spring-webmvctest fixtures (TestSpringBlockResponseFunction): it was unconditionally returningtrue/falseinstead of delegating to the previously registeredBlockResponseFunctionwhenRequestContextHolderis not yet populated by Spring, which clobbered Tomcat's realBlockResponseFunctioninlatestDepTestruns and surfaced as 10 unrelated test failures oncetryCommitAndReportstarted being exercised on that path.blockingResponseInitiatedguard made it safe to report on them without false positives.Contributor Checklist