This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/clean-uploaded-files
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 831568929cfba700f790f6ebe6e335f9f33fb468
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Wed Aug 6 13:46:15 2025 +0200
Cleans up all uploaded files
---
.gitignore | 11 +-
CLAUDE.md | 160 ++++++++++++++++++
.../multipart/JakartaMultiPartRequest.java | 44 ++++-
.../multipart/JakartaMultiPartRequestTest.java | 183 +++++++++++++++++++++
4 files changed, 385 insertions(+), 13 deletions(-)
diff --git a/.gitignore b/.gitignore
index af4e6813c..4105b5462 100644
--- a/.gitignore
+++ b/.gitignore
@@ -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/
diff --git a/CLAUDE.md b/CLAUDE.md
new file mode 100644
index 000000000..3710f0354
--- /dev/null
+++ b/CLAUDE.md
@@ -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
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
index d55e35001..d27b2159d 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
@@ -42,7 +42,6 @@ import java.util.Enumeration;
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 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
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 class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
// 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 class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
* @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();
}
}
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
new file mode 100644
index 000000000..9c6abcd08
--- /dev/null
+++
b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
@@ -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
