Add server.request.body.files_content AppSec address for Tomcat and Netty multipart (#11198)

refactor(appsec): extract MultipartContentDecoder to internal-api for reuse across integrations

test(appsec): add missing corner cases to MultipartContentDecoderTest

fix(appsec): use machine default charset as fallback in MultipartContentDecoder

Replaces hardcoded UTF-8 (no-charset default) and ISO-8859-1 (fallback)
with Charset.defaultCharset() in both cases, per reviewer feedback.

test(appsec): migrate MultipartContentDecoderTest from Groovy/Spock to Java/JUnit 5

feat(appsec): extend files_content WAF address to Tomcat and Netty multipart

Read up to 4096 bytes per file (ISO-8859-1, max 25 files) and fire the
server.request.body.files_content callback after the filenames callback, only
when the request has not already been blocked.

- ParameterCollector: add readContent() via reflection on getInputStream(),
  getContents() getter, and MAX_FILES_TO_INSPECT guard
- ParsePartsInstrumentation: dispatch requestFilesContent() after
  requestFilesFilenames() when t == null
- HttpPostRequestDecoderInstrumentation: collect file content inline in the
  getBodyHttpDatas() loop (in-memory via ByteBuf, on-disk via FileInputStream)
  and dispatch requestFilesContent() when thr == null after filenames
- Add ParameterCollectorImplTest covering truncation, 25-file cap, form-field
  skip, IOException fallback, and ISO-8859-1 round-trip

fix(appsec): address techdebt in Tomcat/Netty files_content instrumentation

Issue 1 — Netty: extract MAX_CONTENT_BYTES=4096 and MAX_FILES_TO_INSPECT=25 as
named constants in ParseBodyAdvice, replacing raw magic literals.

Issue 3 — Tomcat + Netty: call effectivelyBlocked() after tryCommitBlockingResponse
in both the filenames and files_content blocking paths, matching the behaviour
already present in CommonsFileUploadAppSecInstrumentation.

Issue 4 — Netty: collect file content for ALL FileUpload data items regardless
of whether filename is present or empty; HttpDataType.FileUpload is the
file-vs-form-field discriminator in Netty's model, so the filename-empty check
was incorrectly skipping content for unnamed file parts (inconsistent with
FileItemContentReader which only skips isFormField() items).

fix(appsec): further cleanup of Tomcat/Netty files_content instrumentation

Issue 1 — Tomcat: inspect file content even when submitted filename is empty.
getSubmittedFileName() returns null for form fields (no filename parameter) and ""
for file uploads whose client explicitly sent filename="". Null is now the only
skip condition, so file parts with an empty filename are inspected but not added
to the filenames list, matching the commons-fileupload and Netty behaviour.

Issue 2 — Netty: remove unnecessary (long) casts in Math.min; both operands are
already int so Math.min(int,int) resolves directly.

Issue 3 — Netty: replace manual try/finally on FileInputStream with
try-with-resources, consistent with FileItemContentReader style.

fix(appsec): align blocking paths with canonical pattern in Tomcat/Netty files_content

Move BlockingException assignment inside the brf != null guard so it is
only thrown when the blocking response was actually committed.  Also add
effectivelyBlocked() to the Tomcat body-processed path, and remove the
redundant if (t == null) guard there.

For the Netty body-processed path (urlencoded), effectivelyBlocked() is
intentionally omitted: tryCommitBlockingResponse() finishes the Netty
span synchronously, so calling effectivelyBlocked() afterwards would
interact with an already-closed TraceSegment.

fix(appsec): skip file content I/O in Netty when requestFilesContent callback is unregistered

Pre-fetch the requestFilesContent callback before the body-data loop and
guard ByteBuf/FileInputStream reads behind it, consistent with how
CommonsFileUploadAppSecInstrumentation already does. Also flattens the
post-loop dispatch block by one nesting level.

fix(appsec): align Netty filenames blocking and merge CommonsFileUpload loop

Move thr = new BlockingException(...) for the filenames path in Netty
inside the brf != null guard, consistent with the body-processed and
content paths in the same advice and with the Tomcat/Liberty pattern.

Merge the two FileItem iterations in CommonsFileUploadAppSecInstrumentation
into a single loop that builds both filenames and filesContent in one pass,
calling FileItemContentReader.readContent() per item instead of a separate
FileItemContentReader.readContents() sweep.

refactor(appsec): remove dead readContents() from FileItemContentReader

The bulk readContents(List<FileItem>) method is no longer called — the
advice iterates file items inline calling readContent(FileItem) per item.
Remove the method, its now-unused ArrayList/List imports, and the
corresponding test cases. Also fix redundant type witness on ArrayList
construction in CommonsFileUploadAppSecInstrumentation.

fix(appsec): lazy-init filesContent in Netty and align CommonsFileUpload guard style

Skip filesContent ArrayList allocation in Netty ParseBodyAdvice when the
requestFilesContent callback is not registered, matching the null-sentinel
pattern already used in CommonsFileUploadAppSecInstrumentation.

Replace the early-return in CommonsFileUploadAppSecInstrumentation's
filenames blocking branch with a t == null guard on the content dispatch
block, aligning control flow with ParsePartsInstrumentation and
HttpPostRequestDecoderInstrumentation.

fix(appsec): skip file content I/O in Tomcat when requestFilesContent callback is unregistered

Add an inspectContent boolean to ParameterCollectorImpl, set from the
presence of the requestFilesContent callback in ParsePartsInstrumentation
before(). When false, addPart() still collects filenames but skips the
getInputStream() read, matching the lazy-init approach already used by
the Netty and commons-fileupload integrations.

fix(appsec): move BlockingException inside brf guard in all remaining paths

The content path in Netty and both the filenames and content paths in
Tomcat were setting the BlockingException outside the if (brf != null)
block, meaning the exception would be thrown even when no blocking
response had actually been committed. Align all three with the canonical
pattern: tryCommitBlockingResponse + effectivelyBlocked + exception
assignment all inside if (brf != null).

The Netty body-processed path intentionally still omits effectivelyBlocked()
because tryCommitBlockingResponse() closes the Netty span synchronously in
the test environment, making a subsequent effectivelyBlocked() call fail.

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.

test(appsec): add Netty integration tests for multipart filenames and file content

Also fix HttpPostRequestDecoderInstrumentation to fire on PREEPILOGUE+isLastChunk
so that multipart requests delivered as a FullHttpRequest (single shot) trigger the
requestBodyProcessed/requestFilesFilenames/requestFilesContent callbacks. The
PREEPILOGUE→EPILOGUE transition requires a second parseBody() call which never
comes when the full request arrives in one shot; PREEPILOGUE+isLastChunk is
equivalent to EPILOGUE in that scenario.

refactor(appsec): extract Netty file content reading to NettyFileUploadContentReader helper

Move the in-memory/disk-backed FileUpload content reading logic out of ParseBodyAdvice
into a testable static helper class. Also fixes the partial-read risk on disk-backed
uploads (single fis.read() replaced by a loop) and broadens the catch to Exception to
handle IllegalReferenceCountException from released ByteBufs.

test(appsec): add integration tests for file content size and count limits

test(appsec): increase Netty test server aggregator limit to support large multipart payloads

fix(appsec): do not gate Netty file content callback on requestBodyProcessed subscriber

style: apply spotless formatting to HttpServerTest

fix(appsec): remove effectivelyBlocked() calls in Netty filenames and content blocking paths

fix(appsec): set t before effectivelyBlocked() in Tomcat filenames and content blocking paths

perf(appsec): skip attribute I/O in Netty multipart when body callback is absent

When only requestFilesContent is registered, avoid calling getValue()
on form attributes (which may trigger disk reads for large fields).

refactor(appsec): unify file content limits via Config for DD_APPSEC_MAX_FILE_CONTENT_BYTES/COUNT

Replace hardcoded constants MAX_CONTENT_BYTES=4096 and MAX_FILES_TO_INSPECT=25
(duplicated across commons-fileupload, Netty, and Tomcat) with two Config-backed
variables: appsec.max.file-content.bytes and appsec.max.file-content.count.

chore: register DD_APPSEC_MAX_FILE_CONTENT_BYTES/COUNT in supported-configurations.json

test(appsec): use Config values instead of hardcoded literals in ParameterCollectorImplTest

fix(appsec): move MAX_FILES_TO_INSPECT out of advice inner class to fix muzzle validation

Static fields in @RequiresRequestContext advice inner classes trigger muzzle to
treat the advice class itself as a user-classpath class; moving the constant to
NettyFileUploadContentReader (a helper) avoids the self-referential muzzle failure.

style: apply spotless formatting to HttpPostRequestDecoderInstrumentation

fix(appsec): include filenames callback in Netty multipart early-return gate

requestFilesFilenames() was consulted after data collection, so when it was
the only registered AppSec callback (no requestBodyProcessed, no
requestFilesContent) the early-return gate fired and filenames were never
published to the WAF. Fetch all three callbacks upfront and gate on all three.

fix(appsec): use per-part charset for files_content in Tomcat and Netty

Introduces MultipartContentDecoder (internal-api) to decode multipart file bytes
using the charset declared in each part's Content-Type header, with JVM-default
fallback and REPLACE on malformed input. Mirrors the approach in PR #11212 for
commons-fileupload so all three integrations share the same decoding logic.

refactor(appsec): extract shared InputStream read loop into MultipartContentDecoder

Add readInputStream(InputStream, int, String) to eliminate the identical bounded
read loop duplicated across FileItemContentReader, NettyFileUploadContentReader and
ParameterCollector. Each caller still owns its own MAX_CONTENT_BYTES constant.

Also consolidate the two volatile reflection caches in ParameterCollector.readContent
(getInputStream + getContentType) into a single Method[] entry to halve volatile reads
per invocation.

test(appsec): add charset-aware decoding tests for readInputStream, Netty and Tomcat helpers

test(appsec): add getContentType() to Tomcat test part doubles to fix reflective method resolution

perf(appsec): skip filenames list allocation in CommonsFileUpload when filenames callback is absent

refactor(appsec): extract Netty multipart data collection to NettyMultipartHelper and add unit tests

test(appsec): declare ISO-8859-1 content type on RawBytesPart to match decoder charset expectation

refactor(appsec): extract tryBlock helper to NettyMultipartHelper

Deduplicates the identical RequestBlockingAction dispatch pattern
that appeared three times in the Netty multipart advice.

test(appsec): add unit tests for NettyMultipartHelper.tryBlock

refactor(appsec): move MAX_FILES_TO_INSPECT to FileItemContentReader helper with addToContents()

The constant now lives in the helper class (listed in helperClassNames()) where it is actually
used, instead of the outer instrumentation class which is not visible from inlined advice
bytecode in the app classloader. addToContents() encapsulates the limit check alongside
the constant.

refactor(appsec): replace Map.Entry method cache with four flat volatile fields in ParameterCollector

Use separate volatile fields (cachedPartClass, cachedGetInputStream, cachedGetContentType,
cachedGetFilename) instead of Map.Entry<Class<?>, Method[]> + Map.Entry<Class<?>, Method>.
cachedPartClass is written last to safely publish the three methods per JMM volatile ordering.

test(appsec): add unit tests for FileItemContentReader.addToContents() limit behaviour

fix(appsec): preserve decoder exception when body callback does not block in Netty multipart

Unconditionally assigning thr = tryBlock(...) would null out an original
decoder exception (e.g. malformed multipart on an over-limit body) when
tryBlock returns null for a non-blocking flow. Only overwrite thr when
tryBlock returns a blocking exception.

fix(appsec): make Tomcat Part method cache atomic with an immutable holder

Four flat volatile fields have a TOCTOU race: thread A passes the
cachedPartClass check, thread B overwrites the three method fields for a
different class, and thread A invokes the wrong Method on its instance
(IllegalArgumentException is swallowed, the part is silently dropped).

Replace the four fields with a single volatile CachedMethods reference
that holds an immutable snapshot of all three methods. Readers load the
reference once into a local variable, so their snapshot stays consistent
even if another thread publishes a new entry concurrently.

Add a concurrency regression test that runs two threads simultaneously
with different concrete Part classes; it detects dropped parts caused by
the race.

fix(appsec): inject CachedMethods helper class in Tomcat ParsePartsInstrumentation

ByteBuddy only injects classes explicitly listed in helperClassNames(); nested
inner classes are not auto-discovered. Without this entry, the first addPart()
call triggers NoClassDefFoundError on CachedMethods, which is swallowed by the
broad catch in addPart(), silently dropping all filenames and file contents.

Co-authored-by: alejandro.gonzalez <alejandro.gonzalez@datadoghq.com>

Author: jandro996

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy

@@ -136,6 +136,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     ss.registerCallback(events.requestBodyDone(), callbacks.requestBodyEndCb)
     ss.registerCallback(events.requestBodyProcessed(), callbacks.requestBodyObjectCb)
     ss.registerCallback(events.requestFilesFilenames(), callbacks.requestFilesFilenamesCb)
+    ss.registerCallback(events.requestFilesContent(), callbacks.requestFilesContentCb)
     ss.registerCallback(events.responseBody(), callbacks.responseBodyObjectCb)
     ss.registerCallback(events.responseStarted(), callbacks.responseStartedCb)
     ss.registerCallback(events.responseHeader(), callbacks.responseHeaderCb)
@@ -372,6 +373,10 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     false
   }
 
+  boolean testBodyFilesContent() {
+    false
+  }
+
   boolean testBodyJson() {
     false
   }
@@ -1652,6 +1657,82 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     response.close()
   }
 
+  def 'test instrumentation gateway file upload content'() {
+    setup:
+    assumeTrue(testBodyFilesContent())
+    RequestBody fileBody = RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')
+    def body = new MultipartBody.Builder()
+    .setType(MultipartBody.FORM)
+    .addFormDataPart('file', 'test.bin', fileBody)
+    .build()
+    def httpRequest = request(BODY_MULTIPART, 'POST', body).build()
+    def response = client.newCall(httpRequest).execute()
+
+    when:
+    TEST_WRITER.waitForTraces(1)
+
+    then:
+    TEST_WRITER.get(0).any {
+      it.getTag('request.body.files_content') == '[file content]'
+    }
+
+    cleanup:
+    response.close()
+  }
+
+  def 'test instrumentation gateway file upload content truncated at max size'() {
+    setup:
+    assumeTrue(testBodyFilesContent())
+    def maxContentBytes = Config.get().getAppSecMaxFileContentBytes()
+    def body = new MultipartBody.Builder()
+    .setType(MultipartBody.FORM)
+    .addFormDataPart('file', 'large.bin',
+    RequestBody.create(MediaType.parse('application/octet-stream'), 'X' * (maxContentBytes + 500)))
+    .build()
+    def httpRequest = request(BODY_MULTIPART, 'POST', body).build()
+    def response = client.newCall(httpRequest).execute()
+
+    when:
+    TEST_WRITER.waitForTraces(1)
+
+    then:
+    TEST_WRITER.get(0).any {
+      span ->
+      span.getTag('request.body.files_content') == '[' + 'X' * maxContentBytes + ']'
+    }
+
+    cleanup:
+    response.close()
+  }
+
+  def 'test instrumentation gateway file upload content max files limit'() {
+    setup:
+    assumeTrue(testBodyFilesContent())
+    def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount()
+    def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM)
+    (1..maxFilesToInspect + 1).each {
+      i ->
+      bodyBuilder.addFormDataPart("file$i", "file${i}.bin",
+      RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_$i"))
+    }
+    def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build()
+    def response = client.newCall(httpRequest).execute()
+
+    when:
+    TEST_WRITER.waitForTraces(1)
+
+    then:
+    TEST_WRITER.get(0).any {
+      span ->
+      def tag = span.getTag('request.body.files_content') as String
+      tag?.contains("content_of_file_$maxFilesToInspect") &&
+      !tag.contains("content_of_file_${maxFilesToInspect + 1}")
+    }
+
+    cleanup:
+    response.close()
+  }
+
   def 'test instrumentation gateway json request body'() {
     setup:
     assumeTrue(testBodyJson())
@@ -2589,6 +2670,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
       boolean responseBodyTag
       Object responseBody
       List<String> uploadedFilenames
+      List<String> uploadedFilesContent
     }
 
     static final String stringOrEmpty(String string) {
@@ -2765,6 +2847,15 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
       Flow.ResultFlow.empty()
     } as BiFunction<RequestContext, List<String>, Flow<Void>>)
 
+    final BiFunction<RequestContext, List<String>, Flow<Void>> requestFilesContentCb =
+    ({
+      RequestContext rqCtxt, List<String> contents ->
+      rqCtxt.traceSegment.setTagTop('request.body.files_content', contents as String)
+      Context context = rqCtxt.getData(RequestContextSlot.APPSEC)
+      context.uploadedFilesContent = contents
+      Flow.ResultFlow.empty()
+    } as BiFunction<RequestContext, List<String>, Flow<Void>>)
+
     final BiFunction<RequestContext, Object, Flow<Void>> responseBodyObjectCb =
     ({
       RequestContext rqCtxt, Object obj ->

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

@@ -26,7 +26,6 @@
 @AutoService(InstrumenterModule.class)
 public class CommonsFileUploadAppSecInstrumentation extends InstrumenterModule.AppSec
     implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
-
   public CommonsFileUploadAppSecInstrumentation() {
     super("commons-fileupload");
   }
@@ -54,7 +53,6 @@ public void methodAdvice(MethodTransformer transformer) {
 
   @RequiresRequestContext(RequestContextSlot.APPSEC)
   public static class ParseRequestAdvice {
-
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
     static void after(
         @Advice.Return final List<FileItem> fileItems,
@@ -73,22 +71,22 @@ static void after(
         return;
       }
 
-      List<String> filenames = new ArrayList<>();
+      List<String> filenames = filenamesCallback != null ? new ArrayList<>() : null;
+      List<String> filesContent = contentCallback != null ? new ArrayList<>() : null;
       for (FileItem fileItem : fileItems) {
         if (fileItem.isFormField()) {
           continue;
         }
         String name = fileItem.getName();
-        if (name != null && !name.isEmpty()) {
+        if (filenames != null && name != null && !name.isEmpty()) {
           filenames.add(name);
         }
-      }
-      if (filenames.isEmpty() && contentCallback == null) {
-        return;
+        if (filesContent != null) {
+          FileItemContentReader.addToContents(fileItem, filesContent);
+        }
       }
 
-      // Fire filenames event
-      if (filenamesCallback != null && !filenames.isEmpty()) {
+      if (filenames != null && !filenames.isEmpty()) {
         Flow<Void> flow = filenamesCallback.apply(reqCtx, filenames);
         Flow.Action action = flow.getAction();
         if (action instanceof Flow.Action.RequestBlockingAction) {
@@ -98,28 +96,21 @@ static void after(
             brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
             t = new BlockingException("Blocked request (multipart file upload)");
             reqCtx.getTraceSegment().effectivelyBlocked();
-            return;
           }
         }
       }
 
-      // Fire content event only if not blocked
-      if (contentCallback == null) {
-        return;
-      }
-      List<String> filesContent = FileItemContentReader.readContents(fileItems);
-      if (filesContent.isEmpty()) {
-        return;
-      }
-      Flow<Void> contentFlow = contentCallback.apply(reqCtx, filesContent);
-      Flow.Action contentAction = contentFlow.getAction();
-      if (contentAction instanceof Flow.Action.RequestBlockingAction) {
-        Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction;
-        BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
-        if (brf != null) {
-          brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
-          t = new BlockingException("Blocked request (multipart file upload content)");
-          reqCtx.getTraceSegment().effectivelyBlocked();
+      if (t == null && filesContent != null && !filesContent.isEmpty()) {
+        Flow<Void> contentFlow = contentCallback.apply(reqCtx, filesContent);
+        Flow.Action contentAction = contentFlow.getAction();
+        if (contentAction instanceof Flow.Action.RequestBlockingAction) {
+          Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction;
+          BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
+          if (brf != null) {
+            brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
+            t = new BlockingException("Blocked request (multipart file upload content)");
+            reqCtx.getTraceSegment().effectivelyBlocked();
+          }
         }
       }
     }

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

@@ -1,45 +1,31 @@
 package datadog.trace.instrumentation.commons.fileupload;
 
+import datadog.trace.api.Config;
 import datadog.trace.api.http.MultipartContentDecoder;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
 import java.util.List;
 import org.apache.commons.fileupload.FileItem;
 
 /** Reads uploaded file content for WAF inspection. */
 public final class FileItemContentReader {
-  public static final int MAX_CONTENT_BYTES = 4096;
-  public static final int MAX_FILES_TO_INSPECT = 25;
-
-  public static List<String> readContents(List<FileItem> fileItems) {
-    List<String> result = new ArrayList<>();
-    for (FileItem fileItem : fileItems) {
-      if (result.size() >= MAX_FILES_TO_INSPECT) {
-        break;
-      }
-      if (fileItem.isFormField()) {
-        continue;
-      }
-      result.add(readContent(fileItem));
-    }
-    return result;
-  }
+  public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes();
+  public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount();
 
   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 MultipartContentDecoder.decodeBytes(buf, total, fileItem.getContentType());
+      return MultipartContentDecoder.readInputStream(
+          is, MAX_CONTENT_BYTES, fileItem.getContentType());
     } catch (IOException ignored) {
       return "";
     }
   }
 
+  public static void addToContents(FileItem fileItem, List<String> contents) {
+    if (contents.size() < MAX_FILES_TO_INSPECT) {
+      contents.add(readContent(fileItem));
+    }
+  }
+
   private FileItemContentReader() {}
 }

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

@@ -48,61 +48,41 @@ class FileItemContentReaderTest extends Specification {
     FileItemContentReader.readContent(item) == text
   }
 
-  void 'readContents returns content for each non-form file with a name'() {
+  void 'addToContents adds content when below MAX_FILES_TO_INSPECT'() {
     given:
-    def items = [fileItem('content-a', 'file-a.txt'), fileItem('content-b', 'file-b.txt'),]
+    def contents = []
+    def item = fileItem('hello')
 
     when:
-    def result = FileItemContentReader.readContents(items)
+    FileItemContentReader.addToContents(item, contents)
 
     then:
-    result == ['content-a', 'content-b']
+    contents == ['hello']
   }
 
-  void 'readContents skips form fields'() {
+  void 'addToContents does not add when at MAX_FILES_TO_INSPECT limit'() {
     given:
-    FileItem formField = Stub(FileItem)
-    formField.isFormField() >> true
-    def items = [formField, fileItem('content', 'real.txt')]
+    def contents = (1..FileItemContentReader.MAX_FILES_TO_INSPECT).collect { "content-${it}" }
+    def item = fileItem('extra')
 
     when:
-    def result = FileItemContentReader.readContents(items)
+    FileItemContentReader.addToContents(item, contents)
 
     then:
-    result == ['content']
+    contents.size() == FileItemContentReader.MAX_FILES_TO_INSPECT
   }
 
-  void 'readContents includes file parts with empty or null name'() {
+  void 'addToContents fills up to exactly MAX_FILES_TO_INSPECT'() {
     given:
-    def items = [
-      fileItem('content-no-name', null),
-      fileItem('content-empty-name', ''),
-      fileItem('content-named', 'named.txt'),
-    ]
+    def contents = (1..<FileItemContentReader.MAX_FILES_TO_INSPECT).collect { "content-${it}" }
+    def item = fileItem('last')
 
     when:
-    def result = FileItemContentReader.readContents(items)
+    FileItemContentReader.addToContents(item, contents)
 
     then:
-    result == ['content-no-name', 'content-empty-name', 'content-named']
-  }
-
-  void 'readContents stops after MAX_FILES_TO_INSPECT files'() {
-    given:
-    def items = (1..FileItemContentReader.MAX_FILES_TO_INSPECT + 1).collect {
-      fileItem("content-${it}", "file-${it}.txt")
-    }
-
-    when:
-    def result = FileItemContentReader.readContents(items)
-
-    then:
-    result.size() == FileItemContentReader.MAX_FILES_TO_INSPECT
-  }
-
-  void 'readContents returns empty list for empty input'() {
-    expect:
-    FileItemContentReader.readContents([]) == []
+    contents.size() == FileItemContentReader.MAX_FILES_TO_INSPECT
+    contents.last() == 'last'
   }
 
   private FileItem fileItem(String content) {