From ad1dd81aac6c906af0835b35e6fda9c9f9551ab8 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 13:47:37 +0200 Subject: [PATCH 01/13] feat(appsec/tomcat): add GlassFish/Payara multipart filename instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GlassFish 5 / Payara 5 does not have Request.parseParts() — instead Request.getParts() delegates entirely to org.apache.catalina.fileupload.Multipart.getParts(). That class uses its own ArrayList field (INVOKEVIRTUAL, not INVOKEINTERFACE) and calls a private initParts() instead of the Tomcat parseParts() that ParsePartsInstrumentation looks for. As a result, ParsePartsInstrumentation is a complete no-op on Payara: the method matcher finds nothing, and the bytecode visitor intercepts no Collection.add() calls. File names never reach the WAF even though the request is parsed without error. GlassFishMultipartInstrumentation targets org.apache.catalina.fileupload.Multipart.getParts() — a public method that exists only in GlassFish/Payara's web-core.jar and is therefore automatically skipped by ByteBuddy on standard Tomcat. After the method returns it iterates the Collection result and uses the existing ParameterCollector reflection helpers (getSubmittedFileName()) to extract file names, then fires requestFilesFilenames (and optionally requestFilesContent) callbacks exactly as ParsePartsInstrumentation does. --- .../GlassFishMultipartInstrumentation.java | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java 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 new file mode 100644 index 00000000000..cbd89ba316a --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java @@ -0,0 +1,137 @@ +package datadog.trace.instrumentation.tomcat7; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.api.gateway.Events.EVENTS; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.util.Collection; +import java.util.List; +import java.util.function.BiFunction; +import net.bytebuddy.asm.Advice; + +/** + * GlassFish/Payara does not have {@code Request.parseParts()} — instead {@code Request.getParts()} + * delegates to {@code org.apache.catalina.fileupload.Multipart.getParts()}. This instrumentation + * hooks that GlassFish-specific class to report uploaded file names to the AppSec WAF via the + * {@code requestFilesFilenames} IG event. + * + *

