Skip to content

Commit ecf65c5

Browse files
committed
Fix GetFilenamesAdvice double-firing and extend coverage to getParts(MultiMap) path
- jetty-appsec-9.3: add call-depth guard (Collection.class) to GetFilenamesAdvice to prevent double callback invocation when getParts() calls getParts(MultiMap) internally - jetty-appsec-9.2: extend GetFilenamesAdvice matcher to all getParts overloads (not just no-arg) to cover getParameter*()/getParameterMap() code paths, guarded with same call-depth mechanism to avoid double-firing
1 parent b276e16 commit ecf65c5

File tree

3 files changed

+19
-8
lines changed

3 files changed

+19
-8
lines changed

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/RequestGetPartsInstrumentation.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,7 @@ static void after(
216216
// Resolve getSubmittedFileName once (Servlet 3.1+; null on Servlet 3.0)
217217
Method getSubmittedFileName = null;
218218
try {
219-
getSubmittedFileName =
220-
parts.iterator().next().getClass().getMethod("getSubmittedFileName");
219+
getSubmittedFileName = parts.iterator().next().getClass().getMethod("getSubmittedFileName");
221220
} catch (Exception ignored) {
222221
}
223222
List<String> filenames = new ArrayList<>();

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ public void methodAdvice(MethodTransformer transformer) {
5252
.and(takesArguments(1))
5353
.and(takesArgument(0, named("org.eclipse.jetty.util.MultiMap"))),
5454
getClass().getName() + "$GetPartsAdvice");
55-
transformer.applyAdvice(
56-
named("getParts").and(takesArguments(0)), getClass().getName() + "$GetFilenamesAdvice");
55+
transformer.applyAdvice(named("getParts"), getClass().getName() + "$GetFilenamesAdvice");
5756
}
5857

5958
private static final Reference REQUEST_REFERENCE =
@@ -144,12 +143,19 @@ static void after(
144143

145144
@RequiresRequestContext(RequestContextSlot.APPSEC)
146145
public static class GetFilenamesAdvice {
146+
@Advice.OnMethodEnter(suppress = Throwable.class)
147+
static boolean before() {
148+
return CallDepthThreadLocalMap.incrementCallDepth(Collection.class) == 0;
149+
}
150+
147151
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
148152
static void after(
153+
@Advice.Enter boolean proceed,
149154
@Advice.Return Collection parts,
150155
@ActiveRequestContext RequestContext reqCtx,
151156
@Advice.Thrown(readOnly = false) Throwable t) {
152-
if (t != null || parts == null || parts.isEmpty()) {
157+
CallDepthThreadLocalMap.decrementCallDepth(Collection.class);
158+
if (!proceed || t != null || parts == null || parts.isEmpty()) {
153159
return;
154160
}
155161
List<String> filenames = new ArrayList<>();

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,18 +107,24 @@ static void after(
107107

108108
@RequiresRequestContext(RequestContextSlot.APPSEC)
109109
public static class GetFilenamesAdvice {
110+
@Advice.OnMethodEnter(suppress = Throwable.class)
111+
static boolean before() {
112+
return CallDepthThreadLocalMap.incrementCallDepth(Collection.class) == 0;
113+
}
114+
110115
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
111116
static void after(
117+
@Advice.Enter boolean proceed,
112118
@Advice.Return Collection parts,
113119
@ActiveRequestContext RequestContext reqCtx,
114120
@Advice.Thrown(readOnly = false) Throwable t) {
115-
if (t != null || parts == null || parts.isEmpty()) {
121+
CallDepthThreadLocalMap.decrementCallDepth(Collection.class);
122+
if (!proceed || t != null || parts == null || parts.isEmpty()) {
116123
return;
117124
}
118125
Method getSubmittedFileName = null;
119126
try {
120-
getSubmittedFileName =
121-
parts.iterator().next().getClass().getMethod("getSubmittedFileName");
127+
getSubmittedFileName = parts.iterator().next().getClass().getMethod("getSubmittedFileName");
122128
} catch (Exception ignored) {
123129
}
124130
if (getSubmittedFileName == null) {

0 commit comments

Comments
 (0)