refactor(appsec): cache reflection and use try-with-resources in ParameterCollector

- Replace per-call getMethod() with volatile (Class, Method) entry cache so
  reflection resolves once per concrete Part class rather than per file per request.
- Use try-with-resources for InputStream in readContent() instead of try/finally.
- Add unit test covering the Tomcat 7 getFilename() fallback path.

Author: jandro996

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java

@@ -3,6 +3,7 @@
 import java.io.InputStream;
 import java.lang.reflect.Method;
 import java.nio.charset.StandardCharsets;
+import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -142,11 +143,23 @@ public List<String> getContents() {
       return contents != null ? contents : Collections.<String>emptyList();
     }
 
+    // Entry caches (class → method) stored as a single volatile write for safe publication.
+    // Keyed by Part concrete class; re-resolved when the class changes (different Tomcat version).
+    private static volatile Map.Entry<Class<?>, Method> cachedInputStreamEntry;
+    private static volatile Map.Entry<Class<?>, Method> cachedFilenameEntry;
+
     private static String readContent(Object part) {
       try {
-        Method m = part.getClass().getMethod("getInputStream");
-        InputStream is = (InputStream) m.invoke(part);
-        try {
+        Class<?> partClass = part.getClass();
+        Map.Entry<Class<?>, Method> entry = cachedInputStreamEntry;
+        Method m;
+        if (entry == null || entry.getKey() != partClass) {
+          m = partClass.getMethod("getInputStream");
+          cachedInputStreamEntry = new AbstractMap.SimpleImmutableEntry<>(partClass, m);
+        } else {
+          m = entry.getValue();
+        }
+        try (InputStream is = (InputStream) m.invoke(part)) {
           byte[] buf = new byte[MAX_CONTENT_BYTES];
           int total = 0;
           int n;
@@ -155,28 +168,43 @@ private static String readContent(Object part) {
             total += n;
           }
           return new String(buf, 0, total, StandardCharsets.ISO_8859_1);
-        } finally {
-          is.close();
         }
       } catch (Exception ignored) {
         return "";
       }
     }
 
     private static String getFilename(Object part) {
-      // Try getSubmittedFileName() first — Servlet 3.1+ / Tomcat 8+ (both javax and jakarta)
-      try {
-        Method m = part.getClass().getMethod("getSubmittedFileName");
-        return (String) m.invoke(part);
-      } catch (Exception ignored) {
+      Class<?> partClass = part.getClass();
+      Map.Entry<Class<?>, Method> entry = cachedFilenameEntry;
+      Method m;
+      if (entry == null || entry.getKey() != partClass) {
+        m = null;
+        // Try getSubmittedFileName() first — Servlet 3.1+ / Tomcat 8+ (both javax and jakarta)
+        try {
+          m = partClass.getMethod("getSubmittedFileName");
+        } catch (Exception ignored) {
+        }
+        if (m == null) {
+          // Fall back to getFilename() — Tomcat 7 ApplicationPart specific
+          try {
+            m = partClass.getMethod("getFilename");
+          } catch (Exception ignored) {
+          }
+        }
+        // null value in the entry means "no filename method on this class"
+        cachedFilenameEntry = new AbstractMap.SimpleImmutableEntry<>(partClass, m);
+      } else {
+        m = entry.getValue();
+      }
+      if (m == null) {
+        return null;
       }
-      // Fall back to getFilename() — Tomcat 7 ApplicationPart specific
       try {
-        Method m = part.getClass().getMethod("getFilename");
         return (String) m.invoke(part);
       } catch (Exception ignored) {
+        return null;
       }
-      return null;
     }
   }
 }

dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/groovy/datadog/trace/instrumentation/tomcat7/ParameterCollectorImplTest.groovy

@@ -131,6 +131,18 @@ class ParameterCollectorImplTest extends Specification {
     collector.getFilenames() == ['file.txt']
   }
 
+  void 'addPart falls back to getFilename() when getSubmittedFileName() is absent (Tomcat 7)'() {
+    given:
+    def collector = new ParameterCollector.ParameterCollectorImpl(true)
+
+    when:
+    collector.addPart(new Tomcat7Part('tomcat7.txt', 'tomcat7 content'))
+
+    then:
+    collector.getContents() == ['tomcat7 content']
+    collector.getFilenames() == ['tomcat7.txt']
+  }
+
   void 'ParameterCollectorNoop getContents returns empty list'() {
     expect:
     ParameterCollector.ParameterCollectorNoop.INSTANCE.getContents().isEmpty()
@@ -189,4 +201,23 @@ class ParameterCollectorImplTest extends Specification {
       new ByteArrayInputStream(bytes)
     }
   }
+
+  /** Simulates a Tomcat 7 ApplicationPart that only exposes getFilename(), not getSubmittedFileName(). */
+  static class Tomcat7Part {
+    private final String filename
+    private final String content
+
+    Tomcat7Part(String filename, String content) {
+      this.filename = filename
+      this.content = content
+    }
+
+    String getFilename() {
+      filename
+    }
+
+    InputStream getInputStream() {
+      new ByteArrayInputStream((content ?: '').getBytes('ISO-8859-1'))
+    }
+  }
 }