sharanggupta opened a new pull request, #483: URL: https://github.com/apache/commons-fileupload/pull/483
Backport of the file count limit to the 1.x streaming API ([FILEUPLOAD-360](https://issues.apache.org/jira/browse/FILEUPLOAD-360), affects 1.5 and 1.6.0). **What** `FileUploadBase.setFileCountMax(long)` was only enforced by `parseRequest(RequestContext)`. An application that iterates with `getItemIterator(...)` (the streaming API) got no limit at all, which is the report in the JIRA ticket. On `master` this was fixed for 2.x in #481 (`FileItemInputIteratorImpl`); 1.x still has the gap. **How** `FileItemIteratorImpl` now counts the items it hands out and throws `FileCountLimitExceededException` from `hasNext()` as soon as the next item would exceed the limit. It is checked at the same three places as in #481: a plain form-data part, a file nested in a `multipart/mixed` part and a `multipart/related` part. `findNextItem()` now declares `FileUploadException` (it is only called from the constructor and `hasNext()`, which already declare it). The four copies of the "item is valid" bookkeeping are folded into one `itemValid(FileItemHeaders)` method, like #481 did, which is also where the counter is incremented. **Behaviour to be aware of** - `parseRequest` still throws `FileCountLimitExceededException` with the same `getLimit()`. Because the iterator now throws first, the exception message for `parseRequest` callers changes from `attachment` (the `ATTACHMENT` constant `parseRequest` passed as the message) to `Maximum file count 2 exceeded.` (same wording as 2.x). The old check inside `parseRequest` is now never reached; I left it in place to keep the diff small and can remove it if you prefer. - A limit of `0` now rejects the first item at `getItemIterator(...)` time, because the constructor reads the first item (same as 2.x). **Tests** (`FileUploadTest`, run for both `ServletFileUpload` and `PortletFileUpload`) - form-data, `multipart/mixed` and `multipart/related` bodies with 3 files and a limit of 2: two items are returned, then `hasNext()` throws with `getLimit() == 2`; - a body with exactly the permitted number of items is still accepted; - `parseRequest` still throws (there was no test for the limit at all in 1.x). Without the change, 6 of the 10 new test executions fail (the three iterator tests, on both implementations); with it all 97 tests pass. Verified locally with the default `mvn` goal on Java 17 (0 Checkstyle violations, RAT, Javadoc and japicmp pass) and the test suite on Java 8. Before you push a pull request, review this list: - [x] Read the [contribution guidelines](CONTRIBUTING.md) for this project. - [x] Read the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) if you use Artificial Intelligence (AI). - [x] I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Claude (Anthropic) helped draft the fix and the tests; I reviewed all of it, ran the builds and checked that the new tests fail without the change. - [x] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself. - [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