Because {@code org.apache.catalina.fileupload.Multipart} does not exist in standard Tomcat, + * this instrumentation is automatically skipped by ByteBuddy on non-GlassFish containers. + */ +@AutoService(InstrumenterModule.class) +public class GlassFishMultipartInstrumentation extends InstrumenterModule.AppSec + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public GlassFishMultipartInstrumentation() { + super("tomcat"); + } + + @Override + public String instrumentedType() { + return "org.apache.catalina.fileupload.Multipart"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.tomcat7.ParameterCollector", + "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorNoop", + "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorImpl", + "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorImpl$CachedMethods", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("getParts").and(takesArguments(0)).and(isPublic()), + getClass().getName() + "$GetPartsAdvice"); + } + + public static class GetPartsAdvice { + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Return Collection parts, @Advice.Thrown(readOnly = false) Throwable t) { + if (t != null || parts == null || parts.isEmpty()) { + return; + } + + AgentSpan agentSpan = AgentTracer.activeSpan(); + if (agentSpan == null) { + return; + } + RequestContext reqCtx = agentSpan.getRequestContext(); + if (reqCtx == null || reqCtx.getData(RequestContextSlot.APPSEC) == null) { + return; + } + + ParameterCollector collector = + new ParameterCollector.ParameterCollectorImpl( + AgentTracer.get() + .getCallbackProvider(RequestContextSlot.APPSEC) + .getCallback(EVENTS.requestFilesContent()) + != null); + + for (Object part : parts) { + collector.addPart(part); + } + + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + + List filenames = collector.getFilenames(); + if (!filenames.isEmpty()) { + BiFunction, Flow> filenamesCb = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (filenamesCb != null) { + Flow flow = filenamesCb.apply(reqCtx, filenames); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (GlassFish multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + + if (t == null) { + List contents = collector.getContents(); + if (!contents.isEmpty()) { + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (contentCb != null) { + Flow contentFlow = contentCb.apply(reqCtx, contents); + Flow.Action contentAction = contentFlow.getAction(); + if (contentAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = + (Flow.Action.RequestBlockingAction) contentAction; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = + new BlockingException( + "Blocked request (GlassFish multipart file upload content)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } + } + } +} From 0d5fd6883a724f6da947a42b65e74d3373573133 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 14:37:46 +0200 Subject: [PATCH 02/13] fix(appsec): add muzzleDirective and requestFilesContent to GlassFish/Payara multipart instrumentation - Add `muzzleDirective() { return "glassfish"; }` to prevent the instrumentation from being included in Tomcat muzzle tests, which would violate the `assertInverse` constraint of the `from703` block and fail CI - Consolidate double `getCallbackProvider()` call into a single variable - Add glassfish muzzle block to build.gradle for CI validation against glassfish-embedded-all artifacts - Remove glassfish-embedded-all from testImplementation (its bundled Guava conflicts with the test bootstrap classpath setup) --- .../tomcat-appsec-7.0/build.gradle | 7 +++++++ .../GlassFishMultipartInstrumentation.java | 21 ++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle index f5542d48255..959a6deba8b 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle @@ -15,6 +15,13 @@ muzzle { extraDependency 'org.apache.tomcat:tomcat-catalina:7.0.4' assertInverse = true } + pass { + name = 'glassfish' + group = 'org.glassfish.main.extras' + module = 'glassfish-embedded-all' + versions = '[4.0,]' + skipVersions += '6.1.0' + } } apply from: "$rootDir/gradle/java.gradle" 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 cbd89ba316a..fb80f0ec62f 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 @@ -24,8 +24,8 @@ /** * GlassFish/Payara does not have {@code Request.parseParts()} — instead {@code Request.getParts()} * delegates to {@code org.apache.catalina.fileupload.Multipart.getParts()}. This instrumentation - * hooks that GlassFish-specific class to report uploaded file names to the AppSec WAF via the - * {@code requestFilesFilenames} IG event. + * hooks that GlassFish-specific class to report uploaded file names and contents to the AppSec WAF + * via the {@code requestFilesFilenames} and {@code requestFilesContent} IG events. * *

Because {@code org.apache.catalina.fileupload.Multipart} does not exist in standard Tomcat, * this instrumentation is automatically skipped by ByteBuddy on non-GlassFish containers. @@ -38,6 +38,11 @@ public GlassFishMultipartInstrumentation() { super("tomcat"); } + @Override + public String muzzleDirective() { + return "glassfish"; + } + @Override public String instrumentedType() { return "org.apache.catalina.fileupload.Multipart"; @@ -61,6 +66,7 @@ public void methodAdvice(MethodTransformer transformer) { } public static class GetPartsAdvice { + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after( @Advice.Return Collection parts, @Advice.Thrown(readOnly = false) Throwable t) { @@ -77,19 +83,14 @@ static void after( return; } - ParameterCollector collector = - new ParameterCollector.ParameterCollectorImpl( - AgentTracer.get() - .getCallbackProvider(RequestContextSlot.APPSEC) - .getCallback(EVENTS.requestFilesContent()) - != null); + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + boolean inspectContent = cbp.getCallback(EVENTS.requestFilesContent()) != null; + ParameterCollector collector = new ParameterCollector.ParameterCollectorImpl(inspectContent); for (Object part : parts) { collector.addPart(part); } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - List filenames = collector.getFilenames(); if (!filenames.isEmpty()) { BiFunction, Flow> filenamesCb = From 9668f398bdc7a4f7b4618c72aa520cbb533fa3ce Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 14:44:42 +0200 Subject: [PATCH 03/13] refactor(appsec): align GlassFish multipart advice with canonical CommonsFileUpload pattern Fetch both callbacks upfront before the collection loop, derive inspectContent from the captured contentCb reference, and add early exit when neither callback is registered. Eliminates a redundant getCallback() call and matches the pattern used in CommonsFileUploadAppSecInstrumentation (PR #11137). --- .../GlassFishMultipartInstrumentation.java | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) 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 fb80f0ec62f..d2f34c61209 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 @@ -84,51 +84,50 @@ static void after( } CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - boolean inspectContent = cbp.getCallback(EVENTS.requestFilesContent()) != null; + BiFunction, Flow> filenamesCb = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (filenamesCb == null && contentCb == null) { + return; + } - ParameterCollector collector = new ParameterCollector.ParameterCollectorImpl(inspectContent); + ParameterCollector collector = + new ParameterCollector.ParameterCollectorImpl(contentCb != null); for (Object part : parts) { collector.addPart(part); } List filenames = collector.getFilenames(); - if (!filenames.isEmpty()) { - BiFunction, Flow> filenamesCb = - cbp.getCallback(EVENTS.requestFilesFilenames()); - if (filenamesCb != null) { - Flow flow = filenamesCb.apply(reqCtx, filenames); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (GlassFish multipart file upload)"); - reqCtx.getTraceSegment().effectivelyBlocked(); - } + if (!filenames.isEmpty() && filenamesCb != null) { + Flow flow = filenamesCb.apply(reqCtx, filenames); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (GlassFish multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); } } } if (t == null) { List contents = collector.getContents(); - if (!contents.isEmpty()) { - BiFunction, Flow> contentCb = - cbp.getCallback(EVENTS.requestFilesContent()); - if (contentCb != null) { - Flow contentFlow = contentCb.apply(reqCtx, contents); - Flow.Action contentAction = contentFlow.getAction(); - if (contentAction instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = - (Flow.Action.RequestBlockingAction) contentAction; - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = - new BlockingException( - "Blocked request (GlassFish multipart file upload content)"); - reqCtx.getTraceSegment().effectivelyBlocked(); - } + if (!contents.isEmpty() && contentCb != null) { + Flow contentFlow = contentCb.apply(reqCtx, contents); + Flow.Action contentAction = contentFlow.getAction(); + if (contentAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = + (Flow.Action.RequestBlockingAction) contentAction; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = + new BlockingException( + "Blocked request (GlassFish multipart file upload content)"); + reqCtx.getTraceSegment().effectivelyBlocked(); } } } From 0878a250d7204a047ed223c7a7c2217f5d7256b9 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 16:48:31 +0200 Subject: [PATCH 04/13] fix(appsec): bypass Java module-system IllegalAccessException in GlassFish multipart instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Java 11+ with GlassFish/Payara, reflective Method.invoke() from the injected ParameterCollector helper (unnamed module) to PartItem (GlassFish named module) fails with IllegalAccessException — setAccessible(true) is also blocked by the module system. Replace reflection-based approach with a direct cast to javax.servlet.http.Part inside the @Advice.OnMethodExit body. Because ByteBuddy inlines the advice into the target class (Multipart), it runs in the same classloader/module context as PartItem, so virtual dispatch through the Part interface works without any reflective access. Changes: - Rewrite GetPartsAdvice to cast each part to javax.servlet.http.Part and call getSubmittedFileName()/getInputStream()/getContentType() directly on the interface - Add compileOnly javax.servlet:javax.servlet-api:3.1.0 so getSubmittedFileName() (added in Servlet 3.1) is available at compile time; tomcat-catalina:7.0.4 ships only Servlet 3.0 - Add setAccessible(true) to ParameterCollector.resolveAndCache() (defensive; used by ParsePartsInstrumentation on Tomcat where reflection still works) - Remove helperClassNames() override — no helper injection needed for GlassFish path --- .../tomcat-appsec-7.0/build.gradle | 3 + .../GlassFishMultipartInstrumentation.java | 87 ++++++++++++------- .../tomcat7/ParameterCollector.java | 4 + 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle index 959a6deba8b..7709c17d148 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle @@ -29,6 +29,9 @@ apply from: "$rootDir/gradle/java.gradle" dependencies { compileOnly group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '7.0.4' compileOnly group: 'org.apache.tomcat', name: 'tomcat-coyote', version: '7.0.4' + // Servlet 3.1 API needed to reference Part.getSubmittedFileName() in GlassFishMultipartInstrumentation. + // tomcat-catalina:7.0.4 provides only Servlet 3.0 (no getSubmittedFileName); GlassFish 4+ is Servlet 3.1. + compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.1.0' implementation project(':dd-java-agent:instrumentation:tomcat:tomcat-common') testImplementation group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '7.0.4' 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 d2f34c61209..8372a62dd34 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 @@ -14,11 +14,15 @@ 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.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.io.InputStream; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.function.BiFunction; +import javax.servlet.http.Part; import net.bytebuddy.asm.Advice; /** @@ -29,6 +33,10 @@ * *

Because {@code org.apache.catalina.fileupload.Multipart} does not exist in standard Tomcat, * this instrumentation is automatically skipped by ByteBuddy on non-GlassFish containers. + * + *

This advice casts each {@code Part} through the {@code javax.servlet.http.Part} interface + * (which {@code org.apache.catalina.fileupload.PartItem} implements) to avoid Java module-system + * access restrictions that prevent reflective invocation of methods on GlassFish-internal classes. */ @AutoService(InstrumenterModule.class) public class GlassFishMultipartInstrumentation extends InstrumenterModule.AppSec @@ -48,16 +56,6 @@ public String instrumentedType() { return "org.apache.catalina.fileupload.Multipart"; } - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.instrumentation.tomcat7.ParameterCollector", - "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorNoop", - "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorImpl", - "datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorImpl$CachedMethods", - }; - } - @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -92,14 +90,45 @@ static void after( return; } - ParameterCollector collector = - new ParameterCollector.ParameterCollectorImpl(contentCb != null); - for (Object part : parts) { - collector.addPart(part); + int maxFiles = datadog.trace.api.Config.get().getAppSecMaxFileContentCount(); + int maxBytes = datadog.trace.api.Config.get().getAppSecMaxFileContentBytes(); + + List filenames = null; + List contents = null; + + for (Object partObj : parts) { + if (!(partObj instanceof Part)) { + continue; + } + Part part = (Part) partObj; + String filename = part.getSubmittedFileName(); + // null means no filename parameter → form field, skip + // empty string means filename="" was sent → file upload without a name + if (filename == null) { + continue; + } + if (!filename.isEmpty()) { + if (filenames == null) { + filenames = new ArrayList<>(); + } + filenames.add(filename); + } + if (contentCb != null) { + if (contents == null) { + contents = new ArrayList<>(); + } + if (contents.size() < maxFiles) { + try (InputStream is = part.getInputStream()) { + contents.add( + MultipartContentDecoder.readInputStream(is, maxBytes, part.getContentType())); + } catch (Exception ignored) { + contents.add(""); + } + } + } } - List filenames = collector.getFilenames(); - if (!filenames.isEmpty() && filenamesCb != null) { + if (filenames != null && !filenames.isEmpty() && filenamesCb != null) { Flow flow = filenamesCb.apply(reqCtx, filenames); Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { @@ -113,22 +142,16 @@ static void after( } } - if (t == null) { - List contents = collector.getContents(); - if (!contents.isEmpty() && contentCb != null) { - Flow contentFlow = contentCb.apply(reqCtx, contents); - Flow.Action contentAction = contentFlow.getAction(); - if (contentAction instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = - (Flow.Action.RequestBlockingAction) contentAction; - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = - new BlockingException( - "Blocked request (GlassFish multipart file upload content)"); - reqCtx.getTraceSegment().effectivelyBlocked(); - } + if (t == null && contents != null && !contents.isEmpty() && contentCb != null) { + Flow contentFlow = contentCb.apply(reqCtx, contents); + Flow.Action contentAction = contentFlow.getAction(); + if (contentAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (GlassFish multipart file upload content)"); + reqCtx.getTraceSegment().effectivelyBlocked(); } } } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java index faf095fb4f5..296f7902c0f 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java @@ -172,19 +172,23 @@ private static CachedMethods resolveAndCache(Class partClass) { Method getFilename = null; try { getInputStream = partClass.getMethod("getInputStream"); + getInputStream.setAccessible(true); } catch (Exception ignored) { } try { getContentType = partClass.getMethod("getContentType"); + getContentType.setAccessible(true); } catch (Exception ignored) { } try { getFilename = partClass.getMethod("getSubmittedFileName"); + getFilename.setAccessible(true); } catch (Exception ignored) { } if (getFilename == null) { try { getFilename = partClass.getMethod("getFilename"); + getFilename.setAccessible(true); } catch (Exception ignored) { } } From 2fc449472e9784390bbb1498e95d849afe5c34eb Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 16:57:31 +0200 Subject: [PATCH 05/13] fix(appsec): protect each GlassFish multipart part iteration with per-item try/catch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An uncaught exception from getSubmittedFileName() on part N would short-circuit the loop silently, leaving parts N+1..M unprocessed and the WAF with partial data. suppress=Throwable.class on @Advice.OnMethodExit only swallows the exception after the loop exits — it does not protect individual iterations. Wrap the entire per-part body in try/catch(Exception ignored) so a broken PartItem skips that part without affecting the remaining ones. --- .../GlassFishMultipartInstrumentation.java | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) 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 8372a62dd34..23d5956c089 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 @@ -97,34 +97,37 @@ static void after( List contents = null; for (Object partObj : parts) { - if (!(partObj instanceof Part)) { - continue; - } - Part part = (Part) partObj; - String filename = part.getSubmittedFileName(); - // null means no filename parameter → form field, skip - // empty string means filename="" was sent → file upload without a name - if (filename == null) { - continue; - } - if (!filename.isEmpty()) { - if (filenames == null) { - filenames = new ArrayList<>(); + try { + if (!(partObj instanceof Part)) { + continue; } - filenames.add(filename); - } - if (contentCb != null) { - if (contents == null) { - contents = new ArrayList<>(); + Part part = (Part) partObj; + String filename = part.getSubmittedFileName(); + // null means no filename parameter → form field, skip + // empty string means filename="" was sent → file upload without a name + if (filename == null) { + continue; } - if (contents.size() < maxFiles) { - try (InputStream is = part.getInputStream()) { - contents.add( - MultipartContentDecoder.readInputStream(is, maxBytes, part.getContentType())); - } catch (Exception ignored) { - contents.add(""); + if (!filename.isEmpty()) { + if (filenames == null) { + filenames = new ArrayList<>(); + } + filenames.add(filename); + } + if (contentCb != null) { + if (contents == null) { + contents = new ArrayList<>(); + } + if (contents.size() < maxFiles) { + try (InputStream is = part.getInputStream()) { + contents.add( + MultipartContentDecoder.readInputStream(is, maxBytes, part.getContentType())); + } catch (Exception ignored) { + contents.add(""); + } } } + } catch (Exception ignored) { } } From c08eb04d10ff04f5c2b942ae6d861af80ba86d82 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 17:05:17 +0200 Subject: [PATCH 06/13] revert: remove unnecessary setAccessible(true) from ParameterCollector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These calls were added during an attempt to fix the GlassFish IllegalAccessException via reflection, but that approach was superseded by inlining the advice directly into Multipart (which avoids reflection entirely). ParameterCollector is only used by ParsePartsInstrumentation on standard Tomcat, where reflection on ApplicationPart works without setAccessible(true) — as tests confirmed before this change. --- .../trace/instrumentation/tomcat7/ParameterCollector.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java index 296f7902c0f..faf095fb4f5 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java @@ -172,23 +172,19 @@ private static CachedMethods resolveAndCache(Class partClass) { Method getFilename = null; try { getInputStream = partClass.getMethod("getInputStream"); - getInputStream.setAccessible(true); } catch (Exception ignored) { } try { getContentType = partClass.getMethod("getContentType"); - getContentType.setAccessible(true); } catch (Exception ignored) { } try { getFilename = partClass.getMethod("getSubmittedFileName"); - getFilename.setAccessible(true); } catch (Exception ignored) { } if (getFilename == null) { try { getFilename = partClass.getMethod("getFilename"); - getFilename.setAccessible(true); } catch (Exception ignored) { } } From c5091b1fda492e4863484d82c3cd5691be372dfe Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 17:14:24 +0200 Subject: [PATCH 07/13] style: replace FQN datadog.trace.api.Config with regular import --- .../tomcat7/GlassFishMultipartInstrumentation.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 23d5956c089..1a111cdb730 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 @@ -9,6 +9,7 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.Config; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -90,8 +91,8 @@ static void after( return; } - int maxFiles = datadog.trace.api.Config.get().getAppSecMaxFileContentCount(); - int maxBytes = datadog.trace.api.Config.get().getAppSecMaxFileContentBytes(); + int maxFiles = Config.get().getAppSecMaxFileContentCount(); + int maxBytes = Config.get().getAppSecMaxFileContentBytes(); List filenames = null; List contents = null; From bf025713daf17213e829e0e101ffb465086d79da Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 4 May 2026 17:22:06 +0200 Subject: [PATCH 08/13] style: skip filenames collection when filenamesCb is null Align with CommonsFileUploadAppSecInstrumentation: only populate the filenames list when filenamesCallback is actually registered. Previously the list was created and filled regardless, then silently dropped at dispatch time. No correctness impact, but avoids unnecessary ArrayList allocation and string copies when only the content callback is registered. --- .../tomcat7/GlassFishMultipartInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1a111cdb730..aa73cba2991 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 @@ -109,7 +109,7 @@ static void after( if (filename == null) { continue; } - if (!filename.isEmpty()) { + if (filenamesCb != null && !filename.isEmpty()) { if (filenames == null) { filenames = new ArrayList<>(); } From 6e7f1298b20d9cedee865413c89d48dee6b19df0 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 6 May 2026 10:28:49 +0200 Subject: [PATCH 09/13] feat(appsec): add blocking fallback for GlassFish/Payara multipart instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TomcatServerInstrumentation is muzzled out for Payara's response type, so BlockResponseFunction is never registered. Add GlassFishBlockingHelper that commits the blocking response directly via Servlet API, extracting the HttpServletResponse from Multipart.request (private field) via reflection. The setAccessible call works because the advice is inlined into Multipart.getParts() — the same module as the field owner. Verified against system-tests APPSEC_BLOCKING with spring-boot-payara: - Test_Blocking_request_body_filenames: passes (was missing_feature) - Test_Blocking_request_body_files_content: passes (was missing_feature) --- .../tomcat7/GlassFishBlockingHelper.java | 48 +++++++++++++++++++ .../GlassFishMultipartInstrumentation.java | 44 ++++++++++++++++- 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java 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 new file mode 100644 index 00000000000..4b4a5211be0 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java @@ -0,0 +1,48 @@ +package datadog.trace.instrumentation.tomcat7; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.gateway.Flow; +import datadog.trace.bootstrap.blocking.BlockingActionHelper; +import java.io.OutputStream; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public final class GlassFishBlockingHelper { + + public static boolean commitBlocking( + HttpServletRequest request, + HttpServletResponse response, + Flow.Action.RequestBlockingAction rba) { + if (response == null) { + return false; + } + try { + if (response.isCommitted()) { + return false; + } + response.reset(); + response.setStatus(BlockingActionHelper.getHttpCode(rba.getStatusCode())); + for (Map.Entry e : rba.getExtraHeaders().entrySet()) { + response.setHeader(e.getKey(), e.getValue()); + } + if (rba.getBlockingContentType() != BlockingContentType.NONE) { + String accept = request != null ? request.getHeader("Accept") : null; + BlockingActionHelper.TemplateType type = + BlockingActionHelper.determineTemplateType(rba.getBlockingContentType(), accept); + byte[] body = BlockingActionHelper.getTemplate(type, rba.getSecurityResponseId()); + if (body != null) { + response.setHeader("Content-Type", BlockingActionHelper.getContentType(type)); + response.setHeader("Content-Length", Integer.toString(body.length)); + OutputStream os = response.getOutputStream(); + os.write(body); + os.close(); + } + } + response.flushBuffer(); + return true; + } catch (Exception e) { + return false; + } + } +} 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 aa73cba2991..f4aca5f2a25 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 @@ -19,10 +19,14 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.InputStream; +import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.function.BiFunction; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import javax.servlet.http.Part; import net.bytebuddy.asm.Advice; @@ -57,6 +61,13 @@ public String instrumentedType() { return "org.apache.catalina.fileupload.Multipart"; } + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.tomcat7.GlassFishBlockingHelper", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -68,7 +79,9 @@ public static class GetPartsAdvice { @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after( - @Advice.Return Collection parts, @Advice.Thrown(readOnly = false) Throwable t) { + @Advice.This Object thisMultipart, + @Advice.Return Collection parts, + @Advice.Thrown(readOnly = false) Throwable t) { if (t != null || parts == null || parts.isEmpty()) { return; } @@ -91,6 +104,29 @@ static void after( return; } + // Extract servlet request/response for fallback blocking when no BlockResponseFunction is + // registered (Payara: TomcatServerInstrumentation is muzzled out for Payara's response type). + // setAccessible works here because this code is inlined into Multipart.getParts() — + // the same module as the private field's owner class. + HttpServletRequest fallbackReq = null; + HttpServletResponse fallbackResp = null; + try { + Field f = thisMultipart.getClass().getDeclaredField("request"); + f.setAccessible(true); + Object catReq = f.get(thisMultipart); + if (catReq instanceof HttpServletRequest) { + fallbackReq = (HttpServletRequest) catReq; + } + if (catReq != null) { + Method m = catReq.getClass().getMethod("getResponse"); + Object catResp = m.invoke(catReq); + if (catResp instanceof HttpServletResponse) { + fallbackResp = (HttpServletResponse) catResp; + } + } + } catch (Exception ignored) { + } + int maxFiles = Config.get().getAppSecMaxFileContentCount(); int maxBytes = Config.get().getAppSecMaxFileContentBytes(); @@ -142,6 +178,9 @@ static void after( brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); t = new BlockingException("Blocked request (GlassFish multipart file upload)"); reqCtx.getTraceSegment().effectivelyBlocked(); + } else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) { + t = new BlockingException("Blocked request (GlassFish multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); } } } @@ -156,6 +195,9 @@ static void after( brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); t = new BlockingException("Blocked request (GlassFish multipart file upload content)"); reqCtx.getTraceSegment().effectivelyBlocked(); + } else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) { + t = new BlockingException("Blocked request (GlassFish multipart file upload content)"); + reqCtx.getTraceSegment().effectivelyBlocked(); } } } From b0fe146ebf02c61ed105fd0498073f1e3103c048 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 6 May 2026 11:41:02 +0200 Subject: [PATCH 10/13] style(appsec): address PR review feedback on GlassFish multipart instrumentation - Use try-with-resources for OutputStream in GlassFishBlockingHelper - Cache Config limit values as static finals in GlassFishBlockingHelper to avoid per-request Config.get() calls (consistent with ParameterCollector) - Remove Config import from GlassFishMultipartInstrumentation (now reads limits from GlassFishBlockingHelper static fields) - Add comment to skipVersions explaining the jakarta namespace reason - Add assertInverse to glassfish muzzle block for defensive CI coverage --- .../tomcat-appsec/tomcat-appsec-7.0/build.gradle | 3 ++- .../tomcat7/GlassFishBlockingHelper.java | 10 +++++++--- .../tomcat7/GlassFishMultipartInstrumentation.java | 5 ++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle index 7709c17d148..4d2830da3f9 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle @@ -20,7 +20,8 @@ muzzle { group = 'org.glassfish.main.extras' module = 'glassfish-embedded-all' versions = '[4.0,]' - skipVersions += '6.1.0' + skipVersions += '6.1.0' // GlassFish 6.1.0+ uses jakarta.* namespace; our advice uses javax.servlet.http.Part + assertInverse = 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 4b4a5211be0..8413ac4ec0e 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 @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.tomcat7; import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.Config; import datadog.trace.api.gateway.Flow; import datadog.trace.bootstrap.blocking.BlockingActionHelper; import java.io.OutputStream; @@ -10,6 +11,9 @@ public final class GlassFishBlockingHelper { + static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount(); + static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static boolean commitBlocking( HttpServletRequest request, HttpServletResponse response, @@ -34,9 +38,9 @@ public static boolean commitBlocking( if (body != null) { response.setHeader("Content-Type", BlockingActionHelper.getContentType(type)); response.setHeader("Content-Length", Integer.toString(body.length)); - OutputStream os = response.getOutputStream(); - os.write(body); - os.close(); + try (OutputStream os = response.getOutputStream()) { + os.write(body); + } } } response.flushBuffer(); 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 f4aca5f2a25..73776136178 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 @@ -9,7 +9,6 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.Config; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; @@ -127,8 +126,8 @@ static void after( } catch (Exception ignored) { } - int maxFiles = Config.get().getAppSecMaxFileContentCount(); - int maxBytes = Config.get().getAppSecMaxFileContentBytes(); + int maxFiles = GlassFishBlockingHelper.MAX_FILE_CONTENT_COUNT; + int maxBytes = GlassFishBlockingHelper.MAX_FILE_CONTENT_BYTES; List filenames = null; List contents = null; From 13388cfd6c775f4e01d75c63d0a39286ea2fec12 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 6 May 2026 16:47:47 +0200 Subject: [PATCH 11/13] fix: restrict GlassFish muzzle range to [4.0, 6.1.0) to exclude jakarta.* versions --- .../tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle index 4d2830da3f9..8de6e77af9f 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle @@ -19,8 +19,7 @@ muzzle { name = 'glassfish' group = 'org.glassfish.main.extras' module = 'glassfish-embedded-all' - versions = '[4.0,]' - skipVersions += '6.1.0' // GlassFish 6.1.0+ uses jakarta.* namespace; our advice uses javax.servlet.http.Part + versions = '[4.0, 6.1.0)' // GlassFish 6.1.0+ uses jakarta.* namespace; our advice uses javax.servlet.http.Part assertInverse = true } } From aaac59e106d07418151df6a03acf90eeae8fa051 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 7 May 2026 16:16:25 +0200 Subject: [PATCH 12/13] fix(appsec): make GlassFishBlockingHelper fields public and avoid BlockingException for Payara blocking ByteBuddy inlines advice into Multipart.getParts() (package org.apache.catalina.fileupload), which is a different package than GlassFishBlockingHelper. Package-private static fields caused IllegalAccessError at runtime crashing the Payara container. BlockingException thrown from getParts() propagated through Payara's Grizzly pipeline and caused an ungraceful shutdown. Replace with parts = Collections.emptyList() so the response is committed (via commitBlocking fallback) and the controller skips processing the parts. --- .../tomcat7/GlassFishBlockingHelper.java | 9 +++------ .../GlassFishMultipartInstrumentation.java | 20 +++++++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) 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 8413ac4ec0e..c1f6c5fcd4e 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 @@ -4,15 +4,14 @@ import datadog.trace.api.Config; import datadog.trace.api.gateway.Flow; import datadog.trace.bootstrap.blocking.BlockingActionHelper; -import java.io.OutputStream; import java.util.Map; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; public final class GlassFishBlockingHelper { - static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount(); - static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount(); + public static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); public static boolean commitBlocking( HttpServletRequest request, @@ -38,9 +37,7 @@ public static boolean commitBlocking( if (body != null) { response.setHeader("Content-Type", BlockingActionHelper.getContentType(type)); response.setHeader("Content-Length", Integer.toString(body.length)); - try (OutputStream os = response.getOutputStream()) { - os.write(body); - } + response.getOutputStream().write(body); } } response.flushBuffer(); 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 73776136178..bdf55a63e59 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 @@ -6,7 +6,6 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.gateway.BlockResponseFunction; @@ -22,6 +21,7 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.function.BiFunction; import javax.servlet.http.HttpServletRequest; @@ -79,8 +79,8 @@ public static class GetPartsAdvice { @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after( @Advice.This Object thisMultipart, - @Advice.Return Collection parts, - @Advice.Thrown(readOnly = false) Throwable t) { + @Advice.Return(readOnly = false) Collection parts, + @Advice.Thrown Throwable t) { if (t != null || parts == null || parts.isEmpty()) { return; } @@ -167,6 +167,8 @@ static void after( } } + boolean blocked = false; + if (filenames != null && !filenames.isEmpty() && filenamesCb != null) { Flow flow = filenamesCb.apply(reqCtx, filenames); Flow.Action action = flow.getAction(); @@ -175,16 +177,18 @@ static void after( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (GlassFish multipart file upload)"); + parts = Collections.emptyList(); reqCtx.getTraceSegment().effectivelyBlocked(); + blocked = true; } else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) { - t = new BlockingException("Blocked request (GlassFish multipart file upload)"); + parts = Collections.emptyList(); reqCtx.getTraceSegment().effectivelyBlocked(); + blocked = true; } } } - if (t == null && contents != null && !contents.isEmpty() && contentCb != null) { + if (!blocked && contents != null && !contents.isEmpty() && contentCb != null) { Flow contentFlow = contentCb.apply(reqCtx, contents); Flow.Action contentAction = contentFlow.getAction(); if (contentAction instanceof Flow.Action.RequestBlockingAction) { @@ -192,10 +196,10 @@ static void after( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (GlassFish multipart file upload content)"); + parts = Collections.emptyList(); reqCtx.getTraceSegment().effectivelyBlocked(); } else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) { - t = new BlockingException("Blocked request (GlassFish multipart file upload content)"); + parts = Collections.emptyList(); reqCtx.getTraceSegment().effectivelyBlocked(); } } From a45b4048f78309b9316f669b5d0c2590e5d97b23 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 7 May 2026 16:53:05 +0200 Subject: [PATCH 13/13] refactor(appsec): extract tryBlock() helper and add unit tests for GlassFishBlockingHelper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the repeated brf/fallback blocking pattern from the GlassFish multipart advice into GlassFishBlockingHelper.tryBlock(). This eliminates the duplication between the filenames and content blocking paths, and fixes the ordering issue where blocked=true was assigned after effectivelyBlocked() — if effectivelyBlocked() threw, blocked would stay false and the content callback would fire after filenames already blocked. tryBlock() uses a two-phase try/catch: the outer block handles commit failures (returning false), while the inner block wraps effectivelyBlocked() so a thrown exception does not suppress the true return value when the response was already committed. Add GlassFishBlockingHelperTest covering all branches of commitBlocking() and tryBlock(). --- .../tomcat-appsec-7.0/build.gradle | 2 + .../tomcat7/GlassFishBlockingHelper.java | 35 +++ .../GlassFishMultipartInstrumentation.java | 25 +-- .../tomcat7/GlassFishBlockingHelperTest.java | 211 ++++++++++++++++++ 4 files changed, 255 insertions(+), 18 deletions(-) create mode 100644 dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelperTest.java diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle index 8de6e77af9f..1d3cc39c2c4 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/build.gradle @@ -36,6 +36,8 @@ dependencies { testImplementation group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '7.0.4' testImplementation group: 'org.apache.tomcat', name: 'tomcat-coyote', version: '7.0.4' + testImplementation group: 'javax.servlet', name: 'javax.servlet-api', version: '3.1.0' + testImplementation libs.bundles.mockito } // testing happens in tomcat-5.5 module 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 c1f6c5fcd4e..5062468ae8d 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 @@ -2,7 +2,9 @@ import datadog.appsec.api.blocking.BlockingContentType; import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; import datadog.trace.bootstrap.blocking.BlockingActionHelper; import java.util.Map; import javax.servlet.http.HttpServletRequest; @@ -13,6 +15,39 @@ public final class GlassFishBlockingHelper { public static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount(); public static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + /** + * Attempts to commit a blocking response via the registered {@link BlockResponseFunction} or via + * the Servlet API fallback, then marks the trace segment as effectively blocked. + * + *

Returns {@code true} if the response was committed (regardless of whether {@link + * datadog.trace.api.internal.TraceSegment#effectivelyBlocked()} succeeded). Returns {@code false} + * if no response could be committed. + */ + public static boolean tryBlock( + RequestContext reqCtx, + HttpServletRequest fallbackReq, + HttpServletResponse fallbackResp, + Flow.Action.RequestBlockingAction rba) { + try { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + } else if (!commitBlocking(fallbackReq, fallbackResp, rba)) { + return false; + } + } catch (Exception ignored) { + return false; + } + // Response was committed — mark as blocked on a best-effort basis. + // effectivelyBlocked() can throw if the span is already finished; that must not suppress the + // true return value since the response has already been sent to the client. + try { + reqCtx.getTraceSegment().effectivelyBlocked(); + } catch (Exception ignored) { + } + return true; + } + public static boolean commitBlocking( HttpServletRequest request, HttpServletResponse response, 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 bdf55a63e59..779b2aed8c4 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 @@ -8,7 +8,6 @@ import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -173,16 +172,9 @@ static void after( Flow flow = filenamesCb.apply(reqCtx, filenames); Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (GlassFishBlockingHelper.tryBlock( + reqCtx, fallbackReq, fallbackResp, (Flow.Action.RequestBlockingAction) action)) { parts = Collections.emptyList(); - reqCtx.getTraceSegment().effectivelyBlocked(); - blocked = true; - } else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) { - parts = Collections.emptyList(); - reqCtx.getTraceSegment().effectivelyBlocked(); blocked = true; } } @@ -192,15 +184,12 @@ static void after( Flow contentFlow = contentCb.apply(reqCtx, contents); Flow.Action contentAction = contentFlow.getAction(); if (contentAction instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction; - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - parts = Collections.emptyList(); - reqCtx.getTraceSegment().effectivelyBlocked(); - } else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) { + if (GlassFishBlockingHelper.tryBlock( + reqCtx, + fallbackReq, + fallbackResp, + (Flow.Action.RequestBlockingAction) contentAction)) { parts = Collections.emptyList(); - 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 new file mode 100644 index 00000000000..c7d046d8756 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelperTest.java @@ -0,0 +1,211 @@ +package datadog.trace.instrumentation.tomcat7; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.internal.TraceSegment; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.Test; + +class GlassFishBlockingHelperTest { + + // ------- commitBlocking() ------- + + @Test + void commitBlocking_nullResponse_returnsFalse() { + assertFalse(GlassFishBlockingHelper.commitBlocking(null, null, rba(403))); + } + + @Test + void commitBlocking_committedResponse_returnsFalse() { + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(true); + assertFalse(GlassFishBlockingHelper.commitBlocking(null, resp, rba(403))); + } + + @Test + void commitBlocking_blockingContentTypeNone_setsStatusWithoutBody() throws IOException { + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(false); + + assertTrue( + GlassFishBlockingHelper.commitBlocking( + null, resp, new Flow.Action.RequestBlockingAction(403, BlockingContentType.NONE))); + + verify(resp).setStatus(403); + verify(resp).flushBuffer(); + verify(resp, never()).setHeader(eq("Content-Type"), any()); + verify(resp, never()).getOutputStream(); + } + + @Test + void commitBlocking_withJsonAccept_writesJsonBody() throws IOException { + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeader("Accept")).thenReturn("application/json"); + TestServletOutputStream out = new TestServletOutputStream(); + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(false); + when(resp.getOutputStream()).thenReturn(out); + + assertTrue(GlassFishBlockingHelper.commitBlocking(req, resp, rba(403))); + + verify(resp).setStatus(403); + verify(resp).setHeader(eq("Content-Type"), contains("json")); + verify(resp).setHeader(eq("Content-Length"), any()); + assertTrue(out.getBytes().length > 0); + verify(resp).flushBuffer(); + } + + @Test + void commitBlocking_withHtmlAccept_writesHtmlBody() throws IOException { + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeader("Accept")).thenReturn("text/html"); + TestServletOutputStream out = new TestServletOutputStream(); + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(false); + when(resp.getOutputStream()).thenReturn(out); + + assertTrue(GlassFishBlockingHelper.commitBlocking(req, resp, rba(403))); + + verify(resp).setHeader(eq("Content-Type"), contains("html")); + assertTrue(out.getBytes().length > 0); + } + + @Test + void commitBlocking_nullRequest_defaultsToJsonBody() throws IOException { + TestServletOutputStream out = new TestServletOutputStream(); + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(false); + when(resp.getOutputStream()).thenReturn(out); + + assertTrue(GlassFishBlockingHelper.commitBlocking(null, resp, rba(403))); + + verify(resp).setStatus(403); + assertTrue(out.getBytes().length > 0); + } + + @Test + void commitBlocking_ioException_returnsFalse() throws IOException { + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(false); + when(resp.getOutputStream()).thenThrow(new IOException("stream error")); + + assertFalse(GlassFishBlockingHelper.commitBlocking(null, resp, rba(403))); + } + + // ------- tryBlock() ------- + + @Test + void tryBlock_withBrf_commitsViaFunctionAndReturnsTrue() throws Exception { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); + RequestContext reqCtx = mockReqCtx(brf, segment); + + Flow.Action.RequestBlockingAction action = rba(403); + assertTrue(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, action)); + + verify(brf).tryCommitBlockingResponse(segment, action); + verify(segment).effectivelyBlocked(); + } + + @Test + void tryBlock_noBrf_fallbackSucceeds_returnsTrue() throws IOException { + TraceSegment segment = mock(TraceSegment.class); + RequestContext reqCtx = mockReqCtx(null, segment); + TestServletOutputStream out = new TestServletOutputStream(); + HttpServletResponse resp = mock(HttpServletResponse.class); + when(resp.isCommitted()).thenReturn(false); + when(resp.getOutputStream()).thenReturn(out); + + assertTrue(GlassFishBlockingHelper.tryBlock(reqCtx, null, resp, rba(403))); + verify(segment).effectivelyBlocked(); + } + + @Test + void tryBlock_noBrf_nullFallbackResponse_returnsFalse() { + RequestContext reqCtx = mock(RequestContext.class); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + verify(reqCtx, never()).getTraceSegment(); + } + + @Test + void tryBlock_brfThrows_returnsFalse() throws Exception { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); + RequestContext reqCtx = mockReqCtx(brf, segment); + doThrow(new RuntimeException("commit failed")) + .when(brf) + .tryCommitBlockingResponse(any(), any(Flow.Action.RequestBlockingAction.class)); + + assertFalse(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + verify(segment, never()).effectivelyBlocked(); + } + + @Test + void tryBlock_effectivelyBlockedThrows_stillReturnsTrue() throws Exception { + TraceSegment segment = mock(TraceSegment.class); + BlockResponseFunction brf = mock(BlockResponseFunction.class); + RequestContext reqCtx = mockReqCtx(brf, segment); + doThrow(new RuntimeException("span already finished")).when(segment).effectivelyBlocked(); + + assertTrue(GlassFishBlockingHelper.tryBlock(reqCtx, null, null, rba(403))); + } + + // ------- Helpers ------- + + private static Flow.Action.RequestBlockingAction rba(int statusCode) { + return new Flow.Action.RequestBlockingAction(statusCode, BlockingContentType.AUTO); + } + + private static RequestContext mockReqCtx(BlockResponseFunction brf, TraceSegment segment) { + RequestContext reqCtx = mock(RequestContext.class); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + when(reqCtx.getTraceSegment()).thenReturn(segment); + return reqCtx; + } + + private static final class TestServletOutputStream extends ServletOutputStream { + private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setWriteListener(WriteListener listener) {} + + @Override + public void write(int b) throws IOException { + buffer.write(b); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + buffer.write(b, off, len); + } + + public byte[] getBytes() { + return buffer.toByteArray(); + } + } +}