diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 0b2a28d954b..a599a987cbc 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -65,6 +65,8 @@ import java.util.function.Supplier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_COMBINED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_REPEATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS @@ -368,6 +370,14 @@ abstract class HttpServerTest extends WithHttpServer { false } + boolean testBodyFilenamesCalledOnce() { + false + } + + boolean testBodyFilenamesCalledOnceCombined() { + false + } + boolean testBodyFilenames() { false } @@ -476,6 +486,8 @@ abstract class HttpServerTest extends WithHttpServer { CREATED_IS("created_input_stream", 201, "created"), BODY_URLENCODED("body-urlencoded?ignore=pair", 200, '[a:[x]]'), BODY_MULTIPART("body-multipart?ignore=pair", 200, '[a:[x]]'), + BODY_MULTIPART_REPEATED("body-multipart-repeated", 200, "ok"), + BODY_MULTIPART_COMBINED("body-multipart-combined", 200, "ok"), BODY_JSON("body-json", 200, '{"a":"x"}'), BODY_XML("body-xml", 200, 'mytext'), REDIRECT("redirect", 302, "/redirected"), @@ -1646,6 +1658,54 @@ abstract class HttpServerTest extends WithHttpServer { response.close() } + def 'test instrumentation gateway file upload filenames called once'() { + setup: + assumeTrue(testBodyFilenamesCalledOnce()) + RequestBody fileBody = RequestBody.create(MediaType.parse('application/octet-stream'), 'file content') + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', 'evil.php', fileBody) + .build() + def httpRequest = request(BODY_MULTIPART_REPEATED, 'POST', body).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { + it.getTag('request.body.filenames') == "[evil.php]" + && it.getTag('_dd.appsec.filenames.cb.calls') == 1 + } + + cleanup: + response.close() + } + + def 'test instrumentation gateway file upload filenames called once via parameter map'() { + setup: + assumeTrue(testBodyFilenamesCalledOnceCombined()) + RequestBody fileBody = RequestBody.create(MediaType.parse('application/octet-stream'), 'file content') + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', 'evil.php', fileBody) + .build() + def httpRequest = request(BODY_MULTIPART_COMBINED, 'POST', body).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { + it.getTag('request.body.filenames') == "[evil.php]" + && it.getTag('_dd.appsec.filenames.cb.calls') == 1 + } + + cleanup: + response.close() + } + def 'test instrumentation gateway json request body'() { setup: assumeTrue(testBodyJson()) @@ -2581,6 +2641,7 @@ abstract class HttpServerTest extends WithHttpServer { boolean responseBodyTag Object responseBody List uploadedFilenames + int uploadedFilenamesCallCount = 0 } static final String stringOrEmpty(String string) { @@ -2754,6 +2815,8 @@ abstract class HttpServerTest extends WithHttpServer { rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String) Context context = rqCtxt.getData(RequestContextSlot.APPSEC) context.uploadedFilenames = filenames + context.uploadedFilenamesCallCount++ + rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount) Flow.ResultFlow.empty() } as BiFunction, Flow>) diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/MultipartHelper.java new file mode 100644 index 00000000000..28426fd9667 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/MultipartHelper.java @@ -0,0 +1,38 @@ +package datadog.trace.instrumentation.jetty8; + +public class MultipartHelper { + + private MultipartHelper() {} + + /** + * Extracts the filename value from a {@code Content-Disposition} header string. + * + *

Jetty 8 implements Servlet 3.0, where {@code Part.getSubmittedFileName()} does not exist. + * This method parses the filename from the header manually. + * + *

Examples of handled inputs: + * + *

+   *   form-data; name="file"; filename="photo.jpg"  → "photo.jpg"
+   *   form-data; name="file"; filename=photo.jpg    → "photo.jpg"
+   * 
+ * + * @return the filename, or {@code null} if not present or empty + */ + public static String filenameFromContentDisposition(String cd) { + if (cd == null) { + return null; + } + for (String tok : cd.split(";")) { + tok = tok.trim(); + if (tok.startsWith("filename=")) { + String name = tok.substring(9).trim(); + if (name.startsWith("\"") && name.endsWith("\"")) { + name = name.substring(1, name.length() - 1); + } + return name.isEmpty() ? null : name; + } + } + return null; + } +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/RequestGetPartsInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/RequestGetPartsInstrumentation.java index 104a3affa7c..515f988e5bd 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/RequestGetPartsInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/RequestGetPartsInstrumentation.java @@ -7,6 +7,8 @@ import com.google.auto.service.AutoService; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.advice.ActiveRequestContext; +import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.gateway.BlockResponseFunction; @@ -18,8 +20,12 @@ import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.function.BiFunction; import javax.servlet.ServletException; +import javax.servlet.http.Part; import net.bytebuddy.asm.Advice; import net.bytebuddy.asm.AsmVisitorWrapper; import net.bytebuddy.description.field.FieldDescription; @@ -27,6 +33,7 @@ import net.bytebuddy.description.method.MethodList; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.implementation.Implementation; +import net.bytebuddy.implementation.bytecode.assign.Assigner; import net.bytebuddy.jar.asm.ClassReader; import net.bytebuddy.jar.asm.ClassVisitor; import net.bytebuddy.jar.asm.ClassWriter; @@ -58,6 +65,7 @@ public String[] helperClassNames() { packageName + ".ParameterCollector", packageName + ".ParameterCollector$ParameterCollectorImpl", packageName + ".ParameterCollector$ParameterCollectorNoop", + packageName + ".MultipartHelper", }; } @@ -74,6 +82,8 @@ public void methodAdvice(MethodTransformer transformer) { .and(takesArgument(0, String.class)) .or(named("getParts").and(takesArguments(0))), getClass().getName() + "$GetPartsAdvice"); + transformer.applyAdvice( + named("getParts").and(takesArguments(0)), getClass().getName() + "$GetFilenamesAdvice"); } @Override @@ -194,6 +204,60 @@ static void muzzle(Request req) throws ServletException, IOException { } } + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before( + @Advice.FieldValue(value = "_multiPartInputStream", typing = Assigner.Typing.DYNAMIC) + final Object multiPartInputStream) { + // _multiPartInputStream is null only on the first getParts() call; subsequent calls + // return the cached multipart result without re-parsing, but we must not re-fire the WAF. + return multiPartInputStream == null; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.Return Collection parts, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + if (!proceed || t != null || parts == null || parts.isEmpty()) { + return; + } + // Jetty 8 implements Servlet 3.0; getSubmittedFileName does not exist. + // Parse filename from Content-Disposition header instead. + List filenames = new ArrayList<>(); + for (Part part : parts) { + String name = + MultipartHelper.filenameFromContentDisposition(part.getHeader("content-disposition")); + if (name != null) { + filenames.add(name); + } + } + if (filenames.isEmpty()) { + return; + } + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> callback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (callback == null) { + return; + } + Flow flow = callback.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 (t == null) { + t = new BlockingException("Blocked request (multipart file upload)"); + } + } + } + } + } + public static class GetPartsVisitorWrapper implements AsmVisitorWrapper { @Override public int mergeWriter(int flags) { diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/MultipartHelperTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/MultipartHelperTest.groovy new file mode 100644 index 00000000000..46d96465146 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/MultipartHelperTest.groovy @@ -0,0 +1,28 @@ +package datadog.trace.instrumentation.jetty8 + +import spock.lang.Specification +import spock.lang.Unroll + +class MultipartHelperTest extends Specification { + + @Unroll + def "filenameFromContentDisposition: #description"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == expected + + where: + description | cd | expected + 'null input' | null | null + 'no filename token' | 'form-data; name="upload"' | null + 'quoted filename' | 'form-data; name="upload"; filename="photo.jpg"' | 'photo.jpg' + 'unquoted filename' | 'form-data; name="upload"; filename=photo.jpg' | 'photo.jpg' + 'filename with spaces' | 'form-data; filename="my file.jpg"' | 'my file.jpg' + 'empty quoted filename' | 'form-data; name="f"; filename=""' | null + 'empty unquoted filename' | 'form-data; name="f"; filename=' | null + 'whitespace-only unquoted filename' | 'form-data; name="f"; filename= ' | null + 'filename first token' | 'filename="evil.php"' | 'evil.php' + 'extra spaces around semicolons' | 'form-data ; name="f" ; filename="a.txt" ' | 'a.txt' + 'filename with dots and dashes' | 'form-data; filename="my-file.v2.tar.gz"' | 'my-file.v2.tar.gz' + 'stops at first filename= token' | 'form-data; filename="first.jpg"; filename="second.jpg"' | 'first.jpg' + } +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java new file mode 100644 index 00000000000..714a6bd5339 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/MultipartHelper.java @@ -0,0 +1,32 @@ +package datadog.trace.instrumentation.jetty92; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import javax.servlet.http.Part; + +public class MultipartHelper { + + private MultipartHelper() {} + + /** + * Extracts non-null, non-empty filenames from a collection of multipart {@link Part}s using + * {@link Part#getSubmittedFileName()} (Servlet 3.1+). + * + * @return list of filenames; never {@code null}, may be empty + */ + public static List extractFilenames(Collection parts) { + if (parts == null || parts.isEmpty()) { + return Collections.emptyList(); + } + List filenames = new ArrayList<>(); + for (Part part : parts) { + String name = part.getSubmittedFileName(); + if (name != null && !name.isEmpty()) { + filenames.add(name); + } + } + return filenames; + } +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java index 0796aa32538..f43bfd88143 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java @@ -19,7 +19,10 @@ import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.util.Collection; +import java.util.List; import java.util.function.BiFunction; +import javax.servlet.http.Part; import net.bytebuddy.asm.Advice; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.MultiMap; @@ -38,6 +41,11 @@ public String instrumentedType() { return "org.eclipse.jetty.server.Request"; } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".MultipartHelper"}; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -48,6 +56,7 @@ public void methodAdvice(MethodTransformer transformer) { .and(takesArguments(1)) .and(takesArgument(0, named("org.eclipse.jetty.util.MultiMap"))), getClass().getName() + "$GetPartsAdvice"); + transformer.applyAdvice(named("getParts"), getClass().getName() + "$GetFilenamesAdvice"); } private static final Reference REQUEST_REFERENCE = @@ -135,4 +144,48 @@ static void after( } } } + + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before(@Advice.FieldValue("_contentParameters") final MultiMap map) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class); + return callDepth == 0 && map == null; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.Return Collection parts, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + CallDepthThreadLocalMap.decrementCallDepth(Collection.class); + if (!proceed || t != null || parts == null || parts.isEmpty()) { + return; + } + List filenames = MultipartHelper.extractFilenames(parts); + if (filenames.isEmpty()) { + return; + } + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> callback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (callback == null) { + return; + } + Flow flow = callback.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 (t == null) { + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/groovy/datadog/trace/instrumentation/jetty92/MultipartHelperTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/groovy/datadog/trace/instrumentation/jetty92/MultipartHelperTest.groovy new file mode 100644 index 00000000000..51bdb50c564 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/test/groovy/datadog/trace/instrumentation/jetty92/MultipartHelperTest.groovy @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.jetty92 + +import javax.servlet.http.Part +import spock.lang.Specification + +class MultipartHelperTest extends Specification { + + def "returns empty list for null collection"() { + expect: + MultipartHelper.extractFilenames(null) == [] + } + + def "returns empty list for empty collection"() { + expect: + MultipartHelper.extractFilenames([]) == [] + } + + def "returns empty list when all parts have null filename"() { + given: + def parts = [part(null), part(null)] + + expect: + MultipartHelper.extractFilenames(parts) == [] + } + + def "returns empty list when all parts have empty filename"() { + given: + def parts = [part(''), part('')] + + expect: + MultipartHelper.extractFilenames(parts) == [] + } + + def "extracts filename from single part"() { + given: + def parts = [part('photo.jpg')] + + expect: + MultipartHelper.extractFilenames(parts) == ['photo.jpg'] + } + + def "extracts filenames from multiple parts"() { + given: + def parts = [part('a.jpg'), part('b.png'), part('c.pdf')] + + expect: + MultipartHelper.extractFilenames(parts) == ['a.jpg', 'b.png', 'c.pdf'] + } + + def "skips parts with null or empty filename and keeps valid ones"() { + given: + def parts = [part(null), part('valid.txt'), part(''), part('other.zip')] + + expect: + MultipartHelper.extractFilenames(parts) == ['valid.txt', 'other.zip'] + } + + def "preserves filenames with spaces and special characters"() { + given: + def parts = [part('my file.tar.gz'), part('résumé.pdf')] + + expect: + MultipartHelper.extractFilenames(parts) == ['my file.tar.gz', 'résumé.pdf'] + } + + private Part part(String submittedFileName) { + Part p = Stub(Part) + p.getSubmittedFileName() >> submittedFileName + return p + } +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java index 3e1e2bf6d5c..a102c1e9981 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java @@ -18,6 +18,10 @@ import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; import org.eclipse.jetty.server.Request; @@ -42,6 +46,11 @@ public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( named("extractContentParameters").and(takesArguments(0)).or(named("getParts")), getClass().getName() + "$ExtractContentParametersAdvice"); + transformer.applyAdvice( + named("getParts").and(takesArguments(0)), getClass().getName() + "$GetFilenamesAdvice"); + transformer.applyAdvice( + named("getParts").and(takesArguments(1)), + getClass().getName() + "$GetFilenamesFromMultiPartAdvice"); } private static final Reference REQUEST_REFERENCE = @@ -99,4 +108,172 @@ static void after( } } } + + /** + * Fires the {@code requestFilesFilenames} event when the application calls public {@code + * getParts()}. Guards prevent double-firing: + * + *
    + *
  • {@code contentParameters != null}: set by {@code extractContentParameters()} (the {@code + * getParameterMap()} path); means filenames were already reported via {@code + * GetFilenamesFromMultiPartAdvice}. + *
  • {@code _multiParts != null} (Jetty 9.4+, read via reflection): set by the first {@code + * getParts()} call; means filenames were already reported. In Jetty 9.3 this field does not + * exist, so the reflection throws {@code NoSuchFieldException} and we treat it as null. + *
+ */ + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before( + @Advice.FieldValue("_contentParameters") final MultiMap contentParameters, + @Advice.This final Request request) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class); + if (callDepth != 0 || contentParameters != null) { + return false; + } + // Check the multipart cache field to detect repeated calls. + // Jetty 9.4+: _multiParts is set after the first getParts() call. + // Jetty 9.3.x: _multiPartInputStream is set instead (_multiParts doesn't exist). + // A non-null value means getParts() was already invoked and filenames were reported. + try { + java.lang.reflect.Field f = request.getClass().getDeclaredField("_multiParts"); + f.setAccessible(true); + if (f.get(request) != null) { + return false; + } + } catch (NoSuchFieldException e9_3) { + try { + java.lang.reflect.Field f = request.getClass().getDeclaredField("_multiPartInputStream"); + f.setAccessible(true); + if (f.get(request) != null) { + return false; + } + } catch (Exception ignored) { + } + } catch (Exception ignored) { + } + return true; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.Return Collection parts, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + CallDepthThreadLocalMap.decrementCallDepth(Collection.class); + if (!proceed || t != null || parts == null || parts.isEmpty()) { + return; + } + Method getSubmittedFileName = null; + try { + getSubmittedFileName = parts.iterator().next().getClass().getMethod("getSubmittedFileName"); + } catch (Exception ignored) { + } + if (getSubmittedFileName == null) { + return; + } + List filenames = new ArrayList<>(); + for (Object part : parts) { + try { + String name = (String) getSubmittedFileName.invoke(part); + if (name != null && !name.isEmpty()) { + filenames.add(name); + } + } catch (Exception ignored) { + } + } + if (filenames.isEmpty()) { + return; + } + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> callback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (callback == null) { + return; + } + Flow flow = callback.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 (t == null) { + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } + + /** + * Fires the {@code requestFilesFilenames} event when multipart content is parsed via the internal + * {@code getParts(MultiMap)} path triggered by {@code getParameter*()} / {@code + * getParameterMap()} — i.e. when the application never calls public {@code getParts()}. In Jetty + * 9.3+, {@code extractContentParameters()} assigns {@code _contentParameters} before calling this + * method, so {@code map == null} cannot be used as a "first parse" guard here; the call-depth + * guard prevents double-firing when {@code getParts()} internally delegates to this method. + */ + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesFromMultiPartAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before() { + return CallDepthThreadLocalMap.incrementCallDepth(Collection.class) == 0; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.Return Collection parts, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + CallDepthThreadLocalMap.decrementCallDepth(Collection.class); + if (!proceed || t != null || parts == null || parts.isEmpty()) { + return; + } + Method getSubmittedFileName = null; + try { + getSubmittedFileName = parts.iterator().next().getClass().getMethod("getSubmittedFileName"); + } catch (Exception ignored) { + } + if (getSubmittedFileName == null) { + return; + } + List filenames = new ArrayList<>(); + for (Object part : parts) { + try { + String name = (String) getSubmittedFileName.invoke(part); + if (name != null && !name.isEmpty()) { + filenames.add(name); + } + } catch (Exception ignored) { + } + } + if (filenames.isEmpty()) { + return; + } + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> callback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (callback == null) { + return; + } + Flow flow = callback.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 (t == null) { + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/test/groovy/datadog/trace/instrumentation/jetty10/Jetty10Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/test/groovy/datadog/trace/instrumentation/jetty10/Jetty10Test.groovy index 7dec61c223f..2726574ec83 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/test/groovy/datadog/trace/instrumentation/jetty10/Jetty10Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/test/groovy/datadog/trace/instrumentation/jetty10/Jetty10Test.groovy @@ -85,6 +85,21 @@ abstract class Jetty10Test extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + true + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + true + } + @Override boolean testSessionId() { true diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy index f4da48aaaf3..80afb31077a 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy @@ -67,6 +67,21 @@ abstract class Jetty11Test extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + true + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + true + } + @Override boolean testBlocking() { true diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/JettyAsyncHandlerTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/JettyAsyncHandlerTest.groovy index 38f5b1449ab..bd1e1bb9ecc 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/JettyAsyncHandlerTest.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/JettyAsyncHandlerTest.groovy @@ -25,6 +25,11 @@ class JettyAsyncHandlerTest extends Jetty11Test implements TestingGenericHttpNam false } + @Override + boolean testBodyFilenames() { + false + } + static class ContinuationTestHandler implements Handler { @Delegate private final Handler delegate diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy index 38eb20340c6..8a18ccbc652 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy @@ -85,6 +85,21 @@ abstract class Jetty9Test extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + true + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + true + } + @Override boolean testSessionId() { true diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy index 32a1b300c28..9bdc9e1e469 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy @@ -84,6 +84,21 @@ abstract class Jetty9Test extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + true + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + true + } + @Override boolean testSessionId() { true diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy index 32a1b300c28..9bdc9e1e469 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy @@ -84,6 +84,21 @@ abstract class Jetty9Test extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + true + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + true + } + @Override boolean testSessionId() { true diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy index 38eb20340c6..8a18ccbc652 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy @@ -85,6 +85,21 @@ abstract class Jetty9Test extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + true + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + true + } + @Override boolean testSessionId() { true diff --git a/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/TestServlet5.groovy b/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/TestServlet5.groovy index 51e7c974f6d..93060644456 100644 --- a/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/TestServlet5.groovy +++ b/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/TestServlet5.groovy @@ -11,6 +11,8 @@ import java.lang.reflect.Field import java.lang.reflect.Modifier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_COMBINED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_REPEATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS @@ -68,6 +70,22 @@ class TestServlet5 extends HttpServlet { resp.status = endpoint.status resp.writer.print(req.getHeader("x-forwarded-for")) break + case BODY_MULTIPART_REPEATED: + resp.status = endpoint.status + // Call getParts() 3 times to verify the filenames callback fires only once + req.getParts() + req.getParts() + req.getParts() + resp.writer.print(endpoint.body) + break + case BODY_MULTIPART_COMBINED: + resp.status = endpoint.status + // Call getParameterMap() first (exercises GetFilenamesFromMultiPartAdvice via extractContentParameters), + // then getParts() explicitly (GetFilenamesAdvice must not double-fire since map is already set) + req.parameterMap + req.getParts() + resp.writer.print(endpoint.body) + break case BODY_MULTIPART: case BODY_URLENCODED: resp.status = endpoint.status diff --git a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/TestServlet3.groovy b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/TestServlet3.groovy index 4b0b9df85d4..98a5983a36d 100644 --- a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/TestServlet3.groovy +++ b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/TestServlet3.groovy @@ -15,6 +15,8 @@ import java.lang.reflect.Modifier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_COMBINED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_REPEATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION @@ -95,6 +97,22 @@ class TestServlet3 { resp.status = endpoint.status resp.writer.print(endpoint.bodyForQuery(req.queryString)) break + case BODY_MULTIPART_REPEATED: + resp.status = endpoint.status + // Call getParts() 3 times to verify the filenames callback fires only once + req.getParts() + req.getParts() + req.getParts() + resp.writer.print(endpoint.body) + break + case BODY_MULTIPART_COMBINED: + resp.status = endpoint.status + // Call getParameterMap() first (exercises GetFilenamesFromMultiPartAdvice via extractContentParameters), + // then getParts() explicitly (GetFilenamesAdvice must not double-fire since map is already set) + req.parameterMap + req.getParts() + resp.writer.print(endpoint.body) + break case BODY_URLENCODED: case BODY_MULTIPART: resp.status = endpoint.status