Skip to content

Commit aaac59e

Browse files
committed
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.
1 parent e3426cd commit aaac59e

2 files changed

Lines changed: 15 additions & 14 deletions

File tree

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@
44
import datadog.trace.api.Config;
55
import datadog.trace.api.gateway.Flow;
66
import datadog.trace.bootstrap.blocking.BlockingActionHelper;
7-
import java.io.OutputStream;
87
import java.util.Map;
98
import javax.servlet.http.HttpServletRequest;
109
import javax.servlet.http.HttpServletResponse;
1110

1211
public final class GlassFishBlockingHelper {
1312

14-
static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount();
15-
static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes();
13+
public static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount();
14+
public static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes();
1615

1716
public static boolean commitBlocking(
1817
HttpServletRequest request,
@@ -38,9 +37,7 @@ public static boolean commitBlocking(
3837
if (body != null) {
3938
response.setHeader("Content-Type", BlockingActionHelper.getContentType(type));
4039
response.setHeader("Content-Length", Integer.toString(body.length));
41-
try (OutputStream os = response.getOutputStream()) {
42-
os.write(body);
43-
}
40+
response.getOutputStream().write(body);
4441
}
4542
}
4643
response.flushBuffer();

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
77

88
import com.google.auto.service.AutoService;
9-
import datadog.appsec.api.blocking.BlockingException;
109
import datadog.trace.agent.tooling.Instrumenter;
1110
import datadog.trace.agent.tooling.InstrumenterModule;
1211
import datadog.trace.api.gateway.BlockResponseFunction;
@@ -22,6 +21,7 @@
2221
import java.lang.reflect.Method;
2322
import java.util.ArrayList;
2423
import java.util.Collection;
24+
import java.util.Collections;
2525
import java.util.List;
2626
import java.util.function.BiFunction;
2727
import javax.servlet.http.HttpServletRequest;
@@ -79,8 +79,8 @@ public static class GetPartsAdvice {
7979
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
8080
static void after(
8181
@Advice.This Object thisMultipart,
82-
@Advice.Return Collection<?> parts,
83-
@Advice.Thrown(readOnly = false) Throwable t) {
82+
@Advice.Return(readOnly = false) Collection<?> parts,
83+
@Advice.Thrown Throwable t) {
8484
if (t != null || parts == null || parts.isEmpty()) {
8585
return;
8686
}
@@ -167,6 +167,8 @@ static void after(
167167
}
168168
}
169169

170+
boolean blocked = false;
171+
170172
if (filenames != null && !filenames.isEmpty() && filenamesCb != null) {
171173
Flow<Void> flow = filenamesCb.apply(reqCtx, filenames);
172174
Flow.Action action = flow.getAction();
@@ -175,27 +177,29 @@ static void after(
175177
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
176178
if (brf != null) {
177179
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
178-
t = new BlockingException("Blocked request (GlassFish multipart file upload)");
180+
parts = Collections.emptyList();
179181
reqCtx.getTraceSegment().effectivelyBlocked();
182+
blocked = true;
180183
} else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) {
181-
t = new BlockingException("Blocked request (GlassFish multipart file upload)");
184+
parts = Collections.emptyList();
182185
reqCtx.getTraceSegment().effectivelyBlocked();
186+
blocked = true;
183187
}
184188
}
185189
}
186190

187-
if (t == null && contents != null && !contents.isEmpty() && contentCb != null) {
191+
if (!blocked && contents != null && !contents.isEmpty() && contentCb != null) {
188192
Flow<Void> contentFlow = contentCb.apply(reqCtx, contents);
189193
Flow.Action contentAction = contentFlow.getAction();
190194
if (contentAction instanceof Flow.Action.RequestBlockingAction) {
191195
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction;
192196
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
193197
if (brf != null) {
194198
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
195-
t = new BlockingException("Blocked request (GlassFish multipart file upload content)");
199+
parts = Collections.emptyList();
196200
reqCtx.getTraceSegment().effectivelyBlocked();
197201
} else if (GlassFishBlockingHelper.commitBlocking(fallbackReq, fallbackResp, rba)) {
198-
t = new BlockingException("Blocked request (GlassFish multipart file upload content)");
202+
parts = Collections.emptyList();
199203
reqCtx.getTraceSegment().effectivelyBlocked();
200204
}
201205
}

0 commit comments

Comments
 (0)