Apache Struts: File leak in multipart request processing causes disk exhaustion (DoS) - version ranges fixed

@@ -0,0 +1,160 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Apache Struts 2 Framework (Version 6.7.x)
+
+This is the Apache Struts 2 web framework, a free open-source solution for creating Java web applications. The codebase is a multi-module Maven project with comprehensive plugin architecture.
+
+## Build System & Common Commands
+
+### Maven Commands
+```bash
+# Build entire project
+./mvnw clean install
+
+# Build without running tests
+./mvnw clean install -DskipTests
+
+# Build without assembly
+./mvnw clean install -DskipAssembly
+
+# Run all tests
+./mvnw clean test
+
+# Run tests with coverage
+./mvnw clean verify -Pcoverage -DskipAssembly
+
+# Run integration tests
+./mvnw clean verify -DskipAssembly
+
+# Test specific module
+./mvnw -pl core clean test
+./mvnw -pl plugins/spring clean test
+
+# Check for security vulnerabilities
+./mvnw clean verify -Pdependency-check
+
+# Run Apache RAT license check
+./mvnw clean prepare-package
+```
+
+### Test Framework
+- **Primary**: JUnit 4.13.2 with Maven Surefire Plugin 3.5.1
+- **Pattern**: `**/*Test.java` (excludes `**/TestBean.java`)
+- **Coverage**: JaCoCo 0.8.12
+- **Additional**: Mockito, EasyMock, AssertJ, Spring Test
+- **Integration**: Maven Failsafe Plugin with Jetty on port 8090
+
+## Project Architecture
+
+### Core Structure
+```
+struts6/
+├── core/                    # Core Struts 2 framework (main dependency)
+├── plugins/                 # Plugin modules
+│   ├── spring/             # Spring integration
+│   ├── json/               # JSON support
+│   ├── tiles/              # Apache Tiles integration
+│   ├── velocity/           # Velocity template engine
+│   └── [20+ other plugins]
+├── apps/                   # Sample applications
+│   ├── showcase/           # Feature demonstration app
+│   └── rest-showcase/      # REST API examples
+├── bundles/                # OSGi bundles
+├── bom/                    # Bill of Materials
+└── assembly/               # Distribution packaging
+```
+
+### Key Technologies
+- **Java**: Minimum JDK 8, supports up to JDK 21
+- **Servlet API**: 3.1+ required
+- **Expression Language**: OGNL 3.3.5
+- **Dependency Injection**: Custom container (`com.opensymphony.xwork2.inject`)
+- **Templating**: FreeMarker 2.3.33 (default), Velocity, JSP
+- **Logging**: SLF4J 2.0.16 with Log4j2 2.24.1
+- **Build**: Maven with wrapper (3.9.6)
+
+### Core Components Architecture
+
+#### Action Framework (MVC Pattern)
+- **Actions**: Located in `core/src/main/java/org/apache/struts2/action/`
+- **Action Support**: `ActionSupport` base class with validation and i18n
+- **Action Context**: `ActionContext` provides access to servlet objects
+- **Action Invocation**: `DefaultActionInvocation` handles action execution
+
+#### Configuration System
+- **XML-based**: Primary configuration via `struts.xml` files
+- **Annotation-based**: Convention plugin for zero-config approach  
+- **Java-based**: `StrutsJavaConfiguration` for programmatic setup
+- **Property files**: `struts.properties` for framework settings
+
+#### Interceptor Chain
+- **Framework Core**: All requests processed through interceptor chain
+- **Built-in Interceptors**: 20+ interceptors in `org.apache.struts2.interceptor`
+- **Validation**: `ValidationInterceptor` with annotation support
+- **File Upload**: `FileUploadInterceptor` with security controls
+- **Parameters**: `ParametersInterceptor` with OGNL expression handling
+
+#### Result Framework  
+- **Result Types**: JSP, FreeMarker, Redirect, Stream, JSON, etc.
+- **Chaining**: `ActionChainResult` for action-to-action calls
+- **Templates**: Pluggable result renderers
+
+#### Value Stack (OGNL Integration)
+- **Expression Language**: OGNL for property access and method calls
+- **Security**: `SecurityMemberAccess` prevents dangerous operations
+- **Performance**: Caffeine-based expression caching
+- **Context**: CompoundRoot provides hierarchical value resolution
+
+### Plugin Architecture
+Each plugin is a separate Maven module with:
+- **Plugin Descriptor**: `struts-plugin.xml` defines beans and configuration
+- **Dependency Isolation**: Separate classloaders for plugin resources
+- **Extension Points**: Configurable via dependency injection
+- **Popular Plugins**: Spring (DI), JSON (REST), Tiles (Layout), Bean Validation (JSR-303)
+
+### Security Architecture
+- **OGNL Security**: Restricted method access and class loading
+- **CSRF Protection**: Token-based with `TokenInterceptor`
+- **File Upload Security**: Type and size restrictions  
+- **Content Security Policy**: Built-in CSP support
+- **Input Validation**: Server-side validation framework
+- **Pattern Matching**: Configurable allowed/excluded patterns
+
+## Development Guidelines
+
+### Code Organization
+- **Package Structure**: Follow existing `org.apache.struts2.*` hierarchy
+- **Naming Conventions**: Use Struts conventions (Actions end with `Action`)
+- **Configuration**: Prefer XML configuration in `struts.xml` for complex setups
+- **Testing**: Each module has comprehensive unit and integration tests
+
+### Plugin Development
+```java
+// Plugin descriptor example (struts-plugin.xml)
+<bean type="com.opensymphony.xwork2.ObjectFactory" 
+      name="myObjectFactory" 
+      class="com.example.MyObjectFactory" />
+```
+
+### Common Patterns
+- **Action Implementation**: Extend `ActionSupport` or implement `Action`
+- **Result Mapping**: Use result configuration in `struts.xml`
+- **Interceptor Development**: Extend `AbstractInterceptor`
+- **Type Conversion**: Implement `TypeConverter` for custom types
+- **Validation**: Use validation XML or annotations
+
+### Important Notes
+- **Version**: Currently 6.7.5-SNAPSHOT (release branch: `release/struts-6-7-x`)
+- **Java Compatibility**: Compiled for Java 8, tested through Java 21
+- **Security**: Always validate inputs and follow OWASP guidelines
+- **Performance**: Leverage built-in caching (OGNL expressions, templates)
+- **Deprecation**: Some legacy XWork components marked for removal
+
+### Build Profiles
+- **coverage**: Enables JaCoCo coverage reporting
+- **dependency-check**: OWASP dependency vulnerability scanning  
+- **jdk17**: Special configuration for Java 17+ module system
+
+This is a mature, enterprise-grade framework with extensive documentation at https://struts.apache.org/ and active community support through Apache mailing lists and JIRA (project WW).
\ No newline at end of file

@@ -42,7 +42,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import static org.apache.commons.lang3.StringUtils.normalizeSpace;
 
@@ -59,6 +58,9 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
     // maps parameter name -> List of param values
     protected Map<String, List<String>> params = new HashMap<>();
 
+    // List to track all FileItem instances for comprehensive cleanup
+    protected List<FileItem> allFileItems = new ArrayList<>();
+
     /**
      * Creates a new request wrapper to handle multi-part data using methods adapted from Jason Pell's
      * multipart classes (see class description).
@@ -103,6 +105,10 @@ protected void processUpload(HttpServletRequest request, String saveDir) throws
         if (ServletFileUpload.isMultipartContent(request)) {
             for (FileItem item : parseRequest(request, saveDir)) {
                 LOG.debug("Found file item: [{}]", normalizeSpace(item.getFieldName()));
+                
+                // Track all FileItem instances for comprehensive cleanup
+                allFileItems.add(item);
+                
                 if (item.isFormField()) {
                     processNormalFormField(item, request.getCharacterEncoding());
                 } else {
@@ -240,7 +246,11 @@ public UploadedFile[] getFile(String fieldName) {
             // Ensure file exists even if it is empty.
             if (diskFileItem.getSize() == 0 && storeLocation != null && !storeLocation.exists()) {
                 try {
-                    storeLocation.createNewFile();
+                    if (storeLocation.createNewFile()) {
+                        LOG.debug("File {} has been created", storeLocation.getAbsolutePath());
+                    } else {
+                        LOG.warn("File {} already exists", storeLocation.getAbsolutePath());
+                    }
                 } catch (IOException e) {
                     LOG.error("Cannot write uploaded empty file to disk: {}", storeLocation.getAbsolutePath(), e);
                 }
@@ -357,15 +367,31 @@ public InputStream getInputStream() throws IOException {
      * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
      */
     public void cleanUp() {
-        Set<String> names = files.keySet();
-        for (String name : names) {
-            List<FileItem> items = files.get(name);
-            for (FileItem item : items) {
-                LOG.debug("Removing file [{}]", normalizeSpace(name));
-                if (!item.isInMemory()) {
-                    item.delete();
+        try {
+            LOG.debug("Performing comprehensive cleanup for {} file items.", allFileItems.size());
+            for (FileItem item : allFileItems) {
+                try {
+                    if (item instanceof DiskFileItem) {
+                        DiskFileItem diskItem = (DiskFileItem) item;
+                        File storeLocation = diskItem.getStoreLocation();
+                        if (storeLocation != null && storeLocation.exists()) {
+                            LOG.debug("Deleting temporary file: [{}]", storeLocation.getName());
+                            if (!storeLocation.delete()) {
+                                LOG.warn("Unable to delete temporary file: [{}]", storeLocation.getName());
+                            }
+                        }
+                    }
+                    // Also call the item's delete method as backup
+                    if (!item.isInMemory()) {
+                        item.delete();
+                    }
+                } catch (Exception e) {
+                    LOG.warn("Error during cleanup of file item: [{}]", normalizeSpace(item.getFieldName()), e);
                 }
             }
+        } finally {
+            // Clear only the tracking collection, preserve parsed data
+            allFileItems.clear();
         }
     }
 

@@ -0,0 +1,183 @@
+/*
+ * 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.fileupload.FileItem;
+import org.apache.struts2.StrutsInternalTestCase;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+/**
+ * Test cases for {@link JakartaMultiPartRequest} that verify security-related functionality,
+ * specifically comprehensive cleanup of temporary files.
+ */
+public class JakartaMultiPartRequestTest extends StrutsInternalTestCase {
+
+    private File tempDir;
+    private JakartaMultiPartRequest multiPartRequest;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        
+        // Create a temporary directory for test files
+        Path tempPath = Files.createTempDirectory("struts-multipart-test");
+        tempDir = tempPath.toFile();
+        
+        multiPartRequest = new TestableJakartaMultiPartRequest();
+        multiPartRequest.setMaxSize("2048");
+        multiPartRequest.setMaxFiles("10");
+        multiPartRequest.setMaxFileSize("1024");
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        if (tempDir != null && tempDir.exists()) {
+            // Clean up test directory
+            File[] files = tempDir.listFiles();
+            if (files != null) {
+                for (File file : files) {
+                    file.delete();
+                }
+            }
+            tempDir.delete();
+        }
+        super.tearDown();
+    }
+
+    /**
+     * Test that comprehensive cleanup removes all temporary files created during multipart processing.
+     * This addresses the security vulnerability where temporary files could be leaked.
+     */
+    public void testComprehensiveCleanupRemovesAllTempFiles() throws Exception {
+        // Create a mock multipart request with both file upload and form field
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setContentType("multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW");
+        request.setMethod("POST");
+        
+        String multipartContent = createMultipartContent();
+        request.setContent(multipartContent.getBytes(StandardCharsets.UTF_8));
+
+        // Count files before processing
+        int filesBefore = countTempFiles();
+
+        // Process the multipart request
+        multiPartRequest.parse(request, tempDir.getAbsolutePath());
+
+        // Count files after processing (should be more due to temp files)
+        int filesAfterProcessing = countTempFiles();
+
+        // Verify that temp files were created during processing
+        assertTrue("Temporary files should be created during multipart processing", 
+                   filesAfterProcessing > filesBefore);
+
+        // Perform cleanup
+        multiPartRequest.cleanUp();
+
+        // Count files after cleanup
+        int filesAfterCleanup = countTempFiles();
+
+        // Verify comprehensive cleanup removed all temporary files
+        assertEquals("All temporary files should be cleaned up", filesBefore, filesAfterCleanup);
+    }
+
+    /**
+     * Test that all FileItem instances are tracked for cleanup, including both file uploads and form fields.
+     */
+    public void testAllFileItemsAreTrackedForCleanup() throws Exception {
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setContentType("multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW");
+        request.setMethod("POST");
+        
+        String multipartContent = createMultipartContentWithMultipleFields();
+        request.setContent(multipartContent.getBytes(StandardCharsets.UTF_8));
+
+        // Process the multipart request
+        multiPartRequest.parse(request, tempDir.getAbsolutePath());
+
+        // Access the tracked items through our testable implementation
+        TestableJakartaMultiPartRequest testable = (TestableJakartaMultiPartRequest) multiPartRequest;
+        List<FileItem> trackedItems = testable.getAllFileItems();
+
+        // Verify that all items (both files and form fields) are tracked
+        assertTrue("Should track multiple FileItem instances", trackedItems.size() >= 3);
+        
+        // Verify tracking includes both form fields and file uploads
+        boolean hasFormField = false;
+        boolean hasFileUpload = false;
+        
+        for (FileItem item : trackedItems) {
+            if (item.isFormField()) {
+                hasFormField = true;
+            } else {
+                hasFileUpload = true;
+            }
+        }
+        
+        assertTrue("Should track form field items", hasFormField);
+        assertTrue("Should track file upload items", hasFileUpload);
+    }
+
+    private String createMultipartContent() {
+        return "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\n" +
+               "Content-Disposition: form-data; name=\"textField\"\r\n\r\n" +
+               "test value\r\n" +
+               "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\n" +
+               "Content-Disposition: form-data; name=\"fileField\"; filename=\"test.txt\"\r\n" +
+               "Content-Type: text/plain\r\n\r\n" +
+               "file content\r\n" +
+               "------WebKitFormBoundary7MA4YWxkTrZu0gW--\r\n";
+    }
+
+    private String createMultipartContentWithMultipleFields() {
+        return "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\n" +
+               "Content-Disposition: form-data; name=\"textField1\"\r\n\r\n" +
+               "value1\r\n" +
+               "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\n" +
+               "Content-Disposition: form-data; name=\"textField2\"\r\n\r\n" +
+               "value2\r\n" +
+               "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\n" +
+               "Content-Disposition: form-data; name=\"fileField\"; filename=\"test.txt\"\r\n" +
+               "Content-Type: text/plain\r\n\r\n" +
+               "file content\r\n" +
+               "------WebKitFormBoundary7MA4YWxkTrZu0gW--\r\n";
+    }
+
+    private int countTempFiles() {
+        if (tempDir == null || !tempDir.exists()) {
+            return 0;
+        }
+        File[] files = tempDir.listFiles();
+        return files != null ? files.length : 0;
+    }
+
+    /**
+     * Testable subclass that exposes internal state for verification
+     */
+    private static class TestableJakartaMultiPartRequest extends JakartaMultiPartRequest {
+        public List<FileItem> getAllFileItems() {
+            return allFileItems;
+        }
+    }
+}
\ No newline at end of file

@@ -1,5 +1,5 @@
 # IDEA
-.idea
+.idea/
 *.iml
 *.ipr
 *.iws
@@ -38,11 +38,14 @@ buildNumber.properties
 .mvn/timing.properties
 .mvn/wrapper/maven-wrapper.jar
 
-plugins/testng/test-output
-test-output
+plugins/testng/test-output/
+test-output/
 
 # Sonar
-/.sonar/
+.sonar/
 
 # Tidelift CLI scanner
 .tidelift
+
+# Claude Code specific local settings
+.claude/