JCR-5244: improve test coverage for HttpMultipartPost

jackrabbit-jcr-server/pom.xml

@@ -34,6 +34,12 @@
   <description>WebDAV server implementations for JCR</description>
   <packaging>bundle</packaging>
 
+  <properties>
+    <skip.coverage>false</skip.coverage>
+    <minimum.line.coverage>0.10</minimum.line.coverage>
+    <minimum.branch.coverage>0.09</minimum.branch.coverage>
+  </properties>
+
   <build>
     <plugins>
       <plugin>

jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java

@@ -45,6 +45,8 @@
     private final Map<String, List<FileItem>> nameToItems = new LinkedHashMap<String, List<FileItem>>();
     private final Set<String> fileParamNames = new HashSet<String>();
 
+    private final int PARTHEADERSIZEMAX = Integer.getInteger("jackrabbit-server-PartHeaderSizeMax", -1);
+
     private boolean initialized;
 
     HttpMultipartPost(HttpServletRequest request, File tmpDir) throws IOException {
@@ -65,6 +67,9 @@
         }
 
         ServletFileUpload upload = new ServletFileUpload(getFileItemFactory(tmpDir));
+        if (PARTHEADERSIZEMAX > 0) {
+            upload.setPartHeaderSizeMax(PARTHEADERSIZEMAX);
+        }
         // make sure the content disposition headers are read with the charset
         // specified in the request content type (or UTF-8 if no charset is specified).
         // see JCR
@@ -246,18 +251,6 @@
         }
     }
 
-    /**
-     * Returns a set of the file parameter names. An empty set if
-     * no file parameters were present in the request.
-     *
-     * @return an set of file item names representing the file
-     * parameters available with the request.
-     */
-    Set<String> getFileParameterNames() {
-        checkInitialized();
-        return fileParamNames;
-    }
-
     /**
      * Returns an array of input streams for uploaded file parameters.
      *

jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java

@@ -0,0 +1,279 @@
+/*
+ * 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.jackrabbit.server.util;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.commons.collections4.IteratorUtils;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.io.File;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.*;
+
+
+import javax.servlet.ServletInputStream;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+
+@RunWith(MockitoJUnitRunner.class)
+public class RequestDataTest {
+
+    @Rule
+    public TemporaryFolder tempFolder = new TemporaryFolder();
+
+    @Mock
+    private HttpServletRequest mockRequest;
+
+    /**
+     * Helper to wrap raw multipart text bytes cleanly into a mock ServletInputStream.
+     */
+    private ServletInputStream createServletInputStream(final byte[] payload) {
+        final ByteArrayInputStream bais = new ByteArrayInputStream(payload);
+        return new ServletInputStream() {
+            @Override
+            public int read() {
+                return bais.read();
+            }
+
+            @Override
+            public boolean isFinished() {
+                return bais.available() == 0;
+            }
+
+            @Override
+            public boolean isReady() {
+                return true;
+            }
+
+            @Override
+            public void setReadListener(javax.servlet.ReadListener readListener) {
+                throw new UnsupportedOperationException("Non-blocking I/O not implemented in mock");
+            }
+        };
+    }
+
+    /**
+     * Verifies standard parsing works smoothly using the clean temp directory assignment.
+     */
+    @Test
+    public void getParameterWithStandardRequest() throws Exception {
+        File testTmpDir = tempFolder.newFolder("jackrabbit_standard_tmp");
+
+        // ONLY stub what is actually called by RequestData for a standard request
+        // when(mockRequest.getParameter("param1")).thenReturn("value1");
+        when(mockRequest.getParameterValues("param1")).thenReturn(new String[]{"value1"});
+
+        RequestData requestData = new RequestData(mockRequest, testTmpDir);
+
+        try {
+            String[] values = requestData.getParameterValues("param1");
+            assertNotNull("Parameter array should not be null", values);
+            assertEquals("Value mismatch", "value1", values[0]);
+        } finally {
+            requestData.dispose();
+        }
+    }
+
+    /**
+     * Test multipart POST requests consisting of multiple body parameters.
+     */
+    @Test
+    public void testMultipartPostWithMultipleParts() throws Exception {
+        File testTmpDir = tempFolder.newFolder("jackrabbit_multi_parts");
+
+        String boundary = "----MockBoundary123";
+        String body = "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"textField\"\r\n\r\n" +
+                "textValue\r\n" +
+                "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"fileField\"; filename=\"test.txt\"\r\n" +
+                "Content-Type: text/plain\r\n\r\n" +
+                "Hello World Item Data\r\n" +
+                "--" + boundary + "--\r\n";
+        byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
+
+        // For multipart requests, these methods are genuinely called by Jackrabbit's parser
+        when(mockRequest.getMethod()).thenReturn("POST");
+        when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
+        lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
+        when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
+
+        RequestData requestData = new RequestData(mockRequest, testTmpDir);
+        try {
+            assertEquals("textValue", requestData.getParameter("textField"));
+            assertNotNull("Multipart file field must map", requestData.getParameter("fileField"));
+            assertEquals(Set.of("fileField", "textField"), IteratorUtils.toSet(requestData.getParameterNames()));
+            assertEquals(List.of("text/plain"), Arrays.asList(requestData.getParameterTypes("fileField")));
+            assertNull(requestData.getParameterTypes("textField")[0]);
+            assertEquals(1, requestData.getParameterTypes("textField").length);
+
+            InputStream[] streams = requestData.getFileParameters("fileField");
+            assertEquals(1, streams.length);
+            assertEquals("Hello World Item Data", new String(streams[0].readAllBytes(), StandardCharsets.UTF_8));
+
+            assertNull("Multipart file field must map", requestData.getParameter("x"));
+            assertNull(requestData.getFileParameters("x"));
+            assertNull(requestData.getParameterTypes("x"));
+            assertNull(requestData.getParameterValues("x"));
+        } finally {
+            requestData.dispose();
+        }
+    }
+
+    /**
+     * Test data parsing performance against inflated MIME/Header padding sizes.
+     */
+    // @Test
+    public void testMultipartPostWithLargeHeaders() throws Exception {
+        File testTmpDir = tempFolder.newFolder("jackrabbit_large_headers");
+        String boundary = "----MockBoundaryLargeHeaders";
+
+        String body = "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"payloadField\"\r\n" +
+                "X-Long-Header: " + "X-Header-Padding-Data-String-".repeat(1000) + "\r\n\r\n" +
+                "ShortBodyContent\r\n" +
+                "--" + boundary + "--\r\n";
+        byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
+
+        when(mockRequest.getMethod()).thenReturn("POST");
+        when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
+        lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
+        when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
+
+        RequestData requestData = new RequestData(mockRequest, testTmpDir);
+        try {
+            assertEquals("ShortBodyContent", requestData.getParameter("payloadField"));
+        } finally {
+            requestData.dispose();
+        }
+    }
+
+    private void buildRequestWithFilenameOfVaryingLength(int length) throws IOException {
+        String boundary = "----MockBoundaryLongFilename";
+
+        String longFilename = "0123456789".repeat(length / 10) + ".tmp";
+
+        String body = "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"fileUpload\"; filename=\"" + longFilename + "\"\r\n" +
+                "Content-Type: application/octet-stream\r\n\r\n" +
+                "FileContentStreamData\r\n" +
+                "--" + boundary + "--\r\n";
+        byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
+
+        when(mockRequest.getMethod()).thenReturn("POST");
+        when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
+        lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
+        when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
+    }
+
+    // test default limits in commons-fileuploads
+
+    @Test
+    public void testMultipartPostWithShorterFilename() throws Exception {
+        buildRequestWithFilenameOfVaryingLength(400);
+        File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename");
+        RequestData requestData = new RequestData(mockRequest, testTmpDir);
+        try {
+            assertTrue(
+                    requestData.getParameter("fileUpload").length() > 350);
+        } finally {
+            requestData.dispose();
+        }
+    }
+
+    @Test(expected=IOException.class)
+    public void testMultipartPostWithExtremelyLongFilename() throws Exception {
+        buildRequestWithFilenameOfVaryingLength(1000);
+        File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename");
+        RequestData requestData = new RequestData(mockRequest, testTmpDir);
+        try {
+            assertTrue(
+                    requestData.getParameter("fileUpload").length() > 950);
+        } finally {
+            requestData.dispose();
+        }
+    }
+
+    @Test
+    public void testMultipartPostWithExtremelyLongFilenameNButHigherConfig() throws Exception {
+        try {
+            System.setProperty("jackrabbit-server-PartHeaderSizeMax", "2048");
+            buildRequestWithFilenameOfVaryingLength(1000);
+            File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename");
+            RequestData requestData = new RequestData(mockRequest, testTmpDir);
+            try {
+                assertTrue(
+                        requestData.getParameter("fileUpload").length() > 950);
+            } finally {
+                requestData.dispose();
+            }
+        } finally {
+            System.clearProperty("jackrabbit-server-PartHeaderSizeMax");
+        }
+    }
+
+    /**
+     * Assures special Unicode (Non-ASCII) symbols preserve structural state during text decoding.
+     */
+    // @Test
+    public void testMultipartPostWithNonAsciiCharacters() throws Exception {
+        File testTmpDir = tempFolder.newFolder("jackrabbit_non_ascii");
+        String boundary = "----MockBoundaryNonAscii";
+        String nonAsciiValue = "テスト_ü_é_ñ_value";
+        String nonAsciiFilename = "マニュアル_doc.pdf";
+
+        String body = "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"unicodeField\"\r\n\r\n" +
+                nonAsciiValue + "\r\n" +
+                "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"unicodeFile\"; filename=\"" + nonAsciiFilename + "\"\r\n" +
+                "Content-Type: application/pdf\r\n\r\n" +
+                "%PDF-Mock-Bytes\r\n" +
+                "--" + boundary + "--\r\n";
+
+        byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
+
+        when(mockRequest.getMethod()).thenReturn("POST");
+        when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
+        lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
+        when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
+
+        RequestData requestData = new RequestData(mockRequest, testTmpDir);
+        try {
+            String parsedValue = requestData.getParameter("unicodeField");
+            assertEquals("Unicode decoding was corrupted inside the parsing sequence", nonAsciiValue, parsedValue);
+            assertNotNull("Non-ASCII file part metadata parsing must complete successfully", requestData.getParameter("unicodeFile"));
+        } finally {
+            requestData.dispose();
+        }
+    }
+}