lukaszlenart opened a new pull request, #1805: URL: https://github.com/apache/struts/pull/1805
Fixes [WW-5413](https://issues.apache.org/jira/browse/WW-5413) ## Background The original ticket (filed against 6.3.0) was a commons-io `DeferredFileOutputStream`/`ThresholdingOutputStream` regression that made multipart uploads read as empty. **That root cause is already resolved** by the migration to commons-fileupload2 2.0.0-M5 + commons-io 2.22.0 — no threshold is forced to `-1`, so small uploads legitimately stay in memory (`DiskFileItem.isInMemory()`). What remained is the ticket's **performance** half: `JakartaMultiPartRequest.processFileField()` still took every in-memory item and eagerly wrote it to a temp file, wrapping that `File` in `StrutsUploadedFile`. The redundant filesystem write just moved from the fileupload layer into Struts' own code. ## What this changes Small in-memory uploads are no longer eagerly written to disk. A temp file is materialized **only when a caller actually asks for a `File`** — and the standard interceptor validation path no longer forces that. - **`UploadedFile.getInputStream()`** — new `default` method: read upload bytes without forcing a disk write. `default`, so third-party implementations keep compiling. `getContent()` still returns a `java.io.File` everywhere (never `byte[]`), so all existing consumers are unaffected. - **`StrutsInMemoryUploadedFile`** — a byte-array-backed `UploadedFile`. `getInputStream()` reads from memory (no disk); `getContent()`/`getAbsolutePath()` lazily write the bytes to a secure `upload_<uuid>.tmp` file exactly once and cache it; `isFile()` is false until materialized. `Serializable` (holds a `java.io.File` target, not a non-serializable `Path`) and thread-safe (`synchronized` materialize, `volatile` cache). - **`JakartaMultiPartRequest`** — builds the in-memory type for `isInMemory()` items; the eager `FileOutputStream` write, the `temporaryFiles` list, and `cleanUpTemporaryFiles()` are removed. Cleanup rides the existing `AbstractMultiPartRequest.cleanUp()` `delete()`-when-`isFile()` loop. - **`AbstractFileUploadInterceptor.acceptFile()`** — previously called `getContent()` as its first (failed-upload) check, which materialized every small upload during validation on the common `fileUpload`/`actionFileUpload` path. It now uses a new non-materializing `UploadedFile.isMissing()` (default `= getContent() == null`, overridden in `StrutsInMemoryUploadedFile` to answer from memory). A redundant dead `getContent() == null` block was removed. Net result: the `UploadedFilesAware` flow validates and hands files to the action **without ever writing a small upload to disk**; only a consumer that explicitly needs a `File` (the legacy `File`-typed action property via `UploadedFileConverter`) triggers the write. ## Backward compatibility - `getContent()` runtime type stays `java.io.File` for every implementation. - `getInputStream()` and `isMissing()` are `default` methods — third-party `UploadedFile` impls are unaffected. - Upload validation is unchanged: size / content-type / extension / missing-content rejections all still fire (verified), and the secure UUID temp-file naming (original filename never influences the on-disk name) is preserved. ## Behavior note A materialization write failure now surfaces as an unchecked `StrutsException` at the point of consumption rather than as a parse-time `LocalizedMessage` (the interface's `getContent()`/`getAbsolutePath()` cannot declare checked exceptions). This affects only the rare "cannot write to the save directory" case, and only on the legacy `File`/`getContent()` path — the `getInputStream()` fast path never writes. ## Known limitation `StrutsInMemoryUploadedFile` bakes an absolute temp path resolved on the originating node. If an un-materialized instance is serialized (e.g. session replication) and deserialized on another node, a later `getContent()` materializes to that originating node's path. Consumers needing content to survive cross-node replication should read via `getInputStream()`. Documented on the class Javadoc. ## Minor cleanup left for a follow-up `STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY` in `AbstractFileUploadInterceptor` is now unused (its only site was the removed dead block) but was left in place because it is `public static final` and removing it would be an API break. The vestigial `throws IOException` on `JakartaMultiPartRequest.processFileField` was likewise left in place to avoid breaking a subclass that catches it from `super`. ## Tests New: `UploadedFileTest`, `StrutsUploadedFileTest`, `StrutsInMemoryUploadedFileTest` (lazy materialization, no-write stream path, serialization round-trip, secure naming, `isMissing()`), a `JakartaMultiPartRequestTest` integration test proving no disk write until content is requested, and `ActionFileUploadInterceptorTest` cases proving validation does not materialize on accept or reject. Existing tests that asserted the old eager-write behavior were removed or updated. Full multipart + interceptor + converter suite: **114 tests green**. ## Design docs `docs/superpowers/specs/2026-07-22-WW-5413-inmemory-upload-optimization-design.md` and `docs/superpowers/plans/2026-07-22-WW-5413-inmemory-upload-optimization.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
