Skip to content

Commit 4ee36dd

Browse files
reschkereschke
authored
JCR-5244: improve test coverage for HttpMultipartPost (#356)
1 parent 8d63452 commit 4ee36dd

3 files changed

Lines changed: 290 additions & 12 deletions

File tree

‎jackrabbit-jcr-server/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@
3434
<description>WebDAV server implementations for JCR</description>
3535
<packaging>bundle</packaging>
3636

37+
<properties>
38+
<skip.coverage>false</skip.coverage>
39+
<minimum.line.coverage>0.10</minimum.line.coverage>
40+
<minimum.branch.coverage>0.09</minimum.branch.coverage>
41+
</properties>
42+
3743
<build>
3844
<plugins>
3945
<plugin>

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ class HttpMultipartPost {
4545
private final Map<String, List<FileItem>> nameToItems = new LinkedHashMap<String, List<FileItem>>();
4646
private final Set<String> fileParamNames = new HashSet<String>();
4747

48+
private final int PARTHEADERSIZEMAX = Integer.getInteger("jackrabbit-server-PartHeaderSizeMax", -1);
49+
4850
private boolean initialized;
4951

5052
HttpMultipartPost(HttpServletRequest request, File tmpDir) throws IOException {
@@ -65,6 +67,9 @@ private void extractMultipart(HttpServletRequest request, File tmpDir)
6567
}
6668

6769
ServletFileUpload upload = new ServletFileUpload(getFileItemFactory(tmpDir));
70+
if (PARTHEADERSIZEMAX > 0) {
71+
upload.setPartHeaderSizeMax(PARTHEADERSIZEMAX);
72+
}
6873
// make sure the content disposition headers are read with the charset
6974
// specified in the request content type (or UTF-8 if no charset is specified).
7075
// see JCR
@@ -246,18 +251,6 @@ String[] getParameterValues(String name) {
246251
}
247252
}
248253

249-
/**
250-
* Returns a set of the file parameter names. An empty set if
251-
* no file parameters were present in the request.
252-
*
253-
* @return an set of file item names representing the file
254-
* parameters available with the request.
255-
*/
256-
Set<String> getFileParameterNames() {
257-
checkInitialized();
258-
return fileParamNames;
259-
}
260-
261254
/**
262255
* Returns an array of input streams for uploaded file parameters.
263256
*

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

Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,279 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.jackrabbit.server.util;
18+
19+
import javax.servlet.http.HttpServletRequest;
20+
21+
import org.apache.commons.collections4.IteratorUtils;
22+
import org.junit.Rule;
23+
import org.junit.Test;
24+
import org.junit.rules.TemporaryFolder;
25+
import org.junit.runner.RunWith;
26+
import org.mockito.Mock;
27+
import org.mockito.junit.MockitoJUnitRunner;
28+
29+
import java.io.File;
30+
31+
import static org.junit.Assert.assertEquals;
32+
import static org.junit.Assert.assertNotNull;
33+
import static org.junit.Assert.assertNull;
34+
import static org.junit.Assert.assertTrue;
35+
import static org.mockito.Mockito.*;
36+
37+
38+
import javax.servlet.ServletInputStream;
39+
import java.io.ByteArrayInputStream;
40+
import java.io.IOException;
41+
import java.io.InputStream;
42+
import java.nio.charset.StandardCharsets;
43+
import java.util.Arrays;
44+
import java.util.List;
45+
import java.util.Set;
46+
47+
@RunWith(MockitoJUnitRunner.class)
48+
public class RequestDataTest {
49+
50+
@Rule
51+
public TemporaryFolder tempFolder = new TemporaryFolder();
52+
53+
@Mock
54+
private HttpServletRequest mockRequest;
55+
56+
/**
57+
* Helper to wrap raw multipart text bytes cleanly into a mock ServletInputStream.
58+
*/
59+
private ServletInputStream createServletInputStream(final byte[] payload) {
60+
final ByteArrayInputStream bais = new ByteArrayInputStream(payload);
61+
return new ServletInputStream() {
62+
@Override
63+
public int read() {
64+
return bais.read();
65+
}
66+
67+
@Override
68+
public boolean isFinished() {
69+
return bais.available() == 0;
70+
}
71+
72+
@Override
73+
public boolean isReady() {
74+
return true;
75+
}
76+
77+
@Override
78+
public void setReadListener(javax.servlet.ReadListener readListener) {
79+
throw new UnsupportedOperationException("Non-blocking I/O not implemented in mock");
80+
}
81+
};
82+
}
83+
84+
/**
85+
* Verifies standard parsing works smoothly using the clean temp directory assignment.
86+
*/
87+
@Test
88+
public void getParameterWithStandardRequest() throws Exception {
89+
File testTmpDir = tempFolder.newFolder("jackrabbit_standard_tmp");
90+
91+
// ONLY stub what is actually called by RequestData for a standard request
92+
// when(mockRequest.getParameter("param1")).thenReturn("value1");
93+
when(mockRequest.getParameterValues("param1")).thenReturn(new String[]{"value1"});
94+
95+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
96+
97+
try {
98+
String[] values = requestData.getParameterValues("param1");
99+
assertNotNull("Parameter array should not be null", values);
100+
assertEquals("Value mismatch", "value1", values[0]);
101+
} finally {
102+
requestData.dispose();
103+
}
104+
}
105+
106+
/**
107+
* Test multipart POST requests consisting of multiple body parameters.
108+
*/
109+
@Test
110+
public void testMultipartPostWithMultipleParts() throws Exception {
111+
File testTmpDir = tempFolder.newFolder("jackrabbit_multi_parts");
112+
113+
String boundary = "----MockBoundary123";
114+
String body = "--" + boundary + "\r\n" +
115+
"Content-Disposition: form-data; name=\"textField\"\r\n\r\n" +
116+
"textValue\r\n" +
117+
"--" + boundary + "\r\n" +
118+
"Content-Disposition: form-data; name=\"fileField\"; filename=\"test.txt\"\r\n" +
119+
"Content-Type: text/plain\r\n\r\n" +
120+
"Hello World Item Data\r\n" +
121+
"--" + boundary + "--\r\n";
122+
byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
123+
124+
// For multipart requests, these methods are genuinely called by Jackrabbit's parser
125+
when(mockRequest.getMethod()).thenReturn("POST");
126+
when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
127+
lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
128+
when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
129+
130+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
131+
try {
132+
assertEquals("textValue", requestData.getParameter("textField"));
133+
assertNotNull("Multipart file field must map", requestData.getParameter("fileField"));
134+
assertEquals(Set.of("fileField", "textField"), IteratorUtils.toSet(requestData.getParameterNames()));
135+
assertEquals(List.of("text/plain"), Arrays.asList(requestData.getParameterTypes("fileField")));
136+
assertNull(requestData.getParameterTypes("textField")[0]);
137+
assertEquals(1, requestData.getParameterTypes("textField").length);
138+
139+
InputStream[] streams = requestData.getFileParameters("fileField");
140+
assertEquals(1, streams.length);
141+
assertEquals("Hello World Item Data", new String(streams[0].readAllBytes(), StandardCharsets.UTF_8));
142+
143+
assertNull("Multipart file field must map", requestData.getParameter("x"));
144+
assertNull(requestData.getFileParameters("x"));
145+
assertNull(requestData.getParameterTypes("x"));
146+
assertNull(requestData.getParameterValues("x"));
147+
} finally {
148+
requestData.dispose();
149+
}
150+
}
151+
152+
/**
153+
* Test data parsing performance against inflated MIME/Header padding sizes.
154+
*/
155+
// @Test
156+
public void testMultipartPostWithLargeHeaders() throws Exception {
157+
File testTmpDir = tempFolder.newFolder("jackrabbit_large_headers");
158+
String boundary = "----MockBoundaryLargeHeaders";
159+
160+
String body = "--" + boundary + "\r\n" +
161+
"Content-Disposition: form-data; name=\"payloadField\"\r\n" +
162+
"X-Long-Header: " + "X-Header-Padding-Data-String-".repeat(1000) + "\r\n\r\n" +
163+
"ShortBodyContent\r\n" +
164+
"--" + boundary + "--\r\n";
165+
byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
166+
167+
when(mockRequest.getMethod()).thenReturn("POST");
168+
when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
169+
lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
170+
when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
171+
172+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
173+
try {
174+
assertEquals("ShortBodyContent", requestData.getParameter("payloadField"));
175+
} finally {
176+
requestData.dispose();
177+
}
178+
}
179+
180+
private void buildRequestWithFilenameOfVaryingLength(int length) throws IOException {
181+
String boundary = "----MockBoundaryLongFilename";
182+
183+
String longFilename = "0123456789".repeat(length / 10) + ".tmp";
184+
185+
String body = "--" + boundary + "\r\n" +
186+
"Content-Disposition: form-data; name=\"fileUpload\"; filename=\"" + longFilename + "\"\r\n" +
187+
"Content-Type: application/octet-stream\r\n\r\n" +
188+
"FileContentStreamData\r\n" +
189+
"--" + boundary + "--\r\n";
190+
byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
191+
192+
when(mockRequest.getMethod()).thenReturn("POST");
193+
when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
194+
lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
195+
when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
196+
}
197+
198+
// test default limits in commons-fileuploads
199+
200+
@Test
201+
public void testMultipartPostWithShorterFilename() throws Exception {
202+
buildRequestWithFilenameOfVaryingLength(400);
203+
File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename");
204+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
205+
try {
206+
assertTrue(
207+
requestData.getParameter("fileUpload").length() > 350);
208+
} finally {
209+
requestData.dispose();
210+
}
211+
}
212+
213+
@Test(expected=IOException.class)
214+
public void testMultipartPostWithExtremelyLongFilename() throws Exception {
215+
buildRequestWithFilenameOfVaryingLength(1000);
216+
File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename");
217+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
218+
try {
219+
assertTrue(
220+
requestData.getParameter("fileUpload").length() > 950);
221+
} finally {
222+
requestData.dispose();
223+
}
224+
}
225+
226+
@Test
227+
public void testMultipartPostWithExtremelyLongFilenameNButHigherConfig() throws Exception {
228+
try {
229+
System.setProperty("jackrabbit-server-PartHeaderSizeMax", "2048");
230+
buildRequestWithFilenameOfVaryingLength(1000);
231+
File testTmpDir = tempFolder.newFolder("jackrabbit_long_filename");
232+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
233+
try {
234+
assertTrue(
235+
requestData.getParameter("fileUpload").length() > 950);
236+
} finally {
237+
requestData.dispose();
238+
}
239+
} finally {
240+
System.clearProperty("jackrabbit-server-PartHeaderSizeMax");
241+
}
242+
}
243+
244+
/**
245+
* Assures special Unicode (Non-ASCII) symbols preserve structural state during text decoding.
246+
*/
247+
// @Test
248+
public void testMultipartPostWithNonAsciiCharacters() throws Exception {
249+
File testTmpDir = tempFolder.newFolder("jackrabbit_non_ascii");
250+
String boundary = "----MockBoundaryNonAscii";
251+
String nonAsciiValue = "テスト_ü_é_ñ_value";
252+
String nonAsciiFilename = "マニュアル_doc.pdf";
253+
254+
String body = "--" + boundary + "\r\n" +
255+
"Content-Disposition: form-data; name=\"unicodeField\"\r\n\r\n" +
256+
nonAsciiValue + "\r\n" +
257+
"--" + boundary + "\r\n" +
258+
"Content-Disposition: form-data; name=\"unicodeFile\"; filename=\"" + nonAsciiFilename + "\"\r\n" +
259+
"Content-Type: application/pdf\r\n\r\n" +
260+
"%PDF-Mock-Bytes\r\n" +
261+
"--" + boundary + "--\r\n";
262+
263+
byte[] payloadBytes = body.getBytes(StandardCharsets.UTF_8);
264+
265+
when(mockRequest.getMethod()).thenReturn("POST");
266+
when(mockRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
267+
lenient().when(mockRequest.getCharacterEncoding()).thenReturn("UTF-8");
268+
when(mockRequest.getInputStream()).thenReturn(createServletInputStream(payloadBytes));
269+
270+
RequestData requestData = new RequestData(mockRequest, testTmpDir);
271+
try {
272+
String parsedValue = requestData.getParameter("unicodeField");
273+
assertEquals("Unicode decoding was corrupted inside the parsing sequence", nonAsciiValue, parsedValue);
274+
assertNotNull("Non-ASCII file part metadata parsing must complete successfully", requestData.getParameter("unicodeFile"));
275+
} finally {
276+
requestData.dispose();
277+
}
278+
}
279+
}

0 commit comments

Comments
 (0)