diff --git a/dd-java-agent/appsec/appsec-test-fixtures/src/main/groovy/com/datadog/appsec/AppSecInactiveHttpServerTest.groovy b/dd-java-agent/appsec/appsec-test-fixtures/src/main/groovy/com/datadog/appsec/AppSecInactiveHttpServerTest.groovy index 140893ec525..37a0d56f7fa 100644 --- a/dd-java-agent/appsec/appsec-test-fixtures/src/main/groovy/com/datadog/appsec/AppSecInactiveHttpServerTest.groovy +++ b/dd-java-agent/appsec/appsec-test-fixtures/src/main/groovy/com/datadog/appsec/AppSecInactiveHttpServerTest.groovy @@ -139,8 +139,13 @@ abstract class AppSecInactiveHttpServerTest extends WithHttpServer { } } + protected boolean supportsMultipart() { + true + } + void multipart() { setup: + assumeTrue supportsMultipart() def body = new MultipartBody.Builder() .setType(MultipartBody.FORM) .addFormDataPart('a', 'x') 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/armeria/armeria-jetty-1.24/build.gradle b/dd-java-agent/instrumentation/armeria/armeria-jetty-1.24/build.gradle index 65cde281c07..2d8e7cf20a5 100644 --- a/dd-java-agent/instrumentation/armeria/armeria-jetty-1.24/build.gradle +++ b/dd-java-agent/instrumentation/armeria/armeria-jetty-1.24/build.gradle @@ -87,6 +87,8 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-9.0') testRuntimeOnly(project(':dd-java-agent:instrumentation:jetty:jetty-util-9.4.31')) testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3') + testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4') + testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-11.0') testRuntimeOnly project(':dd-java-agent:instrumentation:servlet:jakarta-servlet-5.0') testRuntimeOnly project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0') } diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/build.gradle new file mode 100644 index 00000000000..72cfb8495dd --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/build.gradle @@ -0,0 +1,28 @@ +muzzle { + pass { + group = 'org.eclipse.jetty' + module = 'jetty-server' + versions = '[11.0,12.0)' + assertInverse = true + javaVersion = 11 + } +} + +apply from: "$rootDir/gradle/java.gradle" + +dependencies { + compileOnly(group: 'org.eclipse.jetty', name: 'jetty-server', version: '11.0.26') { + exclude group: 'org.slf4j', module: 'slf4j-api' + } + testImplementation(group: 'org.eclipse.jetty.toolchain', name: 'jetty-jakarta-servlet-api', version: '5.0.1') +} + +tasks.withType(JavaCompile).configureEach { + configureCompiler(it, 11, JavaVersion.VERSION_1_8) +} + +tasks.withType(Test).configureEach { + javaLauncher = getJavaLauncherFor(11) +} + +// testing happens in the jetty-* modules diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/gradle.lockfile b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/gradle.lockfile new file mode 100644 index 00000000000..ddb930c8200 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/gradle.lockfile @@ -0,0 +1,127 @@ +# This is a Gradle generated file for dependency locking. +# Manual edits can break the build and are not advised. +# This file is expected to be part of source control. +cafe.cryptography:curve25519-elisabeth:0.1.0=testRuntimeClasspath +cafe.cryptography:ed25519-elisabeth:0.1.0=testRuntimeClasspath +ch.qos.logback:logback-classic:1.2.13=testCompileClasspath,testRuntimeClasspath +ch.qos.logback:logback-core:1.2.13=testCompileClasspath,testRuntimeClasspath +com.blogspot.mydailyjava:weak-lock-free:0.17=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +com.datadoghq.okhttp3:okhttp:3.12.15=testCompileClasspath,testRuntimeClasspath +com.datadoghq.okio:okio:1.17.6=testCompileClasspath,testRuntimeClasspath +com.datadoghq:dd-instrument-java:0.0.3=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +com.datadoghq:dd-javac-plugin-client:0.2.2=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +com.datadoghq:java-dogstatsd-client:4.4.3=testRuntimeClasspath +com.datadoghq:sketches-java:0.8.3=testRuntimeClasspath +com.github.javaparser:javaparser-core:3.25.6=codenarc +com.github.jnr:jffi:1.3.14=testRuntimeClasspath +com.github.jnr:jnr-a64asm:1.0.0=testRuntimeClasspath +com.github.jnr:jnr-constants:0.10.4=testRuntimeClasspath +com.github.jnr:jnr-enxio:0.32.19=testRuntimeClasspath +com.github.jnr:jnr-ffi:2.2.18=testRuntimeClasspath +com.github.jnr:jnr-posix:3.1.21=testRuntimeClasspath +com.github.jnr:jnr-unixsocket:0.38.24=testRuntimeClasspath +com.github.jnr:jnr-x86asm:1.0.2=testRuntimeClasspath +com.github.spotbugs:spotbugs-annotations:4.9.8=compileClasspath,spotbugs,testCompileClasspath,testRuntimeClasspath +com.github.spotbugs:spotbugs:4.9.8=spotbugs +com.github.stephenc.jcip:jcip-annotations:1.0-1=spotbugs +com.google.auto.service:auto-service-annotations:1.1.1=annotationProcessor,compileClasspath,testAnnotationProcessor,testCompileClasspath +com.google.auto.service:auto-service:1.1.1=annotationProcessor,testAnnotationProcessor +com.google.auto:auto-common:1.2.1=annotationProcessor,testAnnotationProcessor +com.google.code.findbugs:jsr305:3.0.2=annotationProcessor,compileClasspath,spotbugs,testAnnotationProcessor,testCompileClasspath,testRuntimeClasspath +com.google.code.gson:gson:2.13.2=spotbugs +com.google.errorprone:error_prone_annotations:2.18.0=annotationProcessor,testAnnotationProcessor +com.google.errorprone:error_prone_annotations:2.41.0=spotbugs +com.google.guava:failureaccess:1.0.1=annotationProcessor,testAnnotationProcessor +com.google.guava:guava:20.0=testCompileClasspath,testRuntimeClasspath +com.google.guava:guava:32.0.1-jre=annotationProcessor,testAnnotationProcessor +com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava=annotationProcessor,testAnnotationProcessor +com.google.j2objc:j2objc-annotations:2.8=annotationProcessor,testAnnotationProcessor +com.google.re2j:re2j:1.7=testRuntimeClasspath +com.squareup.moshi:moshi:1.11.0=testCompileClasspath,testRuntimeClasspath +com.squareup.okhttp3:logging-interceptor:3.12.12=testCompileClasspath,testRuntimeClasspath +com.squareup.okhttp3:okhttp:3.12.12=testCompileClasspath,testRuntimeClasspath +com.squareup.okio:okio:1.17.5=testCompileClasspath,testRuntimeClasspath +com.thoughtworks.qdox:qdox:1.12.1=codenarc +commons-fileupload:commons-fileupload:1.5=testCompileClasspath,testRuntimeClasspath +commons-io:commons-io:2.11.0=testCompileClasspath,testRuntimeClasspath +commons-io:commons-io:2.20.0=spotbugs +de.thetaphi:forbiddenapis:3.10=compileClasspath +io.leangen.geantyref:geantyref:1.3.16=testRuntimeClasspath +io.sqreen:libsqreen:17.3.0=testRuntimeClasspath +javax.servlet:javax.servlet-api:3.1.0=testCompileClasspath,testRuntimeClasspath +jaxen:jaxen:2.0.0=spotbugs +junit:junit:4.13.2=testRuntimeClasspath +net.bytebuddy:byte-buddy-agent:1.18.3=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +net.bytebuddy:byte-buddy:1.18.3=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +net.java.dev.jna:jna-platform:5.8.0=testRuntimeClasspath +net.java.dev.jna:jna:5.8.0=testRuntimeClasspath +net.sf.saxon:Saxon-HE:12.9=spotbugs +org.apache.ant:ant-antlr:1.10.14=codenarc +org.apache.ant:ant-junit:1.10.14=codenarc +org.apache.bcel:bcel:6.11.0=spotbugs +org.apache.commons:commons-lang3:3.19.0=spotbugs +org.apache.commons:commons-text:1.14.0=spotbugs +org.apache.logging.log4j:log4j-api:2.25.2=spotbugs +org.apache.logging.log4j:log4j-core:2.25.2=spotbugs +org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath +org.checkerframework:checker-qual:3.33.0=annotationProcessor,testAnnotationProcessor +org.codehaus.groovy:groovy-ant:3.0.23=codenarc +org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc +org.codehaus.groovy:groovy-groovydoc:3.0.23=codenarc +org.codehaus.groovy:groovy-json:3.0.23=codenarc +org.codehaus.groovy:groovy-json:3.0.25=testCompileClasspath,testRuntimeClasspath +org.codehaus.groovy:groovy-templates:3.0.23=codenarc +org.codehaus.groovy:groovy-xml:3.0.23=codenarc +org.codehaus.groovy:groovy:3.0.23=codenarc +org.codehaus.groovy:groovy:3.0.25=testCompileClasspath,testRuntimeClasspath +org.codenarc:CodeNarc:3.7.0=codenarc +org.dom4j:dom4j:2.2.0=spotbugs +org.eclipse.jetty.toolchain:jetty-jakarta-servlet-api:5.0.1=compileClasspath,testCompileClasspath,testRuntimeClasspath +org.eclipse.jetty:jetty-http:11.0.0=compileClasspath +org.eclipse.jetty:jetty-io:11.0.0=compileClasspath +org.eclipse.jetty:jetty-server:11.0.0=compileClasspath +org.eclipse.jetty:jetty-util:11.0.0=compileClasspath +org.gmetrics:GMetrics:2.1.0=codenarc +org.hamcrest:hamcrest-core:1.3=testRuntimeClasspath +org.hamcrest:hamcrest:3.0=testCompileClasspath,testRuntimeClasspath +org.jctools:jctools-core-jdk11:4.0.6=testRuntimeClasspath +org.jctools:jctools-core:4.0.6=testRuntimeClasspath +org.junit.jupiter:junit-jupiter-api:5.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.jupiter:junit-jupiter-engine:5.14.1=testRuntimeClasspath +org.junit.jupiter:junit-jupiter-params:5.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.jupiter:junit-jupiter:5.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.platform:junit-platform-commons:1.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.platform:junit-platform-engine:1.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.platform:junit-platform-launcher:1.14.1=testRuntimeClasspath +org.junit.platform:junit-platform-runner:1.14.1=testRuntimeClasspath +org.junit.platform:junit-platform-suite-api:1.14.1=testRuntimeClasspath +org.junit.platform:junit-platform-suite-commons:1.14.1=testRuntimeClasspath +org.junit:junit-bom:5.14.0=spotbugs +org.junit:junit-bom:5.14.1=testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=testRuntimeClasspath +org.objenesis:objenesis:3.3=testCompileClasspath,testRuntimeClasspath +org.opentest4j:opentest4j:1.3.0=testCompileClasspath,testRuntimeClasspath +org.ow2.asm:asm-analysis:9.7.1=testRuntimeClasspath +org.ow2.asm:asm-analysis:9.9=spotbugs +org.ow2.asm:asm-commons:9.9=spotbugs +org.ow2.asm:asm-commons:9.9.1=testRuntimeClasspath +org.ow2.asm:asm-tree:9.9=spotbugs +org.ow2.asm:asm-tree:9.9.1=testRuntimeClasspath +org.ow2.asm:asm-util:9.7.1=testRuntimeClasspath +org.ow2.asm:asm-util:9.9=spotbugs +org.ow2.asm:asm:9.9=spotbugs +org.ow2.asm:asm:9.9.1=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +org.slf4j:jcl-over-slf4j:1.7.30=testCompileClasspath,testRuntimeClasspath +org.slf4j:jul-to-slf4j:1.7.30=testCompileClasspath,testRuntimeClasspath +org.slf4j:log4j-over-slf4j:1.7.30=testCompileClasspath,testRuntimeClasspath +org.slf4j:slf4j-api:1.7.30=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath +org.slf4j:slf4j-api:1.7.32=testCompileClasspath,testRuntimeClasspath +org.slf4j:slf4j-api:2.0.17=spotbugs,spotbugsSlf4j +org.slf4j:slf4j-simple:2.0.17=spotbugsSlf4j +org.snakeyaml:snakeyaml-engine:2.9=buildTimeInstrumentationPlugin,muzzleTooling,runtimeClasspath,testRuntimeClasspath +org.spockframework:spock-bom:2.4-groovy-3.0=testCompileClasspath,testRuntimeClasspath +org.spockframework:spock-core:2.4-groovy-3.0=testCompileClasspath,testRuntimeClasspath +org.tabletest:tabletest-junit:1.2.1=testCompileClasspath,testRuntimeClasspath +org.tabletest:tabletest-parser:1.2.0=testCompileClasspath,testRuntimeClasspath +org.xmlresolver:xmlresolver:5.3.3=spotbugs +empty=spotbugsPlugins diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java new file mode 100644 index 00000000000..dc053ed19b8 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/MultipartHelper.java @@ -0,0 +1,32 @@ +package datadog.trace.instrumentation.jetty11; + +import jakarta.servlet.http.Part; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +public class MultipartHelper { + + private MultipartHelper() {} + + /** + * Extracts non-null, non-empty filenames from a collection of multipart {@link Part}s using + * {@link Part#getSubmittedFileName()} (Servlet 5.0+, Jetty 11.x). + * + * @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-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java new file mode 100644 index 00000000000..23af3731435 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/main/java/datadog/trace/instrumentation/jetty11/RequestExtractContentParametersInstrumentation.java @@ -0,0 +1,233 @@ +package datadog.trace.instrumentation.jetty11; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.api.gateway.Events.EVENTS; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +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.agent.tooling.muzzle.Reference; +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.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import jakarta.servlet.http.Part; +import java.util.Collection; +import java.util.List; +import java.util.function.BiFunction; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.MultiMap; + +@AutoService(InstrumenterModule.class) +public class RequestExtractContentParametersInstrumentation extends InstrumenterModule.AppSec + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + private static final String MULTI_MAP_INTERNAL_NAME = "Lorg/eclipse/jetty/util/MultiMap;"; + + public RequestExtractContentParametersInstrumentation() { + super("jetty"); + } + + @Override + 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( + 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"); + } + + // Discriminates Jetty 11.0.x ([11.0, 12.0)): + // - _contentParameters + extractContentParameters(void) exist in 11.x (excludes Jetty 12 + // where org.eclipse.jetty.server.Request was removed) + // - _dispatcherType: Ljakarta/servlet/DispatcherType; in the Request bytecode (excludes + // Jetty 9.4–10.x where the field descriptor is Ljavax/servlet/DispatcherType;). This check + // is tied to Request.class bytecode, NOT just classpath presence, so it works even when both + // javax.servlet and jakarta.servlet are on the classpath simultaneously. + // NOTE: _multiParts changes type at 11.0.10 (MultiPartFormInputStream → MultiParts); both + // are handled transparently because GetFilenamesAdvice reads it with typing=DYNAMIC. + private static final Reference REQUEST_REFERENCE = + new Reference.Builder("org.eclipse.jetty.server.Request") + .withMethod(new String[0], 0, "extractContentParameters", "V") + .withField(new String[0], 0, "_contentParameters", MULTI_MAP_INTERNAL_NAME) + .withField(new String[0], 0, "_dispatcherType", "Ljakarta/servlet/DispatcherType;") + .build(); + + @Override + public Reference[] additionalMuzzleReferences() { + return new Reference[] {REQUEST_REFERENCE}; + } + + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class ExtractContentParametersAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before(@Advice.FieldValue("_contentParameters") final MultiMap map) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Request.class); + return callDepth == 0 && map == null; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.FieldValue("_contentParameters") final MultiMap map, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + CallDepthThreadLocalMap.decrementCallDepth(Request.class); + if (!proceed) { + return; + } + if (map == null || map.isEmpty()) { + return; + } + + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction> callback = + cbp.getCallback(EVENTS.requestBodyProcessed()); + if (callback == null) { + return; + } + + Flow flow = callback.apply(reqCtx, map); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); + if (blockResponseFunction != null) { + blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (t == null) { + t = new BlockingException("Blocked request (for Request/extractContentParameters)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } + + /** + * 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}: set by the first {@code getParts()} call in Jetty 11.0.x; + * means filenames were already reported. + *
+ */ + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before( + @Advice.FieldValue("_contentParameters") final MultiMap contentParameters, + @Advice.FieldValue(value = "_multiParts", typing = Assigner.Typing.DYNAMIC) + final Object multiParts) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class); + return callDepth == 0 && contentParameters == null && multiParts == 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(); + } + } + } + } + } + + /** + * 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()}. + */ + @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; + } + 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-11.0/src/test/groovy/datadog/trace/instrumentation/jetty11/MultipartHelperTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/groovy/datadog/trace/instrumentation/jetty11/MultipartHelperTest.groovy new file mode 100644 index 00000000000..2434cc72fae --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-11.0/src/test/groovy/datadog/trace/instrumentation/jetty11/MultipartHelperTest.groovy @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.jetty11 + +import jakarta.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-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/ParameterCollector.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/ParameterCollector.java deleted file mode 100644 index 73e7038238e..00000000000 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/ParameterCollector.java +++ /dev/null @@ -1,59 +0,0 @@ -package datadog.trace.instrumentation.jetty8; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -public interface ParameterCollector { - boolean isEmpty(); - - void put(String key, String value); - - Map> getMap(); - - class ParameterCollectorNoop implements ParameterCollector { - public static final ParameterCollector INSTANCE = new ParameterCollectorNoop(); - - private ParameterCollectorNoop() {} - - @Override - public boolean isEmpty() { - return true; - } - - @Override - public void put(String key, String value) {} - - @Override - public Map> getMap() { - return Collections.emptyMap(); - } - } - - class ParameterCollectorImpl implements ParameterCollector { - public Map> map; - - public boolean isEmpty() { - return map == null; - } - - public void put(String key, String value) { - if (map == null) { - map = new HashMap<>(); - } - List strings = map.get(key); - if (strings == null) { - strings = new ArrayList<>(); - map.put(key, strings); - } - strings.add(value); - } - - @Override - public Map> getMap() { - return map; - } - } -} diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java new file mode 100644 index 00000000000..0d52dad53a0 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java @@ -0,0 +1,147 @@ +package datadog.trace.instrumentation.jetty8; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import javax.servlet.http.Part; + +/** + * Helper for extracting filenames and form-field values from Servlet 3.0 {@link Part} objects. + * + *

{@code Part.getSubmittedFileName()} was added in Servlet 3.1 (Jetty 9.1+); for Jetty 8.x we + * must parse the {@code Content-Disposition} header manually. + */ +public class PartHelper { + + private PartHelper() {} + + /** + * Returns filenames found in {@code parts} by parsing each part's {@code Content-Disposition} + * header for a {@code filename=} parameter. + */ + public static List extractFilenames(Collection parts) { + if (parts == null || parts.isEmpty()) { + return Collections.emptyList(); + } + List filenames = new ArrayList<>(); + for (Object obj : parts) { + String filename = filenameFromPart((Part) obj); + if (filename != null && !filename.isEmpty()) { + filenames.add(filename); + } + } + return filenames; + } + + /** + * Returns a name→values map of form-field parts (those without a {@code filename=} parameter). + * File-upload parts are skipped to avoid reading potentially large content. + */ + public static Map> extractFormFields(Collection parts) { + if (parts == null || parts.isEmpty()) { + return Collections.emptyMap(); + } + Map> result = new LinkedHashMap<>(); + for (Object obj : parts) { + Part part = (Part) obj; + if (filenameFromPart(part) != null) { + continue; // file-upload part — skip + } + String name = part.getName(); + if (name == null) { + continue; + } + String value = readPartContent(part); + if (value == null) { + continue; + } + List values = result.get(name); + if (values == null) { + values = new ArrayList<>(); + result.put(name, values); + } + values.add(value); + } + return result; + } + + /** + * Extracts the {@code filename} value from a {@code Content-Disposition} header, or {@code null} + * if the part has no filename (i.e. it is a plain form field). + * + *

Uses a quote-aware parser so that semicolons inside a quoted filename (e.g. {@code + * filename="shell;evil.php"}) are not mistaken for parameter separators. + */ + static String filenameFromPart(Part part) { + String cd = part.getHeader("Content-Disposition"); + if (cd == null) { + return null; + } + int len = cd.length(); + int i = 0; + while (i < len) { + // Skip separators between parameters + while (i < len && (cd.charAt(i) == ';' || cd.charAt(i) == ' ' || cd.charAt(i) == '\t')) { + i++; + } + if (i >= len) break; + // Read parameter name (up to '=' or ';') + int nameStart = i; + while (i < len && cd.charAt(i) != '=' && cd.charAt(i) != ';') { + i++; + } + boolean isFilename = "filename".equalsIgnoreCase(cd.substring(nameStart, i).trim()); + if (i >= len || cd.charAt(i) == ';') { + // Value-less token (e.g. "form-data") — skip + continue; + } + i++; // skip '=' + String value; + if (i < len && cd.charAt(i) == '"') { + i++; // skip opening quote + StringBuilder sb = new StringBuilder(); + while (i < len && cd.charAt(i) != '"') { + if (cd.charAt(i) == '\\' && i + 1 < len) { + i++; // consume escape backslash, add next char literally + } + sb.append(cd.charAt(i++)); + } + if (i < len) i++; // skip closing quote + value = sb.toString(); + } else { + int valueStart = i; + while (i < len && cd.charAt(i) != ';') { + i++; + } + value = cd.substring(valueStart, i).trim(); + } + if (isFilename) { + // Return empty string (not null) so callers can distinguish "filename present but empty" + // from "no filename parameter". extractFormFields() uses != null to skip file parts, + // so empty string correctly prevents buffering a file-upload body with filename="". + return value; + } + } + return null; + } + + private static String readPartContent(Part part) { + try (InputStream is = part.getInputStream()) { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int read; + while ((read = is.read(buf)) != -1) { + baos.write(buf, 0, read); + } + return baos.toString("UTF-8"); + } catch (IOException e) { + 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..a72f17efa97 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; @@ -14,35 +16,28 @@ 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.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.io.IOException; import java.io.InputStream; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; 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; -import net.bytebuddy.description.field.FieldList; -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; import net.bytebuddy.jar.asm.FieldVisitor; import net.bytebuddy.jar.asm.MethodVisitor; import net.bytebuddy.jar.asm.Opcodes; -import net.bytebuddy.jar.asm.Type; import net.bytebuddy.matcher.ElementMatcher; -import net.bytebuddy.pool.TypePool; -import org.eclipse.jetty.server.Request; @AutoService(InstrumenterModule.class) public class RequestGetPartsInstrumentation extends InstrumenterModule.AppSec - implements Instrumenter.ForSingleType, - Instrumenter.HasTypeAdvice, - Instrumenter.HasMethodAdvice { + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { public RequestGetPartsInstrumentation() { super("jetty"); } @@ -54,26 +49,16 @@ public String instrumentedType() { @Override public String[] helperClassNames() { - return new String[] { - packageName + ".ParameterCollector", - packageName + ".ParameterCollector$ParameterCollectorImpl", - packageName + ".ParameterCollector$ParameterCollectorNoop", - }; - } - - @Override - public void typeAdvice(TypeTransformer transformer) { - transformer.applyAdvice(new GetPartsVisitorWrapper()); + return new String[] {packageName + ".PartHelper"}; } @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - named("getPart") - .and(takesArguments(1)) - .and(takesArgument(0, String.class)) - .or(named("getParts").and(takesArguments(0))), - getClass().getName() + "$GetPartsAdvice"); + named("getParts").and(takesArguments(0)), getClass().getName() + "$GetFilenamesAdvice"); + transformer.applyAdvice( + named("getPart").and(takesArguments(1)).and(takesArgument(0, String.class)), + getClass().getName() + "$GetPartAdvice"); } @Override @@ -142,134 +127,155 @@ public void visitMethodInsn( } } - public static class GetPartsAdvice { + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - static void before( - @Advice.Local("collector") ParameterCollector collector, - @Advice.Local("reqCtx") RequestContext reqCtx) { - AgentSpan agentSpan = AgentTracer.activeSpan(); - if (agentSpan != null) { - RequestContext requestContext = agentSpan.getRequestContext(); - if (requestContext != null && requestContext.getData(RequestContextSlot.APPSEC) != null) { - reqCtx = requestContext; - collector = new ParameterCollector.ParameterCollectorImpl(); - return; - } - } - // this variable is used in the custom instrumentation below - collector = ParameterCollector.ParameterCollectorNoop.INSTANCE; + static boolean before( + @Advice.FieldValue(value = "_multiPartInputStream", typing = Assigner.Typing.DYNAMIC) + final Object multiPartInputStream) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class); + // _multiPartInputStream is null before the first parse; non-null on cached repeat calls. + return callDepth == 0 && multiPartInputStream == null; } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after( - @Advice.Local("collector") ParameterCollector collector, - @Advice.Local("reqCtx") RequestContext reqCtx, + @Advice.Enter boolean proceed, + @Advice.Return Collection parts, + @ActiveRequestContext RequestContext reqCtx, @Advice.Thrown(readOnly = false) Throwable t) { - if (t != null || reqCtx == null || collector.isEmpty()) { + CallDepthThreadLocalMap.decrementCallDepth(Collection.class); + if (!proceed || t != null || parts == null || parts.isEmpty()) { return; } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - BiFunction> callback = - cbp.getCallback(EVENTS.requestBodyProcessed()); - if (callback == null) { + // Fire requestBodyProcessed with form-field name→values extracted from parts + Map> formFields = PartHelper.extractFormFields(parts); + if (!formFields.isEmpty()) { + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction> bodyCallback = + cbp.getCallback(EVENTS.requestBodyProcessed()); + if (bodyCallback != null) { + Flow flow = bodyCallback.apply(reqCtx, formFields); + 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 form fields)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } + + if (t != null) { return; } - Flow flow = callback.apply(reqCtx, collector.getMap()); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (t == null) { - t = new BlockingException("Blocked request (for Request/parsePart(s))"); + + // Fire requestFilesFilenames with file-upload filenames extracted from parts + List filenames = PartHelper.extractFilenames(parts); + if (!filenames.isEmpty()) { + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (filenamesCallback != null) { + Flow flow = filenamesCallback.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(); + } + } } } } } - - static void muzzle(Request req) throws ServletException, IOException { - req.getParts(); - } } - public static class GetPartsVisitorWrapper implements AsmVisitorWrapper { - @Override - public int mergeWriter(int flags) { - return flags | ClassWriter.COMPUTE_MAXS; - } - - @Override - public int mergeReader(int flags) { - return flags; + /** + * Fires AppSec events for a single-part upload via {@code getPart(String)}, which in Jetty 8.x + * does NOT delegate to {@code getParts()} — it calls {@code + * _multiPartInputStream.getPart(String)} directly. Without this advice, single-file uploads that + * never call the public {@code getParts()} would be missed. + */ + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetPartAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before() { + return CallDepthThreadLocalMap.incrementCallDepth(Part.class) == 0; } - @Override - public ClassVisitor wrap( - TypeDescription instrumentedType, - ClassVisitor classVisitor, - Implementation.Context implementationContext, - TypePool typePool, - FieldList fields, - MethodList methods, - int writerFlags, - int readerFlags) { - return new RequestClassVisitor(Opcodes.ASM8, classVisitor); - } - } + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.Return Part part, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + CallDepthThreadLocalMap.decrementCallDepth(Part.class); + if (!proceed || t != null || part == null) { + return; + } - public static class RequestClassVisitor extends ClassVisitor { - public RequestClassVisitor(int api, ClassVisitor cv) { - super(api, cv); - } + Collection parts = Collections.singletonList(part); - @Override - public MethodVisitor visitMethod( - int access, String name, String descriptor, String signature, String[] exceptions) { - MethodVisitor superMv = super.visitMethod(access, name, descriptor, signature, exceptions); - if ("getPart".equals(name) - && "(Ljava/lang/String;)Ljavax/servlet/http/Part;".equals(descriptor) - || "getParts".equals(name) && "()Ljava/util/Collection;".equals(descriptor)) { - return new GetPartsMethodVisitor(api, superMv, descriptor.startsWith("()") ? 1 : 2); - } else { - return superMv; + // Fire requestBodyProcessed with form-field name→value (if not a file upload) + Map> formFields = PartHelper.extractFormFields(parts); + if (!formFields.isEmpty()) { + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction> bodyCallback = + cbp.getCallback(EVENTS.requestBodyProcessed()); + if (bodyCallback != null) { + Flow flow = bodyCallback.apply(reqCtx, formFields); + 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 form field)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } } - } - } - public static class GetPartsMethodVisitor extends MethodVisitor { - private final int collectedParamsVar; - - public GetPartsMethodVisitor(int api, MethodVisitor superMv, int collectedParamsVar) { - super(api, superMv); - this.collectedParamsVar = collectedParamsVar; - } + if (t != null) { + return; + } - @Override - public void visitMethodInsn( - int opcode, String owner, String name, String descriptor, boolean isInterface) { - if (opcode == Opcodes.INVOKEVIRTUAL - && owner.equals("org/eclipse/jetty/util/MultiMap") - && name.equals("add") - && descriptor.equals("(Ljava/lang/String;Ljava/lang/Object;)V")) { - super.visitVarInsn(Opcodes.ALOAD, collectedParamsVar); - // stack: ..., key, value, collParams - super.visitInsn(Opcodes.DUP_X2); - // stack: ..., collParams, key, value, collParams - super.visitInsn(Opcodes.POP); - // stack: ..., collParams, key, value - super.visitInsn(Opcodes.DUP2_X1); - // stack: ..., key, value, collParams, key, value - super.visitMethodInsn( - Opcodes.INVOKEINTERFACE, - Type.getInternalName(ParameterCollector.class), - "put", - "(Ljava/lang/String;Ljava/lang/String;)V", - true); - // original stack + // Fire requestFilesFilenames with file-upload filename (if a file upload) + List filenames = PartHelper.extractFilenames(parts); + if (!filenames.isEmpty()) { + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (filenamesCallback != null) { + Flow flow = filenamesCallback.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(); + } + } + } + } } - super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); } } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/PartHelperTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/PartHelperTest.groovy new file mode 100644 index 00000000000..8436af5ba6a --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/PartHelperTest.groovy @@ -0,0 +1,218 @@ +package datadog.trace.instrumentation.jetty8 + +import javax.servlet.http.Part +import spock.lang.Specification + +class PartHelperTest extends Specification { + + // ── extractFilenames ──────────────────────────────────────────────────────── + + def "extractFilenames returns empty list for null collection"() { + expect: + PartHelper.extractFilenames(null) == [] + } + + def "extractFilenames returns empty list for empty collection"() { + expect: + PartHelper.extractFilenames([]) == [] + } + + def "extractFilenames returns empty list when no parts have a filename"() { + given: + def parts = [field('a', 'x'), field('b', 'y')] + + expect: + PartHelper.extractFilenames(parts) == [] + } + + def "extractFilenames extracts filename from a single file part"() { + given: + def parts = [filePart('photo.jpg')] + + expect: + PartHelper.extractFilenames(parts) == ['photo.jpg'] + } + + def "extractFilenames extracts filenames from multiple file parts"() { + given: + def parts = [filePart('a.jpg'), filePart('b.png'), filePart('c.pdf')] + + expect: + PartHelper.extractFilenames(parts) == ['a.jpg', 'b.png', 'c.pdf'] + } + + def "extractFilenames skips form-field parts and keeps file parts"() { + given: + def parts = [field('x', 'v'), filePart('upload.zip'), field('y', 'w')] + + expect: + PartHelper.extractFilenames(parts) == ['upload.zip'] + } + + def "extractFilenames preserves filenames with spaces and special characters"() { + given: + def parts = [filePart('my file.tar.gz'), filePart('résumé.pdf')] + + expect: + PartHelper.extractFilenames(parts) == ['my file.tar.gz', 'résumé.pdf'] + } + + // ── filenameFromPart ──────────────────────────────────────────────────────── + + def "filenameFromPart returns null when Content-Disposition header is absent"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> null } + + expect: + PartHelper.filenameFromPart(p) == null + } + + def "filenameFromPart returns null when there is no filename parameter"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="field"' } + + expect: + PartHelper.filenameFromPart(p) == null + } + + def "filenameFromPart extracts unquoted filename"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename=photo.jpg' } + + expect: + PartHelper.filenameFromPart(p) == 'photo.jpg' + } + + def "filenameFromPart strips quotes from filename"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename="photo.jpg"' } + + expect: + PartHelper.filenameFromPart(p) == 'photo.jpg' + } + + def "filenameFromPart returns empty string for empty quoted filename"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename=""' } + + expect: + PartHelper.filenameFromPart(p) == '' + } + + def "filenameFromPart returns empty string for empty unquoted filename"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename=' } + + expect: + PartHelper.filenameFromPart(p) == '' + } + + def "extractFormFields skips part with empty filename"() { + given: + def parts = [emptyFilenamePart('field')] + + expect: + PartHelper.extractFormFields(parts) == [:] + } + + def "extractFilenames skips empty filename"() { + given: + def parts = [emptyFilenamePart('field')] + + expect: + PartHelper.extractFilenames(parts) == [] + } + + def "filenameFromPart preserves semicolons inside a quoted filename"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename="shell;evil.php"' } + + expect: + PartHelper.filenameFromPart(p) == 'shell;evil.php' + } + + def "filenameFromPart handles escaped quote inside filename"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename="file\\"name.txt"' } + + expect: + PartHelper.filenameFromPart(p) == 'file"name.txt' + } + + def "filenameFromPart handles filename before other parameters"() { + given: + Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; filename="first.txt"; name="file"' } + + expect: + PartHelper.filenameFromPart(p) == 'first.txt' + } + + // ── extractFormFields ─────────────────────────────────────────────────────── + + def "extractFormFields returns empty map for null collection"() { + expect: + PartHelper.extractFormFields(null) == [:] + } + + def "extractFormFields returns empty map for empty collection"() { + expect: + PartHelper.extractFormFields([]) == [:] + } + + def "extractFormFields skips file-upload parts"() { + given: + def parts = [filePart('evil.php')] + + expect: + PartHelper.extractFormFields(parts) == [:] + } + + def "extractFormFields extracts single form field"() { + given: + def parts = [field('username', 'alice')] + + expect: + PartHelper.extractFormFields(parts) == [username: ['alice']] + } + + def "extractFormFields groups multiple values under the same name"() { + given: + def parts = [field('tag', 'foo'), field('tag', 'bar')] + + expect: + PartHelper.extractFormFields(parts) == [tag: ['foo', 'bar']] + } + + def "extractFormFields mixes fields and skips files"() { + given: + def parts = [field('a', 'x'), filePart('upload.bin'), field('b', 'y')] + + expect: + PartHelper.extractFormFields(parts) == [a: ['x'], b: ['y']] + } + + // ── helpers ───────────────────────────────────────────────────────────────── + + /** Creates a stub Part that looks like a plain form field (no filename). */ + private Part field(String name, String value) { + Part p = Stub(Part) + p.getHeader('Content-Disposition') >> "form-data; name=\"${name}\"" + p.getName() >> name + p.getInputStream() >> new ByteArrayInputStream(value.getBytes('UTF-8')) + return p + } + + /** Creates a stub Part that looks like a file upload with the given filename. */ + private Part filePart(String filename) { + Part p = Stub(Part) + p.getHeader('Content-Disposition') >> "form-data; name=\"file\"; filename=\"${filename}\"" + return p + } + + /** Creates a stub Part that has filename="" — a file input submitted with no file chosen. */ + private Part emptyFilenamePart(String name) { + Part p = Stub(Part) + p.getHeader('Content-Disposition') >> "form-data; name=\"${name}\"; filename=\"\"" + return p + } +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/build.gradle index 69bad38c12b..f151c5573aa 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/build.gradle @@ -2,7 +2,7 @@ muzzle { pass { group = 'org.eclipse.jetty' module = 'jetty-server' - versions = '[9.3,12)' + versions = '[9.3,9.4.10)' assertInverse = true } } diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java new file mode 100644 index 00000000000..e54f72b8ff7 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/MultipartHelper.java @@ -0,0 +1,32 @@ +package datadog.trace.instrumentation.jetty93; + +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+, Jetty 9.3.x). + * + * @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.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..f9912787b33 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,8 +18,12 @@ 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 net.bytebuddy.implementation.bytecode.assign.Assigner; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.MultiMap; @@ -37,17 +41,36 @@ 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( 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"); } + // Discriminates Jetty 9.3.x–9.4.9.x ([9.3, 9.4.10)): + // - _contentParameters + extractContentParameters(void) exist from 9.3+ (excludes 9.2) + // - _multiPartInputStream exists in 9.3.x and early 9.4.x (< 9.4.10); replaced by _multiParts + // in 9.4.10 (covered by jetty-appsec-9.4) private static final Reference REQUEST_REFERENCE = new Reference.Builder("org.eclipse.jetty.server.Request") .withMethod(new String[0], 0, "extractContentParameters", "V") .withField(new String[0], 0, "_contentParameters", MULTI_MAP_INTERNAL_NAME) + .withField( + new String[0], + 0, + "_multiPartInputStream", + "Lorg/eclipse/jetty/util/MultiPartInputStreamParser;") .build(); @Override @@ -99,4 +122,114 @@ 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 _multiPartInputStream != null}: set by the first {@code getParts()} call in Jetty + * 9.3.x; means filenames were already reported. + *
+ */ + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before( + @Advice.FieldValue("_contentParameters") final MultiMap contentParameters, + @Advice.FieldValue(value = "_multiPartInputStream", typing = Assigner.Typing.DYNAMIC) + final Object multiPartInputStream) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class); + return callDepth == 0 && contentParameters == null && 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) { + 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(); + } + } + } + } + } + + /** + * 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; + } + 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.3/src/test/groovy/datadog/trace/instrumentation/jetty93/MultipartHelperTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/groovy/datadog/trace/instrumentation/jetty93/MultipartHelperTest.groovy new file mode 100644 index 00000000000..31489cb96f9 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/test/groovy/datadog/trace/instrumentation/jetty93/MultipartHelperTest.groovy @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.jetty93 + +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.4/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/build.gradle new file mode 100644 index 00000000000..012ec0c3485 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/build.gradle @@ -0,0 +1,31 @@ +muzzle { + pass { + name = '9_series' + group = 'org.eclipse.jetty' + module = 'jetty-server' + versions = '[9.4.10,10.0)' + } + pass { + name = 'early_10_series' + group = 'org.eclipse.jetty' + module = 'jetty-server' + // _multiParts: MultiPartFormInputStream (before 10.0.10 switched to MultiParts) + versions = '[10.0.0,10.0.10)' + javaVersion = 11 + } + pass { + name = '10_series' + group = 'org.eclipse.jetty' + module = 'jetty-server' + versions = '[10.0.10,11.0)' + javaVersion = 11 + } +} + +apply from: "$rootDir/gradle/java.gradle" + +dependencies { + compileOnly group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.4.21.v20190926' +} + +// testing happens in the jetty-* modules diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/gradle.lockfile b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/gradle.lockfile new file mode 100644 index 00000000000..961940d455a --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/gradle.lockfile @@ -0,0 +1,126 @@ +# This is a Gradle generated file for dependency locking. +# Manual edits can break the build and are not advised. +# This file is expected to be part of source control. +cafe.cryptography:curve25519-elisabeth:0.1.0=testRuntimeClasspath +cafe.cryptography:ed25519-elisabeth:0.1.0=testRuntimeClasspath +ch.qos.logback:logback-classic:1.2.13=testCompileClasspath,testRuntimeClasspath +ch.qos.logback:logback-core:1.2.13=testCompileClasspath,testRuntimeClasspath +com.blogspot.mydailyjava:weak-lock-free:0.17=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +com.datadoghq.okhttp3:okhttp:3.12.15=testCompileClasspath,testRuntimeClasspath +com.datadoghq.okio:okio:1.17.6=testCompileClasspath,testRuntimeClasspath +com.datadoghq:dd-instrument-java:0.0.3=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +com.datadoghq:dd-javac-plugin-client:0.2.2=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +com.datadoghq:java-dogstatsd-client:4.4.3=testRuntimeClasspath +com.datadoghq:sketches-java:0.8.3=testRuntimeClasspath +com.github.javaparser:javaparser-core:3.25.6=codenarc +com.github.jnr:jffi:1.3.14=testRuntimeClasspath +com.github.jnr:jnr-a64asm:1.0.0=testRuntimeClasspath +com.github.jnr:jnr-constants:0.10.4=testRuntimeClasspath +com.github.jnr:jnr-enxio:0.32.19=testRuntimeClasspath +com.github.jnr:jnr-ffi:2.2.18=testRuntimeClasspath +com.github.jnr:jnr-posix:3.1.21=testRuntimeClasspath +com.github.jnr:jnr-unixsocket:0.38.24=testRuntimeClasspath +com.github.jnr:jnr-x86asm:1.0.2=testRuntimeClasspath +com.github.spotbugs:spotbugs-annotations:4.9.8=compileClasspath,spotbugs,testCompileClasspath,testRuntimeClasspath +com.github.spotbugs:spotbugs:4.9.8=spotbugs +com.github.stephenc.jcip:jcip-annotations:1.0-1=spotbugs +com.google.auto.service:auto-service-annotations:1.1.1=annotationProcessor,compileClasspath,testAnnotationProcessor,testCompileClasspath +com.google.auto.service:auto-service:1.1.1=annotationProcessor,testAnnotationProcessor +com.google.auto:auto-common:1.2.1=annotationProcessor,testAnnotationProcessor +com.google.code.findbugs:jsr305:3.0.2=annotationProcessor,compileClasspath,spotbugs,testAnnotationProcessor,testCompileClasspath,testRuntimeClasspath +com.google.code.gson:gson:2.13.2=spotbugs +com.google.errorprone:error_prone_annotations:2.18.0=annotationProcessor,testAnnotationProcessor +com.google.errorprone:error_prone_annotations:2.41.0=spotbugs +com.google.guava:failureaccess:1.0.1=annotationProcessor,testAnnotationProcessor +com.google.guava:guava:20.0=testCompileClasspath,testRuntimeClasspath +com.google.guava:guava:32.0.1-jre=annotationProcessor,testAnnotationProcessor +com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava=annotationProcessor,testAnnotationProcessor +com.google.j2objc:j2objc-annotations:2.8=annotationProcessor,testAnnotationProcessor +com.google.re2j:re2j:1.7=testRuntimeClasspath +com.squareup.moshi:moshi:1.11.0=testCompileClasspath,testRuntimeClasspath +com.squareup.okhttp3:logging-interceptor:3.12.12=testCompileClasspath,testRuntimeClasspath +com.squareup.okhttp3:okhttp:3.12.12=testCompileClasspath,testRuntimeClasspath +com.squareup.okio:okio:1.17.5=testCompileClasspath,testRuntimeClasspath +com.thoughtworks.qdox:qdox:1.12.1=codenarc +commons-fileupload:commons-fileupload:1.5=testCompileClasspath,testRuntimeClasspath +commons-io:commons-io:2.11.0=testCompileClasspath,testRuntimeClasspath +commons-io:commons-io:2.20.0=spotbugs +de.thetaphi:forbiddenapis:3.10=compileClasspath +io.leangen.geantyref:geantyref:1.3.16=testRuntimeClasspath +io.sqreen:libsqreen:17.3.0=testRuntimeClasspath +javax.servlet:javax.servlet-api:3.1.0=compileClasspath,testCompileClasspath,testRuntimeClasspath +jaxen:jaxen:2.0.0=spotbugs +junit:junit:4.13.2=testRuntimeClasspath +net.bytebuddy:byte-buddy-agent:1.18.3=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +net.bytebuddy:byte-buddy:1.18.3=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +net.java.dev.jna:jna-platform:5.8.0=testRuntimeClasspath +net.java.dev.jna:jna:5.8.0=testRuntimeClasspath +net.sf.saxon:Saxon-HE:12.9=spotbugs +org.apache.ant:ant-antlr:1.10.14=codenarc +org.apache.ant:ant-junit:1.10.14=codenarc +org.apache.bcel:bcel:6.11.0=spotbugs +org.apache.commons:commons-lang3:3.19.0=spotbugs +org.apache.commons:commons-text:1.14.0=spotbugs +org.apache.logging.log4j:log4j-api:2.25.2=spotbugs +org.apache.logging.log4j:log4j-core:2.25.2=spotbugs +org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath +org.checkerframework:checker-qual:3.33.0=annotationProcessor,testAnnotationProcessor +org.codehaus.groovy:groovy-ant:3.0.23=codenarc +org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc +org.codehaus.groovy:groovy-groovydoc:3.0.23=codenarc +org.codehaus.groovy:groovy-json:3.0.23=codenarc +org.codehaus.groovy:groovy-json:3.0.25=testCompileClasspath,testRuntimeClasspath +org.codehaus.groovy:groovy-templates:3.0.23=codenarc +org.codehaus.groovy:groovy-xml:3.0.23=codenarc +org.codehaus.groovy:groovy:3.0.23=codenarc +org.codehaus.groovy:groovy:3.0.25=testCompileClasspath,testRuntimeClasspath +org.codenarc:CodeNarc:3.7.0=codenarc +org.dom4j:dom4j:2.2.0=spotbugs +org.eclipse.jetty:jetty-http:9.4.21.v20190926=compileClasspath +org.eclipse.jetty:jetty-io:9.4.21.v20190926=compileClasspath +org.eclipse.jetty:jetty-server:9.4.21.v20190926=compileClasspath +org.eclipse.jetty:jetty-util:9.4.21.v20190926=compileClasspath +org.gmetrics:GMetrics:2.1.0=codenarc +org.hamcrest:hamcrest-core:1.3=testRuntimeClasspath +org.hamcrest:hamcrest:3.0=testCompileClasspath,testRuntimeClasspath +org.jctools:jctools-core-jdk11:4.0.6=testRuntimeClasspath +org.jctools:jctools-core:4.0.6=testRuntimeClasspath +org.junit.jupiter:junit-jupiter-api:5.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.jupiter:junit-jupiter-engine:5.14.1=testRuntimeClasspath +org.junit.jupiter:junit-jupiter-params:5.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.jupiter:junit-jupiter:5.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.platform:junit-platform-commons:1.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.platform:junit-platform-engine:1.14.1=testCompileClasspath,testRuntimeClasspath +org.junit.platform:junit-platform-launcher:1.14.1=testRuntimeClasspath +org.junit.platform:junit-platform-runner:1.14.1=testRuntimeClasspath +org.junit.platform:junit-platform-suite-api:1.14.1=testRuntimeClasspath +org.junit.platform:junit-platform-suite-commons:1.14.1=testRuntimeClasspath +org.junit:junit-bom:5.14.0=spotbugs +org.junit:junit-bom:5.14.1=testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=testRuntimeClasspath +org.objenesis:objenesis:3.3=testCompileClasspath,testRuntimeClasspath +org.opentest4j:opentest4j:1.3.0=testCompileClasspath,testRuntimeClasspath +org.ow2.asm:asm-analysis:9.7.1=testRuntimeClasspath +org.ow2.asm:asm-analysis:9.9=spotbugs +org.ow2.asm:asm-commons:9.9=spotbugs +org.ow2.asm:asm-commons:9.9.1=testRuntimeClasspath +org.ow2.asm:asm-tree:9.9=spotbugs +org.ow2.asm:asm-tree:9.9.1=testRuntimeClasspath +org.ow2.asm:asm-util:9.7.1=testRuntimeClasspath +org.ow2.asm:asm-util:9.9=spotbugs +org.ow2.asm:asm:9.9=spotbugs +org.ow2.asm:asm:9.9.1=buildTimeInstrumentationPlugin,compileClasspath,muzzleTooling,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +org.slf4j:jcl-over-slf4j:1.7.30=testCompileClasspath,testRuntimeClasspath +org.slf4j:jul-to-slf4j:1.7.30=testCompileClasspath,testRuntimeClasspath +org.slf4j:log4j-over-slf4j:1.7.30=testCompileClasspath,testRuntimeClasspath +org.slf4j:slf4j-api:1.7.30=buildTimeInstrumentationPlugin,compileClasspath,muzzleBootstrap,muzzleTooling,runtimeClasspath +org.slf4j:slf4j-api:1.7.32=testCompileClasspath,testRuntimeClasspath +org.slf4j:slf4j-api:2.0.17=spotbugs,spotbugsSlf4j +org.slf4j:slf4j-simple:2.0.17=spotbugsSlf4j +org.snakeyaml:snakeyaml-engine:2.9=buildTimeInstrumentationPlugin,muzzleTooling,runtimeClasspath,testRuntimeClasspath +org.spockframework:spock-bom:2.4-groovy-3.0=testCompileClasspath,testRuntimeClasspath +org.spockframework:spock-core:2.4-groovy-3.0=testCompileClasspath,testRuntimeClasspath +org.tabletest:tabletest-junit:1.2.1=testCompileClasspath,testRuntimeClasspath +org.tabletest:tabletest-parser:1.2.0=testCompileClasspath,testRuntimeClasspath +org.xmlresolver:xmlresolver:5.3.3=spotbugs +empty=spotbugsPlugins diff --git a/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java new file mode 100644 index 00000000000..49a71f632dc --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/MultipartHelper.java @@ -0,0 +1,32 @@ +package datadog.trace.instrumentation.jetty94; + +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+, Jetty 9.4.x–10.x). + * + * @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.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java new file mode 100644 index 00000000000..52cdf1e8032 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/main/java/datadog/trace/instrumentation/jetty94/RequestExtractContentParametersInstrumentation.java @@ -0,0 +1,244 @@ +package datadog.trace.instrumentation.jetty94; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.api.gateway.Events.EVENTS; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +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.agent.tooling.muzzle.Reference; +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.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 net.bytebuddy.implementation.bytecode.assign.Assigner; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.MultiMap; + +@AutoService(InstrumenterModule.class) +public class RequestExtractContentParametersInstrumentation extends InstrumenterModule.AppSec + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + private static final String MULTI_MAP_INTERNAL_NAME = "Lorg/eclipse/jetty/util/MultiMap;"; + + public RequestExtractContentParametersInstrumentation() { + super("jetty"); + } + + @Override + 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( + 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"); + } + + // Discriminates Jetty [9.4.10, 10.0) + [10.0.0, 11.0): + // - _contentParameters + extractContentParameters(void) exist from 9.3+ (excludes 9.2) + // - _multiParts field exists from 9.4.10+ (excludes 9.3.x–9.4.9 covered by jetty-appsec-9.3) + // - primary spec: _multiParts: MultiParts → matches 9.4.10–9.4.x and 10.0.10+ + // - OR spec: _multiParts: MultiPartFormInputStream → matches 10.0.0–10.0.9 + // - _dispatcherType: Ljavax/servlet/DispatcherType; in the Request bytecode (excludes Jetty 11+ + // where the field descriptor is Ljakarta/servlet/DispatcherType;). This check is tied to the + // Request.class bytecode, NOT just classpath presence, so it works even when both + // javax.servlet and jakarta.servlet are on the classpath simultaneously. + // Note: GetFilenamesAdvice reads _multiParts with typing=DYNAMIC so it works for all versions. + private static final Reference REQUEST_REFERENCE = + new Reference.Builder("org.eclipse.jetty.server.Request") + .withMethod(new String[0], 0, "extractContentParameters", "V") + .withField(new String[0], 0, "_contentParameters", MULTI_MAP_INTERNAL_NAME) + .withField(new String[0], 0, "_multiParts", "Lorg/eclipse/jetty/server/MultiParts;") + .withField(new String[0], 0, "_dispatcherType", "Ljavax/servlet/DispatcherType;") + .or() + .withMethod(new String[0], 0, "extractContentParameters", "V") + .withField(new String[0], 0, "_contentParameters", MULTI_MAP_INTERNAL_NAME) + .withField( + new String[0], + 0, + "_multiParts", + "Lorg/eclipse/jetty/server/MultiPartFormInputStream;") + .withField(new String[0], 0, "_dispatcherType", "Ljavax/servlet/DispatcherType;") + .build(); + + @Override + public Reference[] additionalMuzzleReferences() { + return new Reference[] {REQUEST_REFERENCE}; + } + + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class ExtractContentParametersAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before(@Advice.FieldValue("_contentParameters") final MultiMap map) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Request.class); + return callDepth == 0 && map == null; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Enter boolean proceed, + @Advice.FieldValue("_contentParameters") final MultiMap map, + @ActiveRequestContext RequestContext reqCtx, + @Advice.Thrown(readOnly = false) Throwable t) { + CallDepthThreadLocalMap.decrementCallDepth(Request.class); + if (!proceed) { + return; + } + if (map == null || map.isEmpty()) { + return; + } + + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction> callback = + cbp.getCallback(EVENTS.requestBodyProcessed()); + if (callback == null) { + return; + } + + Flow flow = callback.apply(reqCtx, map); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); + if (blockResponseFunction != null) { + blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + if (t == null) { + t = new BlockingException("Blocked request (for Request/extractContentParameters)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + } + + /** + * 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}: set by the first {@code getParts()} call in Jetty 9.4.10+; + * means filenames were already reported. + *
+ */ + @RequiresRequestContext(RequestContextSlot.APPSEC) + public static class GetFilenamesAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + static boolean before( + @Advice.FieldValue("_contentParameters") final MultiMap contentParameters, + @Advice.FieldValue(value = "_multiParts", typing = Assigner.Typing.DYNAMIC) + final Object multiParts) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class); + return callDepth == 0 && contentParameters == null && multiParts == 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(); + } + } + } + } + } + + /** + * 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()}. + */ + @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; + } + 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.4/src/test/groovy/datadog/trace/instrumentation/jetty94/MultipartHelperTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/groovy/datadog/trace/instrumentation/jetty94/MultipartHelperTest.groovy new file mode 100644 index 00000000000..af027e5dd65 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.4/src/test/groovy/datadog/trace/instrumentation/jetty94/MultipartHelperTest.groovy @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.jetty94 + +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-server/jetty-server-10.0/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/build.gradle index 334d032273c..3df32214e41 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/build.gradle @@ -48,9 +48,11 @@ dependencies { } testImplementation project(':dd-java-agent:instrumentation:jetty:jetty-util-9.4.31') - testImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '10.0.0' - testImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '10.0.0' - testImplementation group: 'org.eclipse.jetty.websocket', name: 'websocket-javax-server', version: '10.0.0' + // Use 10.0.10+ so jetty-appsec-9.4 applies: _multiParts changed from MultiPartFormInputStream + // to MultiParts in 10.0.10 (jetty-appsec-9.4's muzzle requires the MultiParts type). + testImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '10.0.10' + testImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '10.0.10' + testImplementation group: 'org.eclipse.jetty.websocket', name: 'websocket-javax-server', version: '10.0.10' testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures') testImplementation testFixtures(project(":dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-9.0")) testImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0')) @@ -63,7 +65,7 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-7.0') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-8.1.3') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.2') - testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3') + testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4') // Include all websocket instrumentation modules for testing. Only the version-compatible module will apply at runtime. testRuntimeOnly project(':dd-java-agent:instrumentation:websocket:javax-websocket-1.0') testRuntimeOnly project(':dd-java-agent:instrumentation:websocket:jakarta-websocket-2.0') @@ -72,13 +74,13 @@ dependencies { latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '10.+' latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '10.+' latestDepTestImplementation group: 'org.eclipse.jetty.websocket', name: 'websocket-javax-server', version: '10.+' - latestDepTestImplementation project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3') + latestDepTestImplementation project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4') latestDepTestImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0')) latestDepForkedTestImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '10.+' latestDepForkedTestImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '10.+' latestDepForkedTestImplementation group: 'org.eclipse.jetty.websocket', name: 'websocket-javax-server', version: '10.+' - latestDepForkedTestImplementation project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3') + latestDepForkedTestImplementation project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4') latestDepForkedTestImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0')) } 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/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/build.gradle index 119dd38ea12..ae74abe7d1b 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/build.gradle @@ -47,7 +47,7 @@ dependencies { testImplementation ("org.eclipse.jetty.websocket:websocket-jakarta-server:11.0.0") { exclude group: 'org.slf4j', module: 'slf4j-api' } - testImplementation(project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3')) + testImplementation(project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-11.0')) testImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:jakarta-servlet-5.0')) testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-9.0') 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..799a7d392b1 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,21 @@ class JettyAsyncHandlerTest extends Jetty11Test implements TestingGenericHttpNam false } + @Override + boolean testBodyFilenames() { + false + } + + @Override + boolean testBodyFilenamesCalledOnce() { + false + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + false + } + static class ContinuationTestHandler implements Handler { @Delegate private final Handler delegate diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-12.0/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-12.0/build.gradle index d2346ef072a..25b4cd11e3f 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-12.0/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-12.0/build.gradle @@ -67,6 +67,8 @@ dependencies { } testImplementation(project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3')) + testRuntimeOnly(project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4')) + testRuntimeOnly(project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-11.0')) testImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:jakarta-servlet-5.0')) testImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0')) testRuntimeOnly project(':dd-java-agent:instrumentation:websocket:javax-websocket-1.0') diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/build.gradle index 613f00a4f3e..0b34ae39684 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/build.gradle @@ -20,6 +20,11 @@ configurations.testRuntimeOnly { exclude group: 'javax.servlet', module: 'javax.servlet-api' } +tasks.named('latestDepForkedTest', Test) { + // Signal that we are running against Jetty 8.x so Jetty8*LatestDepForkedTest activates. + systemProperty 'test.dd.filenames', 'true' +} + dependencies { compileOnly group: 'org.eclipse.jetty', name: 'jetty-server', version: '7.6.0.v20120127' implementation project(':dd-java-agent:instrumentation:jetty:jetty-common') @@ -34,8 +39,14 @@ dependencies { testImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '7.6.0.v20120127' testImplementation group: 'org.eclipse.jetty', name: 'jetty-continuation', version: '7.6.0.v20120127' testImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0')) + // Needed to compile Jetty8LatestDepForkedTest (provides MultipartConfigElement). + // Uses the Orbit repackaging so it is not caught by the javax.servlet:javax.servlet-api exclusion. + // Compile-only: the Orbit jar is provided at runtime by Jetty 8.x in the latestDepForkedTest. + testCompileOnly group: 'org.eclipse.jetty.orbit', name: 'javax.servlet', version: '3.0.0.v201112011016' testRuntimeOnly project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-2.2') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-7.0') + // Activated only on Jetty 8.x (muzzle rejects it for 7.6) + testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-8.1.3') latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.+' latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.+' diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/gradle.lockfile b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/gradle.lockfile index 17fe90eaa61..b1d6c0c1cdc 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/gradle.lockfile +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/gradle.lockfile @@ -76,7 +76,7 @@ org.codehaus.groovy:groovy:3.0.23=codenarc org.codehaus.groovy:groovy:3.0.25=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.codenarc:CodeNarc:3.7.0=codenarc org.dom4j:dom4j:2.2.0=spotbugs -org.eclipse.jetty.orbit:javax.servlet:3.0.0.v201112011016=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestImplementation +org.eclipse.jetty.orbit:javax.servlet:3.0.0.v201112011016=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestImplementation,testCompileClasspath org.eclipse.jetty:jetty-continuation:7.6.0.v20120127=compileClasspath,testCompileClasspath,testRuntimeClasspath org.eclipse.jetty:jetty-continuation:8.2.0.v20160908=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestImplementation org.eclipse.jetty:jetty-http:7.6.0.v20120127=compileClasspath,testCompileClasspath,testRuntimeClasspath diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty8LatestDepForkedTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty8LatestDepForkedTest.groovy new file mode 100644 index 00000000000..8c7a6892608 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-7.6/src/test/groovy/Jetty8LatestDepForkedTest.groovy @@ -0,0 +1,100 @@ +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import org.eclipse.jetty.server.Request +import org.eclipse.jetty.server.handler.AbstractHandler +import spock.lang.Requires + +import javax.servlet.MultipartConfigElement +import javax.servlet.ServletException +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +/** + * Integration tests for multipart filename extraction on Jetty 8.x. + * + *

Jetty 8.x introduced Servlet 3.0 and {@code getParts()}, which is the only entry point for + * multipart processing in this version range (there is no {@code extractContentParameters()} + * instrumentation like in 9.3+). The handler must therefore call {@code getParts()} explicitly + * before {@code getParameterMap()} so that multipart form fields are visible to the servlet. + * + *

Only activated for the {@code latestDepForkedTest} Gradle task (Jetty 8.x). The + * {@code test.dd.filenames} system property gates execution, preventing these tests from + * running against Jetty 7.6 where {@code getParts()} does not exist. + */ +abstract class Jetty8LatestDepForkedTest extends Jetty76Test { + + @Override + AbstractHandler handler() { + new Jetty8TestHandler() + } + + @Override + boolean testBodyMultipart() { + true + } + + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilenamesCalledOnce() { + // Jetty 8.x has no _multiParts field guard; getParts() called multiple times + // (BODY_MULTIPART_REPEATED) fires the event more than once. + false + } + + @Override + boolean testBodyFilenamesCalledOnceCombined() { + // Jetty 8.x has no _contentParameters field guard; BODY_MULTIPART_COMBINED + // fires the event on the getParts() call regardless of prior parameterMap access. + false + } + + static class Jetty8TestHandler extends AbstractHandler { + private static final MultipartConfigElement MULTIPART_CONFIG = + new MultipartConfigElement(System.getProperty('java.io.tmpdir')) + + @Override + void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException { + if (baseRequest.dispatcherType.name() != 'ERROR') { + // Enable Servlet 3.0 multipart processing for all requests. + request.setAttribute('org.eclipse.jetty.multipartConfig', MULTIPART_CONFIG) + request.setAttribute('org.eclipse.multipartConfig', MULTIPART_CONFIG) + + // Jetty 8.x does not populate getParameterMap() from multipart form fields without a + // prior getParts() call (unlike 9.3+ where extractContentParameters() does this). + // Pre-call getParts() for BODY_MULTIPART so the servlet can read form fields via + // getParameterMap(). Skip for BODY_MULTIPART_REPEATED and BODY_MULTIPART_COMBINED, + // which call getParts() themselves and rely on the first call triggering filenames. + def endpoint = HttpServerTest.ServerEndpoint.forPath(request.requestURI) + if (endpoint == HttpServerTest.ServerEndpoint.BODY_MULTIPART) { + try { + request.getParts() + } catch (IOException | ServletException ignored) {} + } + + Jetty76Test.TestHandler.handleRequest(baseRequest, response) + baseRequest.handled = true + } else { + Jetty76Test.errorHandler.handle(target, baseRequest, response, response) + } + } + } +} + +@Requires({ + System.getProperty('test.dd.filenames') +}) +class Jetty8V0LatestDepForkedTest extends Jetty8LatestDepForkedTest +implements TestingGenericHttpNamingConventions.ServerV0 { +} + +@Requires({ + System.getProperty('test.dd.filenames') +}) +class Jetty8V1LatestDepForkedTest extends Jetty8LatestDepForkedTest +implements TestingGenericHttpNamingConventions.ServerV1 { +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy index b54000a00ec..3a1e9bdf844 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy @@ -9,4 +9,12 @@ class Jetty9InactiveAppSecTest extends AppSecInactiveHttpServerTest { HttpServer server() { new JettyServer(TestHandler.INSTANCE) } + + // jetty-appsec-8.1.3 covers [8.1.3, 9.2.0.RC0) which includes Jetty 9.0.x. + // It instruments extractContentParameters() but calls ParameterCollector.put(String, String) + // which does not exist in Jetty 9.0.x → HTTP 500 on multipart requests. + @Override + protected boolean supportsMultipart() { + false + } } 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..36faf0b24a9 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 @@ -82,7 +82,10 @@ abstract class Jetty9Test extends HttpServerTest { @Override boolean testBodyMultipart() { - true + // jetty-appsec-8.1.3 covers [8.1.3, 9.2.0.RC0) which includes Jetty 9.0.x. + // Its extractContentParameters() advice calls ParameterCollector.put(String, String) + // which does not exist in Jetty 9.0.x → HTTP 500 on multipart requests. + false } @Override diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy index b54000a00ec..3a1e9bdf844 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9InactiveAppSecTest.groovy @@ -9,4 +9,12 @@ class Jetty9InactiveAppSecTest extends AppSecInactiveHttpServerTest { HttpServer server() { new JettyServer(TestHandler.INSTANCE) } + + // jetty-appsec-8.1.3 covers [8.1.3, 9.2.0.RC0) which includes Jetty 9.0.x. + // It instruments extractContentParameters() but calls ParameterCollector.put(String, String) + // which does not exist in Jetty 9.0.x → HTTP 500 on multipart requests. + @Override + protected boolean supportsMultipart() { + false + } } 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..848f551bf29 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 @@ -81,7 +81,10 @@ abstract class Jetty9Test extends HttpServerTest { @Override boolean testBodyMultipart() { - true + // jetty-appsec-8.1.3 covers [8.1.3, 9.2.0.RC0) which includes Jetty 9.0.x. + // Its extractContentParameters() advice calls ParameterCollector.put(String, String) + // which does not exist in Jetty 9.0.x → HTTP 500 on multipart requests. + false } @Override diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/build.gradle index 5d08c44f4c9..736630c640e 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/build.gradle @@ -41,6 +41,7 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-8.1.3') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.2') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3') + testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4') latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.4.20.v20190813' latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.4.20.v20190813' 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/build.gradle b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/build.gradle index d9ef585146e..144358e05f5 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/build.gradle +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/build.gradle @@ -42,7 +42,7 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-7.0') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-8.1.3') testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.2') - testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3') + testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4') latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.+' latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.+' 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 diff --git a/settings.gradle.kts b/settings.gradle.kts index 75b2af69561..f77bd79afb9 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -417,6 +417,8 @@ include( ":dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-8.1.3", ":dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.2", ":dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.3", + ":dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-9.4", + ":dd-java-agent:instrumentation:jetty:jetty-appsec:jetty-appsec-11.0", ":dd-java-agent:instrumentation:jetty:jetty-client:jetty-client-10.0", ":dd-java-agent:instrumentation:jetty:jetty-client:jetty-client-12.0", ":dd-java-agent:instrumentation:jetty:jetty-client:jetty-client-9.1",