Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-05 14:47:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

WW-5632 Harden commons-fileupload2 dependency against milestone binary-incompatibility (#1735)

Browse Source 
* WW-5632 docs: add commons-fileupload2 milestone-hardening design spec

Design for hardening the commons-fileupload2 dependency against
milestone binary-incompatibility (manage -core, activate a scoped
enforcer rule, add a runtime API guard in AbstractMultiPartRequest).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* WW-5632 docs: add implementation plan for fileupload2 milestone hardening

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* WW-5632 build(deps): manage commons-fileupload2-core alongside jakarta-servlet6

Pin both commons-fileupload2 artifacts to a single
commons-fileupload2.version property so the volatile -core API can no
longer skew from -jakarta-servlet6 in the reactor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* WW-5632 build: enforce a single commons-fileupload2 version

Activate maven-enforcer-plugin (previously dormant in pluginManagement)
with a fileupload-scoped bannedDependencies rule so any divergent
commons-fileupload2 version fails the build early.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* WW-5632 fix(fileupload): fail fast on incompatible commons-fileupload2 API

Verify once per JVM that the fileupload size-limit setters exist and
throw a clear StrutsException reporting the core/jakarta version skew,
replacing an opaque deep-stack NoSuchMethodError in downstream runtimes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* WW-5632 fix(fileupload): make API-verification guard static

Resolve Sonar java:S2696 (instance method writing a static field) by
making ensureFileUploadApiVerified() static; verification is JVM-global.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Lukasz Lenart
2026-06-13 08:28:47 +02:00
committed by GitHub
parent 210dc86b88
commit 4f3fd69aa6
6 changed files with 718 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+51
View File
@@ -19,6 +19,7 @@
package org.apache.struts2.dispatcher.multipart;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.fileupload2.core.AbstractFileUpload;
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException;
import org.apache.commons.fileupload2.core.FileUploadContentTypeException;
@@ -32,6 +33,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.StrutsException;
import org.apache.struts2.dispatcher.LocalizedMessage;
import org.apache.struts2.inject.Inject;
@@ -60,6 +62,12 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
private static final Logger LOG = LogManager.getLogger(AbstractMultiPartRequest.class);
/**
* Verified once per JVM: whether the commons-fileupload2 API on the classpath matches what
* Struts compiled against. Guards against a mismatched milestone resolving at runtime.
*/
private static volatile boolean fileUploadApiVerified;
/**
* Defines the internal buffer size used during streaming operations.
*/
@@ -211,6 +219,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
}
protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset, Path saveDir) {
ensureFileUploadApiVerified();
JakartaServletDiskFileUpload servletFileUpload = createJakartaFileUpload(charset, saveDir);
if (maxSize != null) {
@@ -228,6 +237,48 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
return servletFileUpload;
}
/**
* Verifies once per JVM that the commons-fileupload2 API on the classpath matches what Struts
* compiled against, failing fast with an actionable message instead of a deep-stack
* {@link NoSuchMethodError} when a mismatched milestone is resolved.
*/
private static void ensureFileUploadApiVerified() {
if (!fileUploadApiVerified) {
verifyFileUploadApi(JakartaServletDiskFileUpload.class);
fileUploadApiVerified = true;
}
}
/**
* Probes {@code uploadClass} for the size-limit setters Struts invokes in
* {@link #prepareServletFileUpload}. Package-private for testing.
*
* @param uploadClass the file upload class to verify
* @throws StrutsException if any required method is absent, indicating a binary-incompatible
* commons-fileupload2 version on the classpath
*/
static void verifyFileUploadApi(Class<?> uploadClass) {
for (String method : new String[]{"setMaxSize", "setMaxFileCount", "setMaxFileSize"}) {
try {
uploadClass.getMethod(method, long.class);
} catch (NoSuchMethodException e) {
throw new StrutsException(String.format(
"Incompatible Apache Commons FileUpload on the classpath: %s.%s(long) is missing. " +
"Detected commons-fileupload2-core version [%s] and commons-fileupload2-jakarta-servlet6 version [%s]. " +
"Align commons-fileupload2-core with commons-fileupload2-jakarta-servlet6 (use the same release for both).",
uploadClass.getName(), method,
implementationVersion(AbstractFileUpload.class),
implementationVersion(uploadClass)), e);
}
}
}
private static String implementationVersion(Class<?> clazz) {
Package pkg = clazz.getPackage();
String version = pkg != null ? pkg.getImplementationVersion() : null;
return version != null ? version : "unknown";
}
protected RequestContext createRequestContext(HttpServletRequest request) {
return new StrutsRequestContext(request);
}
core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java
+47
View File
@@ -0,0 +1,47 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.dispatcher.multipart;
import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload;
import org.apache.struts2.StrutsException;
import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
public class AbstractMultiPartRequestApiCheckTest {
@Test
public void verifyFileUploadApiPassesForCompatibleClass() {
assertThatCode(() -> AbstractMultiPartRequest.verifyFileUploadApi(JakartaServletDiskFileUpload.class))
.doesNotThrowAnyException();
}
@Test
public void verifyFileUploadApiThrowsForIncompatibleClass() {
assertThatThrownBy(() -> AbstractMultiPartRequest.verifyFileUploadApi(IncompatibleFileUpload.class))
.isInstanceOf(StrutsException.class)
.hasMessageContaining("setMaxSize")
.hasMessageContaining("Align commons-fileupload2-core");
}
/** Stub lacking the size-limit setters, simulating a binary-incompatible fileupload version. */
private static class IncompatibleFileUpload {
}
}
docs/superpowers/plans/2026-06-10-WW-5632-fileupload2-milestone-hardening.md
+392
View File
@@ -0,0 +1,392 @@
# commons-fileupload2 Milestone Hardening Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Make a `commons-fileupload2-core` / `-jakarta-servlet6` version skew impossible in Struts's own build and turn a future runtime `NoSuchMethodError` into a clear, actionable `StrutsException`.
**Architecture:** Three independent changes. (A1) Introduce a single `commons-fileupload2.version` property and manage *both* fileupload artifacts in `parent/pom.xml`. (A2) Activate the dormant `maven-enforcer-plugin` with a fileupload-scoped `bannedDependencies` rule. (B) Add a once-per-JVM reflective API guard in `AbstractMultiPartRequest`.
**Tech Stack:** Maven (multi-module), `maven-enforcer-plugin` 3.6.3, Java 17, JUnit 4 + AssertJ (the `core` module's established test stack), Apache Commons FileUpload 2.0.0-M5.
**Ticket:** [WW-5632](https://issues.apache.org/jira/browse/WW-5632)
**Spec:** `docs/superpowers/specs/2026-06-10-fileupload2-milestone-hardening-design.md`
**Branch:** `WW-5632-fileupload2-milestone-hardening` (already checked out)
---
## File Structure
- `pom.xml` (root) — add the `commons-fileupload2.version` property; change the enforcer rule from `dependencyConvergence` to a scoped `bannedDependencies`; bind the enforcer into the active `<plugins>` section.
- `parent/pom.xml` — reference the new property for `commons-fileupload2-jakarta-servlet6` and add a managed entry for `commons-fileupload2-core`.
- `core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java` — add the runtime API guard and call it from `prepareServletFileUpload`.
- `core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java` (new) — unit tests for the guard.
---
## Task 1: Manage both fileupload artifacts via a single version property (A1)
**Files:**
- Modify: `pom.xml:118-119` (properties block)
- Modify: `parent/pom.xml:128-132` (dependencyManagement entry)
- [ ] **Step 1: Add the version property to the root POM**
In `pom.xml`, inside the `<properties>` block, add the property in alphabetical order between `byte-buddy.version` (line 118) and `freemarker.version` (line 119):
```xml
<byte-buddy.version>1.18.8</byte-buddy.version>
<commons-fileupload2.version>2.0.0-M5</commons-fileupload2.version>
<freemarker.version>2.3.34</freemarker.version>
```
- [ ] **Step 2: Reference the property and add the `-core` managed entry**
In `parent/pom.xml`, replace the existing single `commons-fileupload2-jakarta-servlet6` management entry (currently lines 128-132):
```xml
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-fileupload2-jakarta-servlet6</artifactId>
<version>2.0.0-M5</version>
</dependency>
```
with two entries, both referencing the property (the volatile API lives in `-core`, so it must be pinned too):
```xml
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-fileupload2-core</artifactId>
<version>${commons-fileupload2.version}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-fileupload2-jakarta-servlet6</artifactId>
<version>${commons-fileupload2.version}</version>
</dependency>
```
- [ ] **Step 3: Verify both artifacts resolve to the pinned version**
Run:
```bash
mvn -q -pl core dependency:list -DskipAssembly '-Dincludes=org.apache.commons:commons-fileupload2*'
```
Expected: both `commons-fileupload2-core` and `commons-fileupload2-jakarta-servlet6` listed at `2.0.0-M5`.
- [ ] **Step 4: Verify the reactor still builds**
Run:
```bash
mvn -q validate -DskipAssembly
```
Expected: `BUILD SUCCESS` (no errors from the new property / managed dependency).
- [ ] **Step 5: Commit**
```bash
git add pom.xml parent/pom.xml
git commit -m "WW-5632 build(deps): manage commons-fileupload2-core alongside jakarta-servlet6
Pin both commons-fileupload2 artifacts to a single
commons-fileupload2.version property so the volatile -core API can no
longer skew from -jakarta-servlet6 in the reactor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
```
---
## Task 2: Activate a fileupload-scoped enforcer rule (A2)
**Files:**
- Modify: `pom.xml:349-353` (enforcer rule config in `<pluginManagement>`)
- Modify: `pom.xml:373-378` (active `<plugins>` section)
- [ ] **Step 1: Replace the dormant `dependencyConvergence` rule with a scoped `bannedDependencies` rule**
In `pom.xml`, inside the `maven-enforcer-plugin` execution in `<pluginManagement>`, replace the current configuration (lines 349-353):
```xml
<configuration>
<rules>
<dependencyConvergence />
</rules>
</configuration>
```
with a rule that bans all commons-fileupload2 versions except the pinned one (`<includes>` are exceptions to the `<excludes>` bans):
```xml
<configuration>
<rules>
<bannedDependencies>
<excludes>
<exclude>org.apache.commons:commons-fileupload2-core</exclude>
<exclude>org.apache.commons:commons-fileupload2-jakarta-servlet6</exclude>
</excludes>
<includes>
<include>org.apache.commons:commons-fileupload2-core:${commons-fileupload2.version}</include>
<include>org.apache.commons:commons-fileupload2-jakarta-servlet6:${commons-fileupload2.version}</include>
</includes>
</bannedDependencies>
</rules>
</configuration>
```
- [ ] **Step 2: Bind the enforcer into the active `<plugins>` section**
In `pom.xml`, inside the active `<build><plugins>` block, add the enforcer plugin entry immediately after the `maven-release-plugin` entry (after line 378; version is inherited from `<pluginManagement>`):
```xml
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
<version>3.3.1</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
</plugin>
```
- [ ] **Step 3: Verify the enforcer now executes and passes on the clean tree**
Run:
```bash
mvn -q validate -DskipAssembly
```
Expected: `BUILD SUCCESS`. To confirm the rule actually ran (not skipped), run:
```bash
mvn validate -DskipAssembly -pl core | grep -i "enforce"
```
Expected: a line showing `maven-enforcer-plugin:3.6.3:enforce (enforce)` executing.
- [ ] **Step 4: Verify the rule catches a skew (manual negative check, then revert)**
Temporarily edit `parent/pom.xml` to set the `commons-fileupload2-core` managed version to a different value (e.g. `2.0.0-M4` instead of `${commons-fileupload2.version}`), then run:
```bash
mvn validate -DskipAssembly -pl core
```
Expected: `BUILD FAILURE` with a `bannedDependencies` violation naming `commons-fileupload2-core`.
Then revert the edit:
```bash
git checkout -- parent/pom.xml
```
- [ ] **Step 5: Commit**
```bash
git add pom.xml
git commit -m "WW-5632 build: enforce a single commons-fileupload2 version
Activate maven-enforcer-plugin (previously dormant in pluginManagement)
with a fileupload-scoped bannedDependencies rule so any divergent
commons-fileupload2 version fails the build early.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
```
---
## Task 3: Runtime API guard in AbstractMultiPartRequest (B)
**Files:**
- Test: `core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java` (create)
- Modify: `core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java` (imports ~line 22 & 34; new method block near `prepareServletFileUpload` at line 213; call site at line 214)
- [ ] **Step 1: Write the failing test**
Create `core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java`:
```java
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.dispatcher.multipart;
import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload;
import org.apache.struts2.StrutsException;
import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
public class AbstractMultiPartRequestApiCheckTest {
@Test
public void verifyFileUploadApiPassesForCompatibleClass() {
assertThatCode(() -> AbstractMultiPartRequest.verifyFileUploadApi(JakartaServletDiskFileUpload.class))
.doesNotThrowAnyException();
}
@Test
public void verifyFileUploadApiThrowsForIncompatibleClass() {
assertThatThrownBy(() -> AbstractMultiPartRequest.verifyFileUploadApi(IncompatibleFileUpload.class))
.isInstanceOf(StrutsException.class)
.hasMessageContaining("setMaxSize")
.hasMessageContaining("Align commons-fileupload2-core");
}
/** Stub lacking the size-limit setters, simulating a binary-incompatible fileupload version. */
private static class IncompatibleFileUpload {
}
}
```
- [ ] **Step 2: Run the test to verify it fails**
Run:
```bash
mvn test -DskipAssembly -pl core -Dtest=AbstractMultiPartRequestApiCheckTest
```
Expected: FAIL — compilation error `cannot find symbol: method verifyFileUploadApi(java.lang.Class)` (the guard does not exist yet). This is the red state.
- [ ] **Step 3: Add the two imports**
In `AbstractMultiPartRequest.java`, add the `-core` `AbstractFileUpload` import alongside the existing `fileupload2.core` imports (after line 22, `import org.apache.commons.fileupload2.core.DiskFileItemFactory;` — keep alphabetical, so `AbstractFileUpload` goes *before* it):
```java
import org.apache.commons.fileupload2.core.AbstractFileUpload;
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
```
And add the `StrutsException` import after the existing `StrutsConstants` import (line 34):
```java
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.StrutsException;
```
- [ ] **Step 4: Implement the guard and wire it into `prepareServletFileUpload`**
In `AbstractMultiPartRequest.java`, add `ensureFileUploadApiVerified();` as the first statement of `prepareServletFileUpload` (currently line 213-214):
```java
protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset, Path saveDir) {
ensureFileUploadApiVerified();
JakartaServletDiskFileUpload servletFileUpload = createJakartaFileUpload(charset, saveDir);
```
Then add the following members. Place the field next to the other static members (e.g. directly after the `LOG` field at line 61), and the three methods directly after the `prepareServletFileUpload` method (after its closing brace at line 229):
Field (after line 61):
```java
/**
* Verified once per JVM: whether the commons-fileupload2 API on the classpath matches what
* Struts compiled against. Guards against a mismatched milestone resolving at runtime.
*/
private static volatile boolean fileUploadApiVerified;
```
Methods (after `prepareServletFileUpload`):
```java
/**
* Verifies once per JVM that the commons-fileupload2 API on the classpath matches what Struts
* compiled against, failing fast with an actionable message instead of a deep-stack
* {@link NoSuchMethodError} when a mismatched milestone is resolved.
*/
private void ensureFileUploadApiVerified() {
if (!fileUploadApiVerified) {
verifyFileUploadApi(JakartaServletDiskFileUpload.class);
fileUploadApiVerified = true;
}
}
/**
* Probes {@code uploadClass} for the size-limit setters Struts invokes in
* {@link #prepareServletFileUpload}. Package-private for testing.
*
* @param uploadClass the file upload class to verify
* @throws StrutsException if any required method is absent, indicating a binary-incompatible
* commons-fileupload2 version on the classpath
*/
static void verifyFileUploadApi(Class<?> uploadClass) {
for (String method : new String[]{"setMaxSize", "setMaxFileCount", "setMaxFileSize"}) {
try {
uploadClass.getMethod(method, long.class);
} catch (NoSuchMethodException e) {
throw new StrutsException(String.format(
"Incompatible Apache Commons FileUpload on the classpath: %s.%s(long) is missing. " +
"Detected commons-fileupload2-core version [%s] and commons-fileupload2-jakarta-servlet6 version [%s]. " +
"Align commons-fileupload2-core with commons-fileupload2-jakarta-servlet6 (use the same release for both).",
uploadClass.getName(), method,
implementationVersion(AbstractFileUpload.class),
implementationVersion(uploadClass)), e);
}
}
}
private static String implementationVersion(Class<?> clazz) {
Package pkg = clazz.getPackage();
String version = pkg != null ? pkg.getImplementationVersion() : null;
return version != null ? version : "unknown";
}
```
- [ ] **Step 5: Run the test to verify it passes**
Run:
```bash
mvn test -DskipAssembly -pl core -Dtest=AbstractMultiPartRequestApiCheckTest
```
Expected: PASS — both tests green.
- [ ] **Step 6: Run the multipart regression tests**
Run:
```bash
mvn test -DskipAssembly -pl core -Dtest='*MultiPartRequest*'
```
Expected: PASS — `JakartaMultiPartRequestTest` and `JakartaStreamMultiPartRequestTest` still green (the guard runs once and does not change upload behavior).
- [ ] **Step 7: Commit**
```bash
git add core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java \
core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java
git commit -m "WW-5632 fix(fileupload): fail fast on incompatible commons-fileupload2 API
Verify once per JVM that the fileupload size-limit setters exist and
throw a clear StrutsException reporting the core/jakarta version skew,
replacing an opaque deep-stack NoSuchMethodError in downstream runtimes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
```
---
## Final Verification
- [ ] **Run the full core test suite**
Run:
```bash
mvn test -DskipAssembly -pl core
```
Expected: `BUILD SUCCESS`, all tests pass, enforcer rule executed during `validate`.
- [ ] **Confirm the working tree is clean and the branch holds three commits**
Run:
```bash
git status --short && git log --oneline -3
```
Expected: no uncommitted changes; the three WW-5632 commits on top of the spec commit.
docs/superpowers/specs/2026-06-10-fileupload2-milestone-hardening-design.md
+206
View File
@@ -0,0 +1,206 @@
# Design: Harden commons-fileupload2 against milestone churn
**Date:** 2026-06-10
**Status:** Approved design — pending implementation plan
**Ticket:** [WW-5632](https://issues.apache.org/jira/browse/WW-5632)
**Origin:** [user@struts mailing list thread](https://lists.apache.org/thread/fcdls8xvd9tp9o6dcog65vkqozv4nq5x)
(Tamás Barta, Struts 7.1.1 file upload `NoSuchMethodError`)
**Related (closed):** [WW-5615](https://issues.apache.org/jira/browse/WW-5615) — "Adapt to renamed
methods in Apache Commons FileUpload 2.0.0-M5", fixed in 7.2.0 via PR #1584 / commit `d2810d42f`.
## Context
A user on Struts 7.1.1 reported `java.lang.NoSuchMethodError:
'void org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload.setSizeMax(long)'`
at upload time. Apache Commons FileUpload 2.0.0-M5 renamed several `AbstractFileUpload` methods
(`setSizeMax`→`setMaxSize`, `setFileSizeMax`→`setMaxFileSize`, `setFileCountMax`→`setMaxFileCount`),
breaking binary compatibility with M4. Struts declared M4 but the user's build resolved M5.
WW-5615 (PR #1584) fixed the **symptom** for 7.2.0: it renamed the three call sites in
`AbstractMultiPartRequest.java` and bumped `commons-fileupload2-jakarta-servlet6` M4 → M5 in
`parent/pom.xml`. That commit did **nothing else**.
This design addresses the **class of failure** that WW-5615 left open.
## Root-cause chain (verified on current `main`, 7.2.0-SNAPSHOT)
1. **Milestone dependency.** Struts depends on `-M` builds of commons-fileupload2, which break
binary compatibility between milestones. Until a 2.0.0 GA exists, Struts is committed to
milestone artifacts.
2. **The volatile API lives in an unmanaged artifact.** `setMaxSize(long)` / `setMaxFileCount(long)`
/ `setMaxFileSize(long)` are declared on `org.apache.commons.fileupload2.core.AbstractFileUpload`
in **`commons-fileupload2-core`** (verified via `javap`). `JakartaServletDiskFileUpload` merely
inherits them. `parent/pom.xml` `<dependencyManagement>` pins only
`commons-fileupload2-jakarta-servlet6` — **`commons-fileupload2-core` is unmanaged.** A transitive
dependency pulling a different `-core` milestone reproduces the exact `NoSuchMethodError` even
when `-jakarta-servlet6` is pinned correctly.
3. **The build guardrail is dormant.** `maven-enforcer-plugin` is configured with a
`dependencyConvergence` rule, but **only inside `<pluginManagement>`** of the root `pom.xml`; it is
never bound to an active `<plugins>` section, so it never executes. Struts's own build would not
catch a fileupload version skew.
4. **The BOM does not help consumers.** `struts2-bom` exports only Struts module versions, not the
fileupload version. Downstream apps importing the BOM get no convergence assistance.
Net effect: a downstream/transitive dependency can select a mismatched `commons-fileupload2-core`
milestone, and because milestones break binary compatibility, the user gets a runtime
`NoSuchMethodError` deep in request handling, with no build-time warning.
## Goals
- Make a `commons-fileupload2-core` / `-jakarta-servlet6` version skew **impossible within Struts's
own build**, deterministically.
- Fail the Struts build **early and clearly** if a future transitive dependency wants a
commons-fileupload2 version other than the tested one.
- For downstream consumer runtimes (where Struts's build guards cannot reach), replace the opaque
deep-stack `NoSuchMethodError` with a **clear, actionable `StrutsException`**.
## Non-goals
- **Shading/relocating commons-fileupload2.** Rejected: the library is security-sensitive (CVE
history); shading would force Struts to re-release on every fileupload CVE, against Apache norms,
and bloats the artifact.
- **Exporting the fileupload version through `struts2-bom`.** Considered and deferred — out of scope
for this change.
- **Migrating off milestone versions.** Not actionable until a commons-fileupload2 2.0.0 GA exists.
## Design
### Part A — Build-time fail-fast (POM)
**A1. Manage both artifacts at one version.**
Introduce a single `commons-fileupload2.version` property (single source of truth) and add a
`<dependencyManagement>` entry for `org.apache.commons:commons-fileupload2-core` alongside the
existing `commons-fileupload2-jakarta-servlet6` entry in `parent/pom.xml`, both referencing the
property. Because `<dependencyManagement>` wins Maven version mediation, this forces a single,
matched `-core` version across the entire Struts reactor regardless of transitive requests —
closing root-cause #2 deterministically for Struts's own build.
**A2. Activate a narrowly-scoped enforcer (chosen over global `dependencyConvergence`).**
Bind `maven-enforcer-plugin` into an active `<plugins>` section with a `bannedDependencies` rule
scoped **only** to commons-fileupload2: ban all versions of
`org.apache.commons:commons-fileupload2-core` and
`org.apache.commons:commons-fileupload2-jakarta-servlet6` **except** the pinned
`${commons-fileupload2.version}`. This fails the build immediately if any transitive dependency
introduces a different fileupload version, with effectively zero blast radius on unrelated
dependencies.
> **Why not global `dependencyConvergence`?** It has never actually run; activating it may surface
> many pre-existing, unrelated version conflicts across the multi-module build, ballooning scope
> unpredictably. The fileupload-scoped `bannedDependencies` rule targets exactly the failure mode in
> this report. (Global convergence remains a reasonable separate cleanup task, out of scope here.)
The pinned version string lives once in the `commons-fileupload2.version` property and is referenced
by both the `<dependencyManagement>` entries and the enforcer rule — no duplicated literals.
### Part B — Runtime diagnostics guard
Add a one-time, package-private static check in `AbstractMultiPartRequest`, invoked on first use
(e.g. at the top of `prepareServletFileUpload`), guarded so the reflective probe runs **once** per
JVM — no per-request cost.
**Probe (testable, pure):**
`static void verifyFileUploadApi(Class<?> uploadClass)` reflectively confirms that `uploadClass`
declares (inherited included) `setMaxSize(long)`, `setMaxFileCount(long)`, and `setMaxFileSize(long)`.
If any is absent it throws `org.apache.struts2.StrutsException`.
**Self-maintaining message (no hardcoded "expected" version):** the exception reports the
implementation versions read at runtime from both packages —
`org.apache.commons.fileupload2.core.AbstractFileUpload.class.getPackage().getImplementationVersion()`
(the `-core` version) and `JakartaServletDiskFileUpload.class.getPackage().getImplementationVersion()`
(the `-jakarta-servlet6` version) — names the missing method, and instructs the user to align
`commons-fileupload2-core` with `commons-fileupload2-jakarta-servlet6`. Versions fall back to
`"unknown"` when no manifest implementation version is present. Surfacing the **skew** (core vs
jakarta versions) is the actionable signal; no version constant is baked into Struts to drift.
**One-time guard:** the caller wraps `verifyFileUploadApi(JakartaServletDiskFileUpload.class)` with a
JVM-once gate (`static volatile boolean` or a holder). The probe method itself is stateless so tests
can call it repeatedly.
## Testing & verification
**Part A:**
- `mvn validate -DskipAssembly` runs the enforcer clean on the current tree (no fileupload skew
exists today).
- Manual negative check: temporarily declare a conflicting `commons-fileupload2-core` version and
confirm the build fails with the banned-dependency message; revert.
**Part B (unit tests in `AbstractMultiPartRequestTest`):**
- `verifyFileUploadApi(JakartaServletDiskFileUpload.class)` does **not** throw (real classpath has the
M5 API).
- `verifyFileUploadApi(<stub class lacking the setters>)` throws `StrutsException`; assert the message
names the missing method and the remediation (align `-core` with `-jakarta-servlet6`).
Full suite: `mvn test -DskipAssembly -pl core`.
## Risks
- **Enforcer noise (mitigated).** Scoping `bannedDependencies` to commons-fileupload2 only avoids the
unbounded scope risk of global `dependencyConvergence`.
- **Reflective probe drift.** If a future commons-fileupload2 release renames these setters again, the
probe's hardcoded method names become a deliberate tripwire to update alongside the dependency bump
— acceptable and intended.
- **Null implementation version.** Handled via `"unknown"` fallback so the guard never NPEs while
building its diagnostic message.
## Out of scope / follow-ups
- Tracked under [WW-5632](https://issues.apache.org/jira/browse/WW-5632).
- Global `dependencyConvergence` cleanup across the reactor.
- Exporting third-party versions through `struts2-bom`.
- Revisiting the dependency once commons-fileupload2 2.0.0 GA ships.
## JIRA ticket
**Summary (title):**
```
Harden commons-fileupload2 dependency against milestone binary-incompatibility
```
**Description (JIRA wiki markup — paste into the description field):**
```
h3. Background
[WW-5615|https://issues.apache.org/jira/browse/WW-5615] fixed the {{NoSuchMethodError}}
caused by Apache Commons FileUpload 2.0.0-M5 renaming {{setSizeMax}} -> {{setMaxSize}}
(and friends), shipped in 7.2.0 via [#1584|https://github.com/apache/struts/pull/1584].
That fix addressed the *symptom* for one milestone bump but not the underlying *class of
failure*.
h3. Problem
Struts depends on _milestone_ ({{-M}}) builds of commons-fileupload2, which break binary
compatibility between milestones. Three gaps remain on {{main}}:
* The renamed setters ({{setMaxSize}}, {{setMaxFileCount}}, {{setMaxFileSize}}) live in
*{{commons-fileupload2-core}}* ({{AbstractFileUpload}}), but only
{{commons-fileupload2-jakarta-servlet6}} is pinned in {{dependencyManagement}} —
{{-core}} is unmanaged, so a transitive dependency can pull a mismatched {{-core}}
milestone and reproduce the {{NoSuchMethodError}}.
* The {{maven-enforcer-plugin}} {{dependencyConvergence}} rule sits only in
{{<pluginManagement>}} and is never bound to an active {{<plugins>}} section, so it
never runs — the build cannot catch a fileupload version skew.
* Downstream consumer runtimes get an opaque, deep-stack {{NoSuchMethodError}} with no
actionable guidance.
h3. Proposed changes
* *(A1)* Introduce a single {{commons-fileupload2.version}} property and manage *both*
{{commons-fileupload2-core}} and {{commons-fileupload2-jakarta-servlet6}} at that version
in {{parent/pom.xml}}, forcing a matched {{-core}} version across the reactor.
* *(A2)* Activate {{maven-enforcer-plugin}} with a fileupload-scoped {{bannedDependencies}}
rule that fails the build on any commons-fileupload2 version other than the pinned one
(narrow scope, near-zero blast radius).
* *(B)* Add a once-per-JVM reflective guard in {{AbstractMultiPartRequest}} that throws a
clear {{StrutsException}} reporting the {{-core}} vs {{-jakarta-servlet6}} version skew
instead of an opaque {{NoSuchMethodError}}.
Full design: {{docs/superpowers/specs/2026-06-10-fileupload2-milestone-hardening-design.md}}
h3. Affects / Fix version
* Affects: 7.1.1+ (root cause present on 7.2.0-SNAPSHOT {{main}})
* Component: File Upload
```
parent/pom.xml
+6 -1
View File
@@ -125,10 +125,15 @@
<artifactId>commons-collections4</artifactId>
<version>4.5.0</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-fileupload2-core</artifactId>
<version>${commons-fileupload2.version}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-fileupload2-jakarta-servlet6</artifactId>
<version>2.0.0-M5</version>
<version>${commons-fileupload2.version}</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>
pom.xml
+16 -1
View File
@@ -116,6 +116,7 @@
<!-- dependency versions in alphanumeric order -->
<asm.version>9.10.1</asm.version>
<byte-buddy.version>1.18.8</byte-buddy.version>
<commons-fileupload2.version>2.0.0-M5</commons-fileupload2.version>
<freemarker.version>2.3.34</freemarker.version>
<hibernate-validator.version>8.0.2.Final</hibernate-validator.version>
<jackson.version>2.21.3</jackson.version>
@@ -348,7 +349,17 @@
<id>enforce</id>
<configuration>
<rules>
<dependencyConvergence />
<bannedDependencies>
<message>commons-fileupload2 version skew detected: only ${commons-fileupload2.version} is allowed. Align all commons-fileupload2 artifacts (core and jakarta-servlet6) to the version defined by the commons-fileupload2.version property in the root POM.</message>
<excludes>
<exclude>org.apache.commons:commons-fileupload2-core</exclude>
<exclude>org.apache.commons:commons-fileupload2-jakarta-servlet6</exclude>
</excludes>
<includes>
<include>org.apache.commons:commons-fileupload2-core:${commons-fileupload2.version}</include>
<include>org.apache.commons:commons-fileupload2-jakarta-servlet6:${commons-fileupload2.version}</include>
</includes>
</bannedDependencies>
</rules>
</configuration>
<goals>
@@ -376,6 +387,10 @@
<artifactId>maven-release-plugin</artifactId>
<version>3.3.1</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
</plugin>
<plugin>
<artifactId>maven-jar-plugin</artifactId>
<configuration>