From a18b935ed2e49817f1f91bba6572bbcab58d764e Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Fri, 5 Jun 2026 21:41:11 +0100 Subject: [PATCH 1/6] JCR-5244: improve test coverage for HttpMultipartPost - remove unused code --- .../jackrabbit/server/util/HttpMultipartPost.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java index 26b1cb7c54e..3d202bc84f3 100644 --- a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java +++ b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java @@ -246,18 +246,6 @@ String[] getParameterValues(String name) { } } - /** - * 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 getFileParameterNames() { - checkInitialized(); - return fileParamNames; - } - /** * Returns an array of input streams for uploaded file parameters. * From 3b76285e8b367ba50c83423d75be376d492ff5c0 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Mon, 8 Jun 2026 19:14:48 +0100 Subject: [PATCH 2/6] JCR-5244: add minimal unit test (co-authered with Gemini) --- jackrabbit-jcr-server/pom.xml | 6 + .../server/util/RequestDataTest.java | 227 ++++++++++++++++++ 2 files changed, 233 insertions(+) create mode 100755 jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java diff --git a/jackrabbit-jcr-server/pom.xml b/jackrabbit-jcr-server/pom.xml index 29f9dc5403b..22afd85ed6b 100644 --- a/jackrabbit-jcr-server/pom.xml +++ b/jackrabbit-jcr-server/pom.xml @@ -34,6 +34,12 @@ WebDAV server implementations for JCR bundle + + false + 0.10 + 0.09 + + diff --git a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java new file mode 100755 index 00000000000..baa42c09db6 --- /dev/null +++ b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java @@ -0,0 +1,227 @@ +/* + * 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.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.mockito.Mockito.*; + + +import javax.servlet.ServletInputStream; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; + +@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")); + } 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(); + } + } + + /** + * Checks if long file naming arrays break lookahead allocation inside Jackrabbit. + */ + // @Test + public void testMultipartPostWithExtremelyLongFilename() throws Exception { + File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename"); + String boundary = "----MockBoundaryLongFilename"; + + StringBuilder longFilenameSB = new StringBuilder(); + longFilenameSB.append("verylongfilenamechunk".repeat(40)); + String longFilename = longFilenameSB + ".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)); + + RequestData requestData = new RequestData(mockRequest, testTmpDir); + try { + assertNotNull("Long filename parsing should execute without out-of-bounds corruption", + requestData.getParameter("fileUpload")); + } finally { + requestData.dispose(); + } + } + + /** + * 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(); + } + } +} \ No newline at end of file From 0702112593c27e0b46cfbf69b5ddf25800c772f4 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Tue, 9 Jun 2026 14:13:01 +0100 Subject: [PATCH 3/6] JCR-5244: improve test coverage for HttpMultipartPost --- .../apache/jackrabbit/server/util/RequestDataTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java index baa42c09db6..ee050fd6422 100755 --- a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java +++ b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java @@ -18,6 +18,7 @@ import javax.servlet.http.HttpServletRequest; +import org.apache.commons.collections4.IteratorUtils; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -29,12 +30,16 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.mockito.Mockito.*; import javax.servlet.ServletInputStream; import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; +import java.util.Set; @RunWith(MockitoJUnitRunner.class) public class RequestDataTest { @@ -123,6 +128,11 @@ public void testMultipartPostWithMultipleParts() throws Exception { try { assertEquals("textValue", requestData.getParameter("textField")); assertNotNull("Multipart file field must map", requestData.getParameter("fileField")); + assertNull("Multipart file field must map", requestData.getParameter("foobar")); + assertEquals(Set.of("fileField", "textField"), IteratorUtils.toSet(requestData.getParameterNames())); + assertEquals(List.of("text/plain"), Arrays.asList(requestData.getParameterTypes("fileField"))); + assertEquals(null, requestData.getParameterTypes("textField")[0]); + assertEquals(1, requestData.getParameterTypes("textField").length); } finally { requestData.dispose(); } From c8df14e0702f07be4c51c67d6e7e9bc3ed3f1be2 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Tue, 9 Jun 2026 14:30:25 +0100 Subject: [PATCH 4/6] JCR-5244: improve test coverage for HttpMultipartPost --- .../jackrabbit/server/util/RequestDataTest.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java index ee050fd6422..684e1f1bc8c 100755 --- a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java +++ b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java @@ -36,6 +36,7 @@ import javax.servlet.ServletInputStream; import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -128,11 +129,19 @@ public void testMultipartPostWithMultipleParts() throws Exception { try { assertEquals("textValue", requestData.getParameter("textField")); assertNotNull("Multipart file field must map", requestData.getParameter("fileField")); - assertNull("Multipart file field must map", requestData.getParameter("foobar")); assertEquals(Set.of("fileField", "textField"), IteratorUtils.toSet(requestData.getParameterNames())); assertEquals(List.of("text/plain"), Arrays.asList(requestData.getParameterTypes("fileField"))); - assertEquals(null, requestData.getParameterTypes("textField")[0]); + 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(); } @@ -174,9 +183,7 @@ public void testMultipartPostWithExtremelyLongFilename() throws Exception { File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename"); String boundary = "----MockBoundaryLongFilename"; - StringBuilder longFilenameSB = new StringBuilder(); - longFilenameSB.append("verylongfilenamechunk".repeat(40)); - String longFilename = longFilenameSB + ".tmp"; + String longFilename = "verylongfilenamechunk".repeat(40) + ".tmp"; String body = "--" + boundary + "\r\n" + "Content-Disposition: form-data; name=\"fileUpload\"; filename=\"" + longFilename + "\"\r\n" + From 97834e6dcfb8e845b53520f8a7add706cb7fa05d Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Wed, 10 Jun 2026 18:52:52 +0100 Subject: [PATCH 5/6] JCR-5244: improve test coverage for HttpMultipartPost - check fileupload default limits --- .../server/util/RequestDataTest.java | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java index 684e1f1bc8c..1bae82c0be1 100755 --- a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java +++ b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java @@ -31,11 +31,13 @@ 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; @@ -175,15 +177,10 @@ public void testMultipartPostWithLargeHeaders() throws Exception { } } - /** - * Checks if long file naming arrays break lookahead allocation inside Jackrabbit. - */ - // @Test - public void testMultipartPostWithExtremelyLongFilename() throws Exception { - File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename"); + private void buildRequestWithFilenameOfVaryingLength(int length) throws IOException { String boundary = "----MockBoundaryLongFilename"; - String longFilename = "verylongfilenamechunk".repeat(40) + ".tmp"; + String longFilename = "0123456789".repeat(length / 10) + ".tmp"; String body = "--" + boundary + "\r\n" + "Content-Disposition: form-data; name=\"fileUpload\"; filename=\"" + longFilename + "\"\r\n" + @@ -196,11 +193,31 @@ public void testMultipartPostWithExtremelyLongFilename() throws Exception { 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 { - assertNotNull("Long filename parsing should execute without out-of-bounds corruption", - requestData.getParameter("fileUpload")); + assertTrue( + requestData.getParameter("fileUpload").length() > 950); } finally { requestData.dispose(); } From dd86d2095de206fc8c18b7f01264efecc63c989b Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Thu, 11 Jun 2026 12:53:50 +0100 Subject: [PATCH 6/6] JCR-5244: improve test coverage for HttpMultipartPost - check fileupload default limits - temp. system property, but setter deprecated --- .../server/util/HttpMultipartPost.java | 5 +++++ .../server/util/RequestDataTest.java | 20 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java index 3d202bc84f3..ac9c44b2930 100644 --- a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java +++ b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/util/HttpMultipartPost.java @@ -45,6 +45,8 @@ class HttpMultipartPost { private final Map> nameToItems = new LinkedHashMap>(); private final Set fileParamNames = new HashSet(); + private final int PARTHEADERSIZEMAX = Integer.getInteger("jackrabbit-server-PartHeaderSizeMax", -1); + private boolean initialized; HttpMultipartPost(HttpServletRequest request, File tmpDir) throws IOException { @@ -65,6 +67,9 @@ private void extractMultipart(HttpServletRequest request, File tmpDir) } 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 diff --git a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java index 1bae82c0be1..bb86153d1f2 100755 --- a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java +++ b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/server/util/RequestDataTest.java @@ -210,7 +210,7 @@ public void testMultipartPostWithShorterFilename() throws Exception { } } - @Test(expected = IOException.class) + @Test(expected=IOException.class) public void testMultipartPostWithExtremelyLongFilename() throws Exception { buildRequestWithFilenameOfVaryingLength(1000); File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename"); @@ -223,6 +223,24 @@ public void testMultipartPostWithExtremelyLongFilename() throws Exception { } } + @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. */