From 62a8f0ee4527f88a7e5bcb9d15712072203533f9 Mon Sep 17 00:00:00 2001 From: Zibran Khan Date: Sun, 8 Nov 2026 00:33:04 +0530 Subject: [PATCH] spill multipart uploads into the servlet context temp dir --- .../builder/MultipartFormDataBuilder.java | 23 +++++++--- .../builder/MultipartFormDataBuilderTest.java | 44 ++++++++++++++++++- .../org/apache/axis2/webapp/AdminActions.java | 11 ++++- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/modules/kernel/src/org/apache/axis2/builder/MultipartFormDataBuilder.java b/modules/kernel/src/org/apache/axis2/builder/MultipartFormDataBuilder.java index 262e354c73..154ba58190 100644 --- a/modules/kernel/src/org/apache/axis2/builder/MultipartFormDataBuilder.java +++ b/modules/kernel/src/org/apache/axis2/builder/MultipartFormDataBuilder.java @@ -19,6 +19,7 @@ package org.apache.axis2.builder; +import java.io.File; import java.io.InputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -28,6 +29,7 @@ import jakarta.activation.DataHandler; import jakarta.activation.DataSource; +import jakarta.servlet.ServletContext; import jakarta.servlet.http.HttpServletRequest; import org.apache.axiom.om.OMAbstractFactory; @@ -94,7 +96,7 @@ private MultipleEntryHashMap getParameterMap(HttpServletRequest request, MultipleEntryHashMap parameterMap = new MultipleEntryHashMap(); - List items = parseRequest(new JakartaServletRequestContext(request), messageContext); + List items = parseRequest(request, messageContext); Iterator iter = items.iterator(); while (iter.hasNext()) { DiskFileItem diskFileItem = (DiskFileItem)iter.next(); @@ -144,16 +146,27 @@ private static void deleteQuietly(DiskFileItem diskFileItem) { } } - private static List parseRequest(JakartaServletRequestContext requestContext, + private static List parseRequest(HttpServletRequest request, MessageContext messageContext) throws FileUploadException { // Create a factory for disk-based file items. Parts above the factory's // threshold spill to a temp file; register them so the file goes away // once the item that owns it is unreachable. - DiskFileItemFactory fileItemFactory = DiskFileItemFactory.builder() + DiskFileItemFactory.Builder factoryBuilder = DiskFileItemFactory.builder() .setCharset(StandardCharsets.UTF_8) - .setFileCleaningTracker(MultipartTempFileTracker.getTracker()) - .get(); + .setFileCleaningTracker(MultipartTempFileTracker.getTracker()); + // commons-fileupload2 otherwise spills into java.io.tmpdir, creating the + // file with the process umask, so an uploaded part is readable by other + // local accounts. Use the servlet context's private temp directory, as + // the container's own multipart handling does. + ServletContext servletContext = request.getServletContext(); + Object tempDir = servletContext == null ? null + : servletContext.getAttribute(ServletContext.TEMPDIR); + if (tempDir instanceof File) { + factoryBuilder.setPath(((File) tempDir).toPath()); + } + DiskFileItemFactory fileItemFactory = factoryBuilder.get(); + JakartaServletRequestContext requestContext = new JakartaServletRequestContext(request); JakartaServletFileUpload upload = new JakartaServletFileUpload<>(fileItemFactory); // There must be a limit. // This is for contentType="multipart/form-data" diff --git a/modules/kernel/test/org/apache/axis2/builder/MultipartFormDataBuilderTest.java b/modules/kernel/test/org/apache/axis2/builder/MultipartFormDataBuilderTest.java index 3f34ceafc4..eb52d9c668 100644 --- a/modules/kernel/test/org/apache/axis2/builder/MultipartFormDataBuilderTest.java +++ b/modules/kernel/test/org/apache/axis2/builder/MultipartFormDataBuilderTest.java @@ -20,6 +20,7 @@ package org.apache.axis2.builder; import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletContext; import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.HttpServletRequest; import junit.framework.TestCase; @@ -35,6 +36,7 @@ import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -165,6 +167,42 @@ public void testFilePartTempFileIsDeletedOnceUnreachable() throws Exception { awaitTempFileCount(before)); } + /** + * A file part is spilled into the servlet context's private temp directory, + * not the shared {@code java.io.tmpdir}, where the umask-mode file would be + * readable by other local accounts. + */ + public void testFilePartIsSpilledIntoServletContextTempDir() throws Exception { + File contextTempDir = Files.createTempDirectory("axis2-context-tmp").toFile(); + try { + int sharedBefore = countUploadTempFiles(); + + MessageContext messageContext = newMessageContext(buildBody("bigFile", "big.bin")); + HttpServletRequest request = (HttpServletRequest) messageContext.getProperty( + HTTPConstants.MC_HTTP_SERVLETREQUEST); + ServletContext servletContext = mock(ServletContext.class); + when(servletContext.getAttribute(ServletContext.TEMPDIR)).thenReturn(contextTempDir); + when(request.getServletContext()).thenReturn(servletContext); + + OMElement result = new MultipartFormDataBuilder().processDocument(null, + multipartContentType(), messageContext); + assertNotNull(result); + + assertEquals("The file part must not be written to the shared temp directory", + sharedBefore, countUploadTempFiles()); + assertEquals("The file part should be in the servlet context temp directory", + 1, countUploadTempFiles(contextTempDir)); + } finally { + File[] files = contextTempDir.listFiles(); + if (files != null) { + for (File file : files) { + file.delete(); + } + } + contextTempDir.delete(); + } + } + /** * Poll for the temp-file count to fall back to the expected value, nudging * the collector each time, since the reaper only acts once the owning item @@ -182,7 +220,11 @@ private boolean awaitTempFileCount(int expected) throws InterruptedException { } private int countUploadTempFiles() { - String[] names = tempDirectory.list(); + return countUploadTempFiles(tempDirectory); + } + + private static int countUploadTempFiles(File directory) { + String[] names = directory.list(); if (names == null) { return 0; } diff --git a/modules/webapp/src/main/java/org/apache/axis2/webapp/AdminActions.java b/modules/webapp/src/main/java/org/apache/axis2/webapp/AdminActions.java index 7b2960785d..a714327f5a 100644 --- a/modules/webapp/src/main/java/org/apache/axis2/webapp/AdminActions.java +++ b/modules/webapp/src/main/java/org/apache/axis2/webapp/AdminActions.java @@ -41,6 +41,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import jakarta.servlet.ServletContext; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import javax.xml.namespace.QName; @@ -152,7 +153,15 @@ public Redirect doUpload(HttpServletRequest req) throws ServletException { boolean isMultipart = JakartaServletFileUpload.isMultipartContent(reqContext); if (isMultipart) { try { - JakartaServletFileUpload upload = new JakartaServletFileUpload<>(DiskFileItemFactory.builder().get()); + // Spill into the servlet context's private temp directory rather + // than java.io.tmpdir, where the archive would be readable by + // other local accounts while it is being uploaded. + DiskFileItemFactory.Builder factoryBuilder = DiskFileItemFactory.builder(); + Object tempDir = req.getServletContext().getAttribute(ServletContext.TEMPDIR); + if (tempDir instanceof File) { + factoryBuilder.setPath(((File) tempDir).toPath()); + } + JakartaServletFileUpload upload = new JakartaServletFileUpload<>(factoryBuilder.get()); // There must be a limit. This is for an aar file upload, // presumably only one. See: // https://axis.apache.org/axis2/java/core/docs/webadminguide.html#upservice