(struts) 05/05: WW-5632 fix(fileupload): fail fast on incompatible commons-fileupload2 API

[lu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch WW-5632-fileupload2-milestone-hardening
in repository https://gitbox.apache.org/repos/asf/struts.git

commit c3966a1cb93a4a7c48df01fc3559390c9ea745dd
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Wed Jun 10 13:32:30 2026 +0200

    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 <no...@anthropic.com>
---
 .../multipart/AbstractMultiPartRequest.java        | 51 ++++++++++++++++++++++
 .../AbstractMultiPartRequestApiCheckTest.java      | 47 ++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
index 3bb0d17d2..9d33603ad 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
@@ -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 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);
     }
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java
new file mode 100644
index 000000000..3dba48ccf
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestApiCheckTest.java
@@ -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 {
+    }
+}