Skip to content

Commit c302159

Browse files
lachlan-robertslachlan-roberts
authored
Merge pull request #9345 from eclipse/jetty-9.4.x-multipartCleanup
multipart cleanups jetty 9
2 parents 2f431ff + c874c00 commit c302159

File tree

5 files changed

+219
-10
lines changed

5 files changed

+219
-10
lines changed

‎jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,12 @@
6161
public class MultiPartFormInputStream
6262
{
6363
private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
64+
private static final int DEFAULT_MAX_FORM_KEYS = 1000;
6465
private static final MultiMap<Part> EMPTY_MAP = new MultiMap<>(Collections.emptyMap());
65-
private final MultiMap<Part> _parts;
6666
private final EnumSet<NonCompliance> _nonComplianceWarnings = EnumSet.noneOf(NonCompliance.class);
67+
private final MultiMap<Part> _parts;
68+
private final int _maxParts;
69+
private int _numParts = 0;
6770
private InputStream _in;
6871
private MultipartConfigElement _config;
6972
private String _contentType;
@@ -350,18 +353,30 @@ public String getContentDispositionFilename()
350353
* @param contextTmpDir javax.servlet.context.tempdir
351354
*/
352355
public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir)
356+
{
357+
this(in, contentType, config, contextTmpDir, DEFAULT_MAX_FORM_KEYS);
358+
}
359+
360+
/**
361+
* @param in Request input stream
362+
* @param contentType Content-Type header
363+
* @param config MultipartConfigElement
364+
* @param contextTmpDir javax.servlet.context.tempdir
365+
* @param maxParts the maximum number of parts that can be parsed from the multipart content (0 for no parts allowed, -1 for unlimited parts).
366+
*/
367+
public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, int maxParts)
353368
{
354369
_contentType = contentType;
355370
_config = config;
356371
_contextTmpDir = contextTmpDir;
372+
_maxParts = maxParts;
357373
if (_contextTmpDir == null)
358374
_contextTmpDir = new File(System.getProperty("java.io.tmpdir"));
359375

360376
if (_config == null)
361377
_config = new MultipartConfigElement(_contextTmpDir.getAbsolutePath());
362378

363-
MultiMap parts = new MultiMap();
364-
379+
MultiMap<Part> parts = new MultiMap<>();
365380
if (in instanceof ServletInputStream)
366381
{
367382
if (((ServletInputStream)in).isFinished())
@@ -752,6 +767,9 @@ public boolean content(ByteBuffer buffer, boolean last)
752767
public void startPart()
753768
{
754769
reset();
770+
_numParts++;
771+
if (_maxParts >= 0 && _numParts > _maxParts)
772+
throw new IllegalStateException(String.format("Form with too many parts [%d > %d]", _numParts, _maxParts));
755773
}
756774

757775
@Override

‎jetty-server/src/main/java/org/eclipse/jetty/server/MultiParts.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,12 @@ class MultiPartsHttpParser implements MultiParts
5858

5959
public MultiPartsHttpParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request) throws IOException
6060
{
61-
_httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir);
61+
this(in, contentType, config, contextTmpDir, request, ContextHandler.DEFAULT_MAX_FORM_KEYS);
62+
}
63+
64+
public MultiPartsHttpParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request, int maxParts) throws IOException
65+
{
66+
_httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir, maxParts);
6267
_context = request.getContext();
6368
_request = request;
6469
}
@@ -123,7 +128,12 @@ class MultiPartsUtilParser implements MultiParts
123128

124129
public MultiPartsUtilParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request) throws IOException
125130
{
126-
_utilParser = new MultiPartInputStreamParser(in, contentType, config, contextTmpDir);
131+
this(in, contentType, config, contextTmpDir, request, ContextHandler.DEFAULT_MAX_FORM_KEYS);
132+
}
133+
134+
public MultiPartsUtilParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request, int maxParts) throws IOException
135+
{
136+
_utilParser = new MultiPartInputStreamParser(in, contentType, config, contextTmpDir, maxParts);
127137
_context = request.getContext();
128138
_request = request;
129139
}

‎jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2431,7 +2431,21 @@ private Collection<Part> getParts(MultiMap<String> params) throws IOException
24312431
if (config == null)
24322432
throw new IllegalStateException("No multipart config for servlet");
24332433

2434-
_multiParts = newMultiParts(config);
2434+
int maxFormContentSize = ContextHandler.DEFAULT_MAX_FORM_CONTENT_SIZE;
2435+
int maxFormKeys = ContextHandler.DEFAULT_MAX_FORM_KEYS;
2436+
if (_context != null)
2437+
{
2438+
ContextHandler contextHandler = _context.getContextHandler();
2439+
maxFormContentSize = contextHandler.getMaxFormContentSize();
2440+
maxFormKeys = contextHandler.getMaxFormKeys();
2441+
}
2442+
else
2443+
{
2444+
maxFormContentSize = lookupServerAttribute(ContextHandler.MAX_FORM_CONTENT_SIZE_KEY, maxFormContentSize);
2445+
maxFormKeys = lookupServerAttribute(ContextHandler.MAX_FORM_KEYS_KEY, maxFormKeys);
2446+
}
2447+
2448+
_multiParts = newMultiParts(config, maxFormKeys);
24352449
setAttribute(MULTIPARTS, _multiParts);
24362450
Collection<Part> parts = _multiParts.getParts();
24372451

@@ -2465,11 +2479,16 @@ else if (getCharacterEncoding() != null)
24652479
else
24662480
defaultCharset = StandardCharsets.UTF_8;
24672481

2482+
long formContentSize = 0;
24682483
ByteArrayOutputStream os = null;
24692484
for (Part p : parts)
24702485
{
24712486
if (p.getSubmittedFileName() == null)
24722487
{
2488+
formContentSize = Math.addExact(formContentSize, p.getSize());
2489+
if (maxFormContentSize >= 0 && formContentSize > maxFormContentSize)
2490+
throw new IllegalStateException("Form is larger than max length " + maxFormContentSize);
2491+
24732492
// Servlet Spec 3.0 pg 23, parts without filename must be put into params.
24742493
String charset = null;
24752494
if (p.getContentType() != null)
@@ -2494,7 +2513,7 @@ else if (getCharacterEncoding() != null)
24942513
return _multiParts.getParts();
24952514
}
24962515

2497-
private MultiParts newMultiParts(MultipartConfigElement config) throws IOException
2516+
private MultiParts newMultiParts(MultipartConfigElement config, int maxParts) throws IOException
24982517
{
24992518
MultiPartFormDataCompliance compliance = getHttpChannel().getHttpConfiguration().getMultipartFormDataCompliance();
25002519
if (LOG.isDebugEnabled())
@@ -2504,12 +2523,12 @@ private MultiParts newMultiParts(MultipartConfigElement config) throws IOExcepti
25042523
{
25052524
case RFC7578:
25062525
return new MultiParts.MultiPartsHttpParser(getInputStream(), getContentType(), config,
2507-
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this);
2526+
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this, maxParts);
25082527

25092528
case LEGACY:
25102529
default:
25112530
return new MultiParts.MultiPartsUtilParser(getInputStream(), getContentType(), config,
2512-
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this);
2531+
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this, maxParts);
25132532
}
25142533
}
25152534

‎jetty-servlet/src/test/java/org/eclipse/jetty/servlet/MultiPartServletTest.java

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,16 @@
4141
import org.eclipse.jetty.client.util.BytesContentProvider;
4242
import org.eclipse.jetty.client.util.InputStreamResponseListener;
4343
import org.eclipse.jetty.client.util.MultiPartContentProvider;
44+
import org.eclipse.jetty.client.util.OutputStreamContentProvider;
4445
import org.eclipse.jetty.client.util.StringContentProvider;
4546
import org.eclipse.jetty.http.HttpFields;
4647
import org.eclipse.jetty.http.HttpHeader;
4748
import org.eclipse.jetty.http.HttpMethod;
4849
import org.eclipse.jetty.http.HttpScheme;
50+
import org.eclipse.jetty.http.HttpStatus;
4951
import org.eclipse.jetty.http.MimeTypes;
5052
import org.eclipse.jetty.http.MultiPartFormInputStream;
53+
import org.eclipse.jetty.io.EofException;
5154
import org.eclipse.jetty.server.HttpChannel;
5255
import org.eclipse.jetty.server.HttpConnectionFactory;
5356
import org.eclipse.jetty.server.MultiPartFormDataCompliance;
@@ -67,6 +70,8 @@
6770

6871
import static org.hamcrest.MatcherAssert.assertThat;
6972
import static org.hamcrest.Matchers.containsString;
73+
import static org.hamcrest.Matchers.equalTo;
74+
import static org.hamcrest.Matchers.instanceOf;
7075
import static org.hamcrest.Matchers.is;
7176
import static org.hamcrest.Matchers.startsWith;
7277
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -82,13 +87,27 @@ public class MultiPartServletTest
8287
private Path tmpDir;
8388

8489
private static final int MAX_FILE_SIZE = 512 * 1024;
90+
private static final int MAX_REQUEST_SIZE = 1024 * 1024 * 8;
8591
private static final int LARGE_MESSAGE_SIZE = 1024 * 1024;
8692

8793
public static Stream<Arguments> data()
8894
{
8995
return Arrays.asList(MultiPartFormDataCompliance.values()).stream().map(Arguments::of);
9096
}
9197

98+
public static class RequestParameterServlet extends HttpServlet
99+
{
100+
@Override
101+
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
102+
{
103+
req.getParameterMap();
104+
req.getParts();
105+
resp.setStatus(200);
106+
resp.getWriter().print("success");
107+
resp.getWriter().close();
108+
}
109+
}
110+
92111
public static class MultiPartServlet extends HttpServlet
93112
{
94113
@Override
@@ -130,6 +149,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S
130149
public void start() throws Exception
131150
{
132151
tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName());
152+
Files.deleteIfExists(tmpDir);
133153
assertNotNull(tmpDir);
134154

135155
server = new Server();
@@ -138,11 +158,19 @@ public void start() throws Exception
138158

139159
MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
140160
MAX_FILE_SIZE, -1, 1);
161+
MultipartConfigElement requestSizedConfig = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
162+
-1, MAX_REQUEST_SIZE, 1);
163+
MultipartConfigElement defaultConfig = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
164+
-1, -1, 1);
141165

142166
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
143167
contextHandler.setContextPath("/");
144168
ServletHolder servletHolder = contextHandler.addServlet(MultiPartServlet.class, "/");
145169
servletHolder.getRegistration().setMultipartConfig(config);
170+
servletHolder = contextHandler.addServlet(RequestParameterServlet.class, "/defaultConfig");
171+
servletHolder.getRegistration().setMultipartConfig(defaultConfig);
172+
servletHolder = contextHandler.addServlet(RequestParameterServlet.class, "/requestSizeLimit");
173+
servletHolder.getRegistration().setMultipartConfig(requestSizedConfig);
146174
servletHolder = contextHandler.addServlet(MultiPartEchoServlet.class, "/echo");
147175
servletHolder.getRegistration().setMultipartConfig(config);
148176

@@ -169,6 +197,119 @@ public void stop() throws Exception
169197
IO.delete(tmpDir.toFile());
170198
}
171199

200+
@ParameterizedTest
201+
@MethodSource("data")
202+
public void testLargePart(MultiPartFormDataCompliance compliance) throws Exception
203+
{
204+
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
205+
.setMultiPartFormDataCompliance(compliance);
206+
207+
OutputStreamContentProvider content = new OutputStreamContentProvider();
208+
MultiPartContentProvider multiPart = new MultiPartContentProvider();
209+
multiPart.addFieldPart("param", content, null);
210+
multiPart.close();
211+
212+
InputStreamResponseListener listener = new InputStreamResponseListener();
213+
client.newRequest("localhost", connector.getLocalPort())
214+
.path("/defaultConfig")
215+
.scheme(HttpScheme.HTTP.asString())
216+
.method(HttpMethod.POST)
217+
.content(multiPart)
218+
.send(listener);
219+
220+
// Write large amount of content to the part.
221+
byte[] byteArray = new byte[1024 * 1024];
222+
Arrays.fill(byteArray, (byte)1);
223+
for (int i = 0; i < 128 * 2; i++)
224+
{
225+
content.getOutputStream().write(byteArray);
226+
}
227+
content.close();
228+
229+
Response response = listener.get(2, TimeUnit.MINUTES);
230+
assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
231+
String responseContent = IO.toString(listener.getInputStream());
232+
assertThat(responseContent, containsString("Unable to parse form content"));
233+
assertThat(responseContent, containsString("Form is larger than max length"));
234+
}
235+
236+
@ParameterizedTest
237+
@MethodSource("data")
238+
public void testManyParts(MultiPartFormDataCompliance compliance) throws Exception
239+
{
240+
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
241+
.setMultiPartFormDataCompliance(compliance);
242+
243+
byte[] byteArray = new byte[1024];
244+
Arrays.fill(byteArray, (byte)1);
245+
246+
MultiPartContentProvider multiPart = new MultiPartContentProvider();
247+
for (int i = 0; i < 1024 * 1024; i++)
248+
{
249+
BytesContentProvider content = new BytesContentProvider(byteArray);
250+
multiPart.addFieldPart("part" + i, content, null);
251+
}
252+
multiPart.close();
253+
254+
InputStreamResponseListener listener = new InputStreamResponseListener();
255+
client.newRequest("localhost", connector.getLocalPort())
256+
.path("/defaultConfig")
257+
.scheme(HttpScheme.HTTP.asString())
258+
.method(HttpMethod.POST)
259+
.content(multiPart)
260+
.send(listener);
261+
262+
Response response = listener.get(30, TimeUnit.SECONDS);
263+
assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
264+
String responseContent = IO.toString(listener.getInputStream());
265+
assertThat(responseContent, containsString("Unable to parse form content"));
266+
assertThat(responseContent, containsString("Form with too many parts"));
267+
}
268+
269+
@ParameterizedTest
270+
@MethodSource("data")
271+
public void testMaxRequestSize(MultiPartFormDataCompliance compliance) throws Exception
272+
{
273+
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
274+
.setMultiPartFormDataCompliance(compliance);
275+
276+
OutputStreamContentProvider content = new OutputStreamContentProvider();
277+
MultiPartContentProvider multiPart = new MultiPartContentProvider();
278+
multiPart.addFieldPart("param", content, null);
279+
multiPart.close();
280+
281+
InputStreamResponseListener listener = new InputStreamResponseListener();
282+
client.newRequest("localhost", connector.getLocalPort())
283+
.path("/requestSizeLimit")
284+
.scheme(HttpScheme.HTTP.asString())
285+
.method(HttpMethod.POST)
286+
.content(multiPart)
287+
.send(listener);
288+
289+
Throwable writeError = null;
290+
try
291+
{
292+
// Write large amount of content to the part.
293+
byte[] byteArray = new byte[1024 * 1024];
294+
Arrays.fill(byteArray, (byte)1);
295+
for (int i = 0; i < 512; i++)
296+
{
297+
content.getOutputStream().write(byteArray);
298+
}
299+
}
300+
catch (Exception e)
301+
{
302+
writeError = e;
303+
}
304+
305+
if (writeError != null)
306+
assertThat(writeError, instanceOf(EofException.class));
307+
308+
// We should get 400 response.
309+
Response response = listener.get(30, TimeUnit.SECONDS);
310+
assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
311+
}
312+
172313
@ParameterizedTest
173314
@MethodSource("data")
174315
public void testTempFilesDeletedOnError(MultiPartFormDataCompliance compliance) throws Exception

‎jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,11 @@
6464
public class MultiPartInputStreamParser
6565
{
6666
private static final Logger LOG = Log.getLogger(MultiPartInputStreamParser.class);
67+
private static final int DEFAULT_MAX_FORM_KEYS = 1000;
6768
public static final MultipartConfigElement __DEFAULT_MULTIPART_CONFIG = new MultipartConfigElement(System.getProperty("java.io.tmpdir"));
68-
public static final MultiMap<Part> EMPTY_MAP = new MultiMap(Collections.emptyMap());
69+
public static final MultiMap<Part> EMPTY_MAP = new MultiMap<>(Collections.emptyMap());
70+
private final int _maxParts;
71+
private int _numParts;
6972
protected InputStream _in;
7073
protected MultipartConfigElement _config;
7174
protected String _contentType;
@@ -394,10 +397,23 @@ public String getContentDispositionFilename()
394397
* @param contextTmpDir javax.servlet.context.tempdir
395398
*/
396399
public MultiPartInputStreamParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir)
400+
{
401+
this(in, contentType, config, contextTmpDir, DEFAULT_MAX_FORM_KEYS);
402+
}
403+
404+
/**
405+
* @param in Request input stream
406+
* @param contentType Content-Type header
407+
* @param config MultipartConfigElement
408+
* @param contextTmpDir javax.servlet.context.tempdir
409+
* @param maxParts the maximum number of parts that can be parsed from the multipart content (0 for no parts allowed, -1 for unlimited parts).
410+
*/
411+
public MultiPartInputStreamParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, int maxParts)
397412
{
398413
_contentType = contentType;
399414
_config = config;
400415
_contextTmpDir = contextTmpDir;
416+
_maxParts = maxParts;
401417
if (_contextTmpDir == null)
402418
_contextTmpDir = new File(System.getProperty("java.io.tmpdir"));
403419

@@ -693,6 +709,11 @@ else if (tl.startsWith("filename="))
693709
continue;
694710
}
695711

712+
// Check if we can create a new part.
713+
_numParts++;
714+
if (_maxParts >= 0 && _numParts > _maxParts)
715+
throw new IllegalStateException(String.format("Form with too many parts [%d > %d]", _numParts, _maxParts));
716+
696717
//Have a new Part
697718
MultiPart part = new MultiPart(name, filename);
698719
part.setHeaders(headers);

0 commit comments

Comments
 (0)