fix: Replaces testing http client by raw jetty in Apache Http Client instrumentation

bric3 · bric3 · commit ec55364439f6 · 2025-12-16T18:48:43.000+01:00
Now that :dd-java-agent:testing shadows jetty, the jetty instrumentation was not applied.
diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/build.gradle b/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/build.gradle
@@ -39,9 +39,12 @@ dependencies {
   // to instrument the integration test
   iastIntegrationTestImplementation project(':dd-java-agent:agent-iast:iast-test-fixtures')
   iastIntegrationTestImplementation group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
-  iastIntegrationTestRuntimeOnly(project(':dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-9.0'))
+  // Provide real (non-shadowed) jetty for the test server bootstrap
+  iastIntegrationTestImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.4.56.v20240826'
+  iastIntegrationTestImplementation group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.4.56.v20240826'
   iastIntegrationTestRuntimeOnly(project(':dd-java-agent:instrumentation:apache-httpcore:apache-httpcore-4.0'))
   iastIntegrationTestRuntimeOnly(project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-common'))
+  iastIntegrationTestRuntimeOnly(project(':dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-9.0'))
   iastIntegrationTestRuntimeOnly(project(':dd-java-agent:instrumentation:java:java-lang:java-lang-1.8'))
   iastIntegrationTestRuntimeOnly(project(':dd-java-agent:instrumentation:java:java-net:java-net-1.8'))
   iastIntegrationTestRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')
diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/iastIntegrationTest/groovy/IastHttpClientIntegrationTest.groovy b/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/iastIntegrationTest/groovy/IastHttpClientIntegrationTest.groovy
@@ -5,10 +5,16 @@ import datadog.trace.agent.test.base.HttpServer
 import foo.bar.VulnerableUrlBuilder
 import okhttp3.HttpUrl
 import okhttp3.Request
+import org.eclipse.jetty.server.Server
+import org.eclipse.jetty.server.ServerConnector
+import org.eclipse.jetty.servlet.ServletContextHandler
+import org.eclipse.jetty.servlet.ServletHolder
 
-import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import javax.servlet.http.HttpServlet
+import javax.servlet.http.HttpServletRequest
+import javax.servlet.http.HttpServletResponse
 
-class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
+class IastHttpClientIntegrationTest extends IastHttpServerTest<Server> {
 
   static final CLIENTS = [
     'org.apache.http.impl.client.AutoRetryHttpClient',
@@ -20,28 +26,58 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
   @Override
   HttpServer server() {
     final controller = new SsrfController()
-    return httpServer {
-      handlers {
-        prefix('/ssrf/execute') {
-          final msg = controller.apacheSsrf(
-          VulnerableUrlBuilder.url(request),
-          (String) request.getParameter('clientClassName'),
-          (String) request.getParameter('method'),
-          (String) request.getParameter('requestType'),
-          (String) request.getParameter('scheme')
-          )
-          response.status(200).send(msg)
-        }
+
+    // Use _raw_ jetty API, so the jetty instrumentation is applied
+    // (the :dd-java-agent:testing has a shadowed jetty server.)
+    return new HttpServer() {
+      Server jettyServer
+      int port
+
+      @Override
+      void start() {
+        jettyServer = new Server(0) // 0 = random port
+
+        ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS)
+        context.setContextPath('/')
+        jettyServer.setHandler(context)
+
+        // Add servlet for the /ssrf/execute endpoint
+        context.addServlet(new ServletHolder(new HttpServlet() {
+          @Override
+          protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
+            final msg = controller.apacheSsrf(
+              VulnerableUrlBuilder.url(req),
+              req.getParameter('clientClassName'),
+              req.getParameter('method'),
+              req.getParameter('requestType'),
+              req.getParameter('scheme')
+            )
+            resp.setStatus(200)
+            resp.writer.write(msg)
+          }
+        }), '/ssrf/execute')
+
+        jettyServer.start()
+        port = ((ServerConnector) jettyServer.connectors[0]).localPort
       }
-    }.asHttpServer()
+
+      @Override
+      void stop() {
+        jettyServer?.stop()
+      }
+
+      @Override
+      URI address() {
+        return new URI("http://localhost:${port}/")
+      }
+    }
   }
 
   void 'ssrf is present: #suite'() {
     setup:
     final url = suite.url(address)
     final request = new Request.Builder().url(url).get().build()
 
-
     when:
     def response = client.newCall(request).execute()
 
@@ -52,6 +88,9 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
     assert to != null
     hasVulnerability(vul -> vul.type == VulnerabilityType.SSRF && suite.evidenceMatches(vul.evidence))
 
+    cleanup:
+    response?.close()
+
     where:
     suite << createTestSuite()
   }
@@ -73,12 +112,12 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
   private List<TestSuite> createTestSuite(client, method, request, scheme) {
     return TaintedTarget.values().collect {
       new TestSuite(
-      description: "Tainted ${it.name()} with ${client} client and ${method} method with ${request} and ${scheme} scheme",
-      executedMethod: method.name(),
-      clientImplementation: client,
-      requestType: request.name(),
-      scheme: scheme,
-      tainted: it
+        description: "Tainted ${it.name()} with ${client} client and ${method} method with ${request} and ${scheme} scheme",
+        executedMethod: method.name(),
+        clientImplementation: client,
+        requestType: request.name(),
+        scheme: scheme,
+        tainted: it
       )
     }
   }
@@ -106,14 +145,14 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
 
     HttpUrl url(final URI address) {
       final builder = new HttpUrl.Builder()
-      .scheme(address.getScheme())
-      .host(address.getHost())
-      .port(address.getPort())
-      .addPathSegments('ssrf/execute')
-      .addQueryParameter('clientClassName', clientImplementation)
-      .addQueryParameter('method', executedMethod)
-      .addQueryParameter('requestType', requestType)
-      .addQueryParameter('scheme', scheme)
+        .scheme(address.getScheme())
+        .host(address.getHost())
+        .port(address.getPort())
+        .addPathSegments('ssrf/execute')
+        .addQueryParameter('clientClassName', clientImplementation)
+        .addQueryParameter('method', executedMethod)
+        .addQueryParameter('requestType', requestType)
+        .addQueryParameter('scheme', scheme)
       tainted.addTainted(builder, this)
       return builder.build()
     }
@@ -129,7 +168,7 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
   }
 
   private static enum TaintedTarget {
-    URL{
+    URL {
       void addTainted(HttpUrl.Builder builder, TestSuite suite) {
         builder.addQueryParameter('url', suite.scheme + '://inexistent/test?1=1')
       }
@@ -138,7 +177,7 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
         return assertTainted(evidence, 'url', suite.scheme + '://inexistent/test?1=1')
       }
     },
-    SCHEME{
+    SCHEME {
       void addTainted(HttpUrl.Builder builder, TestSuite suite) {
         // already added by the test
       }
@@ -147,7 +186,7 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
         return assertTainted(evidence, 'scheme', suite.scheme)
       }
     },
-    HOST{
+    HOST {
       void addTainted(HttpUrl.Builder builder, TestSuite suite) {
         builder.addQueryParameter('host', 'inexistent')
       }
@@ -156,7 +195,7 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
         return assertTainted(evidence, 'host', 'inexistent')
       }
     },
-    PATH{
+    PATH {
       void addTainted(HttpUrl.Builder builder, TestSuite suite) {
         builder.addQueryParameter('path', '/test')
       }
@@ -165,7 +204,7 @@ class IastHttpClientIntegrationTest extends IastHttpServerTest<HttpServer> {
         return assertTainted(evidence, 'path', '/test')
       }
     },
-    QUERY{
+    QUERY {
       void addTainted(HttpUrl.Builder builder, TestSuite suite) {
         builder.addQueryParameter('query', '?1=1')
       }
diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/iastIntegrationTest/java/foo/bar/VulnerableUrlBuilder.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/iastIntegrationTest/java/foo/bar/VulnerableUrlBuilder.java
@@ -1,28 +1,29 @@
 package foo.bar;
 
 import datadog.trace.agent.test.server.http.TestHttpServer.HandlerApi.RequestApi;
+import javax.servlet.http.HttpServletRequest;
 
 /** Class to be instrumented by IAST call sites containing methods to work with urls */
 public abstract class VulnerableUrlBuilder {
 
   private VulnerableUrlBuilder() {}
 
-  public static String url(RequestApi request) {
-    final String url = (String) request.getParameter("url");
+  public static String url(HttpServletRequest request) {
+    final String url = request.getParameter("url");
     if (url != null) {
       return url;
     }
-    final String scheme = (String) request.getParameter("scheme");
+    final String scheme = request.getParameter("scheme");
     final boolean https = "https".equals(scheme);
-    final String host = (String) request.getParameter("host");
+    final String host = request.getParameter("host");
     if (host != null) {
       return (https ? "https://" : "http://") + host + "/test?1=1";
     }
-    final String path = (String) request.getParameter("path");
+    final String path = request.getParameter("path");
     if (path != null) {
       return (https ? "https://inexistent/" : "http://inexistent/") + path + "?1=1";
     }
-    final String query = (String) request.getParameter("query");
+    final String query = request.getParameter("query");
     if (query != null) {
       return (https ? "https://inexistent/test" : "http://inexistent/test") + query;
     }
