FileItemInputIteratorImpl.findNextItem() now uses the max file count.

commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/core/FileItemInputIteratorImpl.java

@@ -57,6 +57,16 @@ class FileItemInputIteratorImpl implements FileItemInputIterator {
      */
     private long maxFileSize;
 
+    /**
+     * The current number of files.
+     */
+    private long curFileCount;
+
+    /**
+     * The maximum permitted number of files that may be uploaded in a single request. A value of -1 indicates no maximum.
+     */
+    private final long maxFileCount;
+
     /**
      * The multi part stream to process.
      */
@@ -114,12 +124,19 @@ class FileItemInputIteratorImpl implements FileItemInputIterator {
         this.fileUpload = fileUpload;
         this.maxSize = fileUpload.getMaxSize();
         this.maxFileSize = fileUpload.getMaxFileSize();
+        this.maxFileCount = fileUpload.getMaxFileCount();
         this.requestContext = Objects.requireNonNull(requestContext, "requestContext");
         this.multipartRelated = this.requestContext.isMultipartRelated();
         this.skipPreamble = true;
         findNextItem();
     }
 
+    private void checkMaxFileCount() throws FileUploadFileCountLimitException {
+        if (curFileCount == maxFileCount) {
+            throw new FileUploadFileCountLimitException(String.format("Maximum file count %,d exceeded.", maxFileCount), maxFileCount, curFileCount);
+        }
+    }
+
     /**
      * Finds the next item, if any.
      *
@@ -155,13 +172,10 @@ private boolean findNextItem() throws FileUploadException, IOException {
             }
             final var headers = fileUpload.getParsedHeaders(multi.readHeaders());
             if (multipartRelated) {
+                checkMaxFileCount();
                 currentFieldName = "";
-                currentItem = new FileItemInputImpl(
-                        this, null, null, headers.getHeader(AbstractFileUpload.CONTENT_TYPE),
-                        false, getContentLength(headers));
-                currentItem.setHeaders(headers);
-                progressNotifier.noteItem();
-                itemValid = true;
+                currentItem = new FileItemInputImpl(this, null, null, headers.getHeader(AbstractFileUpload.CONTENT_TYPE), false, getContentLength(headers));
+                itemValid(headers);
                 return true;
             }
             if (currentFieldName == null) {
@@ -180,22 +194,20 @@ private boolean findNextItem() throws FileUploadException, IOException {
                         skipPreamble = true;
                         continue;
                     }
+                    checkMaxFileCount();
                     final var fileName = fileUpload.getFileName(headers);
                     currentItem = new FileItemInputImpl(this, fileName, fieldName, headers.getHeader(AbstractFileUpload.CONTENT_TYPE), fileName == null,
                             getContentLength(headers));
-                    currentItem.setHeaders(headers);
-                    progressNotifier.noteItem();
-                    itemValid = true;
+                    itemValid(headers);
                     return true;
                 }
             } else {
                 final var fileName = fileUpload.getFileName(headers);
                 if (fileName != null) {
+                    checkMaxFileCount();
                     currentItem = new FileItemInputImpl(this, fileName, currentFieldName, headers.getHeader(AbstractFileUpload.CONTENT_TYPE), false,
                             getContentLength(headers));
-                    currentItem.setHeaders(headers);
-                    progressNotifier.noteItem();
-                    itemValid = true;
+                    itemValid(headers);
                     return true;
                 }
             }
@@ -304,6 +316,13 @@ protected void init(final AbstractFileUpload<?, ?, ?> fileUpload, final RequestC
         multiPartInput.setHeaderCharset(charset);
     }
 
+    private void itemValid(final FileItemHeaders headers) {
+        currentItem.setHeaders(headers);
+        progressNotifier.noteItem();
+        itemValid = true;
+        curFileCount++;
+    }
+
     /**
      * Returns the next available {@link FileItemInput}.
      *
@@ -336,5 +355,4 @@ public Iterator<FileItemInput> unwrap() {
         // TODO Something better?
         return (Iterator<FileItemInput>) this;
     }
-
 }

commons-fileupload2-jakarta-servlet5/src/test/java/org/apache/commons/fileupload2/jakarta/servlet5/JakartaServletFileUploadGetItemIteratorTest.java

@@ -32,7 +32,10 @@
 import org.apache.commons.fileupload2.core.FileItemInputIterator;
 import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException;
 import org.apache.commons.fileupload2.core.FileUploadException;
+import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import jakarta.servlet.http.HttpServletRequest;
 
@@ -50,6 +53,36 @@ class JakartaServletFileUploadGetItemIteratorTest {
     /** Content-type header value that matches {@link #BOUNDARY}. */
     private static final String CONTENT_TYPE = "multipart/form-data; boundary=" + BOUNDARY;
 
+    /** Content-type header value for multipart/related requests that matches {@link #BOUNDARY}. */
+    private static final String CONTENT_TYPE_RELATED = "multipart/related; boundary=" + BOUNDARY;
+
+    /** Boundary value used for nested multipart/mixed parts. */
+    private static final String MIXED_BOUNDARY = "---9876";
+
+    /**
+     * Builds a complete multipart body that contains a single form-data part holding a nested multipart/mixed part with {@code fileCount} identical files.
+     *
+     * @param fileCount number of nested files to include
+     * @return raw multipart bytes encoded in US-ASCII
+     */
+    private static byte[] buildMixedFileParts(final int fileCount) {
+        final var sb = new StringBuilder();
+        sb.append("--").append(BOUNDARY).append("\r\n");
+        sb.append("Content-Disposition: form-data; name=\"files\"\r\n");
+        sb.append("Content-Type: multipart/mixed; boundary=").append(MIXED_BOUNDARY).append("\r\n");
+        sb.append("\r\n");
+        for (int i = 1; i <= fileCount; i++) {
+            sb.append("--").append(MIXED_BOUNDARY).append("\r\n");
+            sb.append("Content-Disposition: attachment; filename=\"file").append(i).append(".txt\"\r\n");
+            sb.append("Content-Type: text/plain\r\n");
+            sb.append("\r\n");
+            sb.append("Content of file ").append(i).append("\r\n");
+        }
+        sb.append("--").append(MIXED_BOUNDARY).append("--\r\n");
+        sb.append("--").append(BOUNDARY).append("--\r\n");
+        return sb.toString().getBytes(StandardCharsets.US_ASCII);
+    }
+
     /**
      * Builds a complete multipart body that contains {@code fileCount} identical file parts.
      *
@@ -69,6 +102,25 @@ private static byte[] buildMultiFileParts(final int fileCount) {
         return sb.toString().getBytes(StandardCharsets.US_ASCII);
     }
 
+    /**
+     * Builds a complete multipart/related body that contains {@code partCount} identical parts.
+     *
+     * @param partCount number of parts to include
+     * @return raw multipart bytes encoded in US-ASCII
+     */
+    private static byte[] buildMultiRelatedParts(final int partCount) {
+        final var sb = new StringBuilder();
+        for (int i = 1; i <= partCount; i++) {
+            sb.append("--").append(BOUNDARY).append("\r\n");
+            sb.append("Content-Type: text/plain\r\n");
+            sb.append("Content-ID: <part").append(i).append("@example.org>\r\n");
+            sb.append("\r\n");
+            sb.append("Content of part ").append(i).append("\r\n");
+        }
+        sb.append("--").append(BOUNDARY).append("--\r\n");
+        return sb.toString().getBytes(StandardCharsets.US_ASCII);
+    }
+
     /**
      * Builds a complete multipart body that contains exactly one file part.
      *
@@ -126,6 +178,28 @@ void testEmptyMultipartBody() throws Exception {
         assertFalse(iter.hasNext(), "Expected no items in an empty multipart body");
     }
 
+    /**
+     * When the number of uploaded files exceeds {@code maxFileCount}, iterating must throw a {@link FileUploadFileCountLimitException} with the expected
+     * permitted count. Tested for maxFileCount values of 1, 2, 4, and 8.
+     */
+    @ParameterizedTest
+    @ValueSource(longs = { 1, 2, 4, 8 })
+    void testFileCountLimitExceededThrowsException(final long maxFileCount) throws Exception {
+        final var body = buildMultiFileParts((int) maxFileCount + 1);
+        final HttpServletRequest request = new JakartaMockHttpServletRequest(body, CONTENT_TYPE);
+        final var upload = newUpload();
+        upload.setMaxFileCount(maxFileCount);
+        final FileItemInputIterator iter = upload.getItemIterator(request);
+        // Consume allowed items first
+        for (int i = 0; i < maxFileCount; i++) {
+            assertTrue(iter.hasNext());
+            iter.next();
+        }
+        // The next hasNext() call triggers the limit check
+        final var ex = assertThrows(FileUploadFileCountLimitException.class, iter::hasNext);
+        assertEquals(maxFileCount, ex.getPermitted());
+    }
+
     /**
      * A file part: the content read from the item's InputStream must match the uploaded body.
      */
@@ -328,6 +402,50 @@ void testMixedFormFieldAndFileParts() throws Exception {
         assertFalse(iter.hasNext());
     }
 
+    /**
+     * When the number of files in a nested multipart/mixed part exceeds {@code maxFileCount}, iterating must throw a {@link FileUploadFileCountLimitException}
+     * with the expected permitted count. Tested for maxFileCount values of 1, 2, 4, and 8.
+     */
+    @ParameterizedTest
+    @ValueSource(longs = { 1, 2, 4, 8 })
+    void testMultipartMixedFileCountLimitExceededThrowsException(final long maxFileCount) throws Exception {
+        final var body = buildMixedFileParts((int) maxFileCount + 1);
+        final HttpServletRequest request = new JakartaMockHttpServletRequest(body, CONTENT_TYPE);
+        final var upload = newUpload();
+        upload.setMaxFileCount(maxFileCount);
+        final FileItemInputIterator iter = upload.getItemIterator(request);
+        // Consume allowed items first
+        for (int i = 0; i < maxFileCount; i++) {
+            assertTrue(iter.hasNext());
+            iter.next();
+        }
+        // The next hasNext() call triggers the limit check
+        final var ex = assertThrows(FileUploadFileCountLimitException.class, iter::hasNext);
+        assertEquals(maxFileCount, ex.getPermitted());
+    }
+
+    /**
+     * When the number of parts in a multipart/related request exceeds {@code maxFileCount}, iterating must throw a {@link FileUploadFileCountLimitException}
+     * with the expected permitted count. Tested for maxFileCount values of 1, 2, 4, and 8.
+     */
+    @ParameterizedTest
+    @ValueSource(longs = { 1, 2, 4, 8 })
+    void testMultipartRelatedFileCountLimitExceededThrowsException(final long maxFileCount) throws Exception {
+        final var body = buildMultiRelatedParts((int) maxFileCount + 1);
+        final HttpServletRequest request = new JakartaMockHttpServletRequest(body, CONTENT_TYPE_RELATED);
+        final var upload = newUpload();
+        upload.setMaxFileCount(maxFileCount);
+        final FileItemInputIterator iter = upload.getItemIterator(request);
+        // Consume allowed items first
+        for (int i = 0; i < maxFileCount; i++) {
+            assertTrue(iter.hasNext());
+            iter.next();
+        }
+        // The next hasNext() call triggers the limit check
+        final var ex = assertThrows(FileUploadFileCountLimitException.class, iter::hasNext);
+        assertEquals(maxFileCount, ex.getPermitted());
+    }
+
     /**
      * Multiple parts: the iterator must return every part in transmission order.
      */