Skip to content

Commit 30e4b8c

Browse files
committed
Fix GetFilenamesAdvice double-fire for all Jetty 9.3–11 versions
@advice.FieldValue(optional=true) is not supported in ByteBuddy 1.11.22. Replace it with @Advice.This + inline reflection to detect whether getParts() has already been called on this request: - Jetty 9.4+: checks _multiParts (set after first getParts() call) - Jetty 9.3.x: falls back to _multiPartInputStream (the cache field in 9.3.x, where _multiParts does not exist and _contentParameters is only set by the getParameterMap() → extractContentParameters() path, not by getParts()) Covers all forkedTest and latestDepForkedTest suites for Jetty 9.0–11.
1 parent db08e43 commit 30e4b8c

1 file changed

Lines changed: 36 additions & 11 deletions

File tree

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

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.util.List;
2525
import java.util.function.BiFunction;
2626
import net.bytebuddy.asm.Advice;
27-
import net.bytebuddy.implementation.bytecode.assign.Assigner;
2827
import org.eclipse.jetty.server.Request;
2928
import org.eclipse.jetty.util.MultiMap;
3029

@@ -112,23 +111,49 @@ static void after(
112111

113112
/**
114113
* Fires the {@code requestFilesFilenames} event when the application calls public {@code
115-
* getParts()}. The {@code _contentParameters == null} guard ensures the WAF is invoked only on
116-
* the first call — subsequent calls return the cached result without re-processing.
114+
* getParts()}. Guards prevent double-firing:
115+
*
116+
* <ul>
117+
* <li>{@code contentParameters != null}: set by {@code extractContentParameters()} (the {@code
118+
* getParameterMap()} path); means filenames were already reported via {@code
119+
* GetFilenamesFromMultiPartAdvice}.
120+
* <li>{@code _multiParts != null} (Jetty 9.4+, read via reflection): set by the first {@code
121+
* getParts()} call; means filenames were already reported. In Jetty 9.3 this field does not
122+
* exist, so the reflection throws {@code NoSuchFieldException} and we treat it as null.
123+
* </ul>
117124
*/
118125
@RequiresRequestContext(RequestContextSlot.APPSEC)
119126
public static class GetFilenamesAdvice {
120127
@Advice.OnMethodEnter(suppress = Throwable.class)
121128
static boolean before(
122129
@Advice.FieldValue("_contentParameters") final MultiMap<String> contentParameters,
123-
@Advice.FieldValue(value = "_multiParts", optional = true, typing = Assigner.Typing.DYNAMIC)
124-
final Object multiParts) {
130+
@Advice.This final Request request) {
125131
final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class);
126-
// contentParameters is set by extractContentParameters() (called from getParameterMap()),
127-
// so it being non-null means the request was already processed via that path.
128-
// multiParts is set by getParts(MultiMap) (Jetty 9.4+) after the first getParts() call,
129-
// so it being non-null means getParts() was already invoked and filenames were reported.
130-
// In Jetty 9.3, _multiParts does not exist (optional=true → null).
131-
return callDepth == 0 && contentParameters == null && multiParts == null;
132+
if (callDepth != 0 || contentParameters != null) {
133+
return false;
134+
}
135+
// Check the multipart cache field to detect repeated calls.
136+
// Jetty 9.4+: _multiParts is set after the first getParts() call.
137+
// Jetty 9.3.x: _multiPartInputStream is set instead (_multiParts doesn't exist).
138+
// A non-null value means getParts() was already invoked and filenames were reported.
139+
try {
140+
java.lang.reflect.Field f = request.getClass().getDeclaredField("_multiParts");
141+
f.setAccessible(true);
142+
if (f.get(request) != null) {
143+
return false;
144+
}
145+
} catch (NoSuchFieldException e9_3) {
146+
try {
147+
java.lang.reflect.Field f = request.getClass().getDeclaredField("_multiPartInputStream");
148+
f.setAccessible(true);
149+
if (f.get(request) != null) {
150+
return false;
151+
}
152+
} catch (Exception ignored) {
153+
}
154+
} catch (Exception ignored) {
155+
}
156+
return true;
132157
}
133158

134159
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)

0 commit comments

Comments
 (0)