fix(appsec): extract readContent to helper class to fix muzzle failure

The static readContent method in ParseRequestAdvice created a self-reference
in the inlined advice bytecode (invokestatic on CommonsFileUploadAppSecModule$ParseRequestAdvice)
that muzzle could not resolve in the application classloader, causing the
instrumentation to be silently skipped.

Moves readContent to a new FileItemContentReader helper class declared via
helperClassNames(), which muzzle skips and the HelperInjector injects into
the application classloader at runtime.

Author: jandro996

dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java

@@ -17,9 +17,6 @@
 import datadog.trace.api.gateway.RequestContext;
 import datadog.trace.api.gateway.RequestContextSlot;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.BiFunction;
@@ -39,6 +36,13 @@ public String instrumentedType() {
     return "org.apache.commons.fileupload.servlet.ServletFileUpload";
   }
 
+  @Override
+  public String[] helperClassNames() {
+    return new String[] {
+      "datadog.trace.instrumentation.commons.fileupload.FileItemContentReader",
+    };
+  }
+
   @Override
   public void methodAdvice(MethodTransformer transformer) {
     transformer.applyAdvice(
@@ -51,8 +55,6 @@ public void methodAdvice(MethodTransformer transformer) {
   @RequiresRequestContext(RequestContextSlot.APPSEC)
   public static class ParseRequestAdvice {
 
-    static final int MAX_FILE_CONTENT_BYTES = 4096;
-
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
     static void after(
         @Advice.Return final List<FileItem> fileItems,
@@ -114,7 +116,7 @@ static void after(
         if (name == null || name.isEmpty()) {
           continue;
         }
-        filesContent.add(readContent(fileItem));
+        filesContent.add(FileItemContentReader.readContent(fileItem));
       }
       if (filesContent.isEmpty()) {
         return;
@@ -131,20 +133,5 @@ static void after(
         }
       }
     }
-
-    static String readContent(FileItem fileItem) {
-      try (InputStream is = fileItem.getInputStream()) {
-        byte[] buf = new byte[MAX_FILE_CONTENT_BYTES];
-        int total = 0;
-        int n;
-        while (total < MAX_FILE_CONTENT_BYTES
-            && (n = is.read(buf, total, MAX_FILE_CONTENT_BYTES - total)) != -1) {
-          total += n;
-        }
-        return new String(buf, 0, total, StandardCharsets.ISO_8859_1);
-      } catch (IOException ignored) {
-        return "";
-      }
-    }
   }
 }

dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java

@@ -0,0 +1,28 @@
+package datadog.trace.instrumentation.commons.fileupload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.commons.fileupload.FileItem;
+
+/** Helper class injected into the application classloader by the AppSec instrumentation. */
+public final class FileItemContentReader {
+  public static final int MAX_CONTENT_BYTES = 4096;
+
+  public static String readContent(FileItem fileItem) {
+    try (InputStream is = fileItem.getInputStream()) {
+      byte[] buf = new byte[MAX_CONTENT_BYTES];
+      int total = 0;
+      int n;
+      while (total < MAX_CONTENT_BYTES
+          && (n = is.read(buf, total, MAX_CONTENT_BYTES - total)) != -1) {
+        total += n;
+      }
+      return new String(buf, 0, total, StandardCharsets.ISO_8859_1);
+    } catch (IOException ignored) {
+      return "";
+    }
+  }
+
+  private FileItemContentReader() {}
+}

dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy

@@ -1,4 +1,4 @@
-import datadog.trace.instrumentation.commons.fileupload.CommonsFileUploadAppSecModule
+import datadog.trace.instrumentation.commons.fileupload.FileItemContentReader
 import org.apache.commons.fileupload.FileItem
 import spock.lang.Specification
 
@@ -10,20 +10,20 @@ class CommonsFileUploadAppSecModuleTest extends Specification {
     def item = fileItem(content)
 
     expect:
-    CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == content
+    FileItemContentReader.readContent(item) == content
   }
 
-  def "readContent truncates content to MAX_FILE_CONTENT_BYTES"() {
+  def "readContent truncates content to MAX_CONTENT_BYTES"() {
     given:
-    def largeContent = 'X' * (CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + 500)
+    def largeContent = 'X' * (FileItemContentReader.MAX_CONTENT_BYTES + 500)
     def item = fileItem(largeContent)
 
     when:
-    def result = CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item)
+    def result = FileItemContentReader.readContent(item)
 
     then:
-    result.length() == CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES
-    result == 'X' * CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES
+    result.length() == FileItemContentReader.MAX_CONTENT_BYTES
+    result == 'X' * FileItemContentReader.MAX_CONTENT_BYTES
   }
 
   def "readContent returns empty string when getInputStream throws"() {
@@ -32,27 +32,27 @@ class CommonsFileUploadAppSecModuleTest extends Specification {
     item.getInputStream() >> { throw new IOException('simulated error') }
 
     expect:
-    CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == ''
+    FileItemContentReader.readContent(item) == ''
   }
 
   def "readContent returns empty string for empty content"() {
     given:
     def item = fileItem('')
 
     expect:
-    CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == ''
+    FileItemContentReader.readContent(item) == ''
   }
 
-  def "readContent reads exactly MAX_FILE_CONTENT_BYTES when content equals the limit"() {
+  def "readContent reads exactly MAX_CONTENT_BYTES when content equals the limit"() {
     given:
-    def content = 'A' * CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES
+    def content = 'A' * FileItemContentReader.MAX_CONTENT_BYTES
     def item = fileItem(content)
 
     when:
-    def result = CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item)
+    def result = FileItemContentReader.readContent(item)
 
     then:
-    result.length() == CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES
+    result.length() == FileItemContentReader.MAX_CONTENT_BYTES
     result == content
   }