Skip to content

Commit

Permalink
Merge pull request #9345 from eclipse/jetty-9.4.x-multipartCleanup
Browse files Browse the repository at this point in the history 
multipart cleanups jetty 9
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Feb 14, 2023
2 parents 2f431ff + c874c00 commit c302159
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 10 deletions.
24 changes: 21 additions & 3 deletions jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,12 @@
public class MultiPartFormInputStream
{
private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
private static final int DEFAULT_MAX_FORM_KEYS = 1000;
private static final MultiMap<Part> EMPTY_MAP = new MultiMap<>(Collections.emptyMap());
private final MultiMap<Part> _parts;
private final EnumSet<NonCompliance> _nonComplianceWarnings = EnumSet.noneOf(NonCompliance.class);
private final MultiMap<Part> _parts;
private final int _maxParts;
private int _numParts = 0;
private InputStream _in;
private MultipartConfigElement _config;
private String _contentType;
Expand Down Expand Up @@ -350,18 +353,30 @@ public String getContentDispositionFilename()
* @param contextTmpDir javax.servlet.context.tempdir
*/
public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir)
{
this(in, contentType, config, contextTmpDir, DEFAULT_MAX_FORM_KEYS);
}

/**
* @param in Request input stream
* @param contentType Content-Type header
* @param config MultipartConfigElement
* @param contextTmpDir javax.servlet.context.tempdir
* @param maxParts the maximum number of parts that can be parsed from the multipart content (0 for no parts allowed, -1 for unlimited parts).
*/
public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, int maxParts)
{
_contentType = contentType;
_config = config;
_contextTmpDir = contextTmpDir;
_maxParts = maxParts;
if (_contextTmpDir == null)
_contextTmpDir = new File(System.getProperty("java.io.tmpdir"));

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

MultiMap parts = new MultiMap();

MultiMap<Part> parts = new MultiMap<>();
if (in instanceof ServletInputStream)
{
if (((ServletInputStream)in).isFinished())
Expand Down Expand Up @@ -752,6 +767,9 @@ public boolean content(ByteBuffer buffer, boolean last)
public void startPart()
{
reset();
_numParts++;
if (_maxParts >= 0 && _numParts > _maxParts)
throw new IllegalStateException(String.format("Form with too many parts [%d > %d]", _numParts, _maxParts));
}

@Override
Expand Down
14 changes: 12 additions & 2 deletions jetty-server/src/main/java/org/eclipse/jetty/server/MultiParts.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ class MultiPartsHttpParser implements MultiParts

public MultiPartsHttpParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request) throws IOException
{
_httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir);
this(in, contentType, config, contextTmpDir, request, ContextHandler.DEFAULT_MAX_FORM_KEYS);
}

public MultiPartsHttpParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request, int maxParts) throws IOException
{
_httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir, maxParts);
_context = request.getContext();
_request = request;
}
Expand Down Expand Up @@ -123,7 +128,12 @@ class MultiPartsUtilParser implements MultiParts

public MultiPartsUtilParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request) throws IOException
{
_utilParser = new MultiPartInputStreamParser(in, contentType, config, contextTmpDir);
this(in, contentType, config, contextTmpDir, request, ContextHandler.DEFAULT_MAX_FORM_KEYS);
}

public MultiPartsUtilParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request, int maxParts) throws IOException
{
_utilParser = new MultiPartInputStreamParser(in, contentType, config, contextTmpDir, maxParts);
_context = request.getContext();
_request = request;
}
Expand Down
27 changes: 23 additions & 4 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2431,7 +2431,21 @@ private Collection<Part> getParts(MultiMap<String> params) throws IOException
if (config == null)
throw new IllegalStateException("No multipart config for servlet");

_multiParts = newMultiParts(config);
int maxFormContentSize = ContextHandler.DEFAULT_MAX_FORM_CONTENT_SIZE;
int maxFormKeys = ContextHandler.DEFAULT_MAX_FORM_KEYS;
if (_context != null)
{
ContextHandler contextHandler = _context.getContextHandler();
maxFormContentSize = contextHandler.getMaxFormContentSize();
maxFormKeys = contextHandler.getMaxFormKeys();
}
else
{
maxFormContentSize = lookupServerAttribute(ContextHandler.MAX_FORM_CONTENT_SIZE_KEY, maxFormContentSize);
maxFormKeys = lookupServerAttribute(ContextHandler.MAX_FORM_KEYS_KEY, maxFormKeys);
}

_multiParts = newMultiParts(config, maxFormKeys);
setAttribute(MULTIPARTS, _multiParts);
Collection<Part> parts = _multiParts.getParts();

Expand Down Expand Up @@ -2465,11 +2479,16 @@ else if (getCharacterEncoding() != null)
else
defaultCharset = StandardCharsets.UTF_8;

long formContentSize = 0;
ByteArrayOutputStream os = null;
for (Part p : parts)
{
if (p.getSubmittedFileName() == null)
{
formContentSize = Math.addExact(formContentSize, p.getSize());
if (maxFormContentSize >= 0 && formContentSize > maxFormContentSize)
throw new IllegalStateException("Form is larger than max length " + maxFormContentSize);

// Servlet Spec 3.0 pg 23, parts without filename must be put into params.
String charset = null;
if (p.getContentType() != null)
Expand All @@ -2494,7 +2513,7 @@ else if (getCharacterEncoding() != null)
return _multiParts.getParts();
}

private MultiParts newMultiParts(MultipartConfigElement config) throws IOException
private MultiParts newMultiParts(MultipartConfigElement config, int maxParts) throws IOException
{
MultiPartFormDataCompliance compliance = getHttpChannel().getHttpConfiguration().getMultipartFormDataCompliance();
if (LOG.isDebugEnabled())
Expand All @@ -2504,12 +2523,12 @@ private MultiParts newMultiParts(MultipartConfigElement config) throws IOExcepti
{
case RFC7578:
return new MultiParts.MultiPartsHttpParser(getInputStream(), getContentType(), config,
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this);
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this, maxParts);

case LEGACY:
default:
return new MultiParts.MultiPartsUtilParser(getInputStream(), getContentType(), config,
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this);
(_context != null ? (File)_context.getAttribute("javax.servlet.context.tempdir") : null), this, maxParts);
}
}

Expand Down
141 changes: 141 additions & 0 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/MultiPartServletTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,16 @@
import org.eclipse.jetty.client.util.BytesContentProvider;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.client.util.MultiPartContentProvider;
import org.eclipse.jetty.client.util.OutputStreamContentProvider;
import org.eclipse.jetty.client.util.StringContentProvider;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.MultiPartFormInputStream;
import org.eclipse.jetty.io.EofException;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.MultiPartFormDataCompliance;
Expand All @@ -67,6 +70,8 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -82,13 +87,27 @@ public class MultiPartServletTest
private Path tmpDir;

private static final int MAX_FILE_SIZE = 512 * 1024;
private static final int MAX_REQUEST_SIZE = 1024 * 1024 * 8;
private static final int LARGE_MESSAGE_SIZE = 1024 * 1024;

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

public static class RequestParameterServlet extends HttpServlet
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
req.getParameterMap();
req.getParts();
resp.setStatus(200);
resp.getWriter().print("success");
resp.getWriter().close();
}
}

public static class MultiPartServlet extends HttpServlet
{
@Override
Expand Down Expand Up @@ -130,6 +149,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S
public void start() throws Exception
{
tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName());
Files.deleteIfExists(tmpDir);
assertNotNull(tmpDir);

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

MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
MAX_FILE_SIZE, -1, 1);
MultipartConfigElement requestSizedConfig = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
-1, MAX_REQUEST_SIZE, 1);
MultipartConfigElement defaultConfig = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
-1, -1, 1);

ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
contextHandler.setContextPath("/");
ServletHolder servletHolder = contextHandler.addServlet(MultiPartServlet.class, "/");
servletHolder.getRegistration().setMultipartConfig(config);
servletHolder = contextHandler.addServlet(RequestParameterServlet.class, "/defaultConfig");
servletHolder.getRegistration().setMultipartConfig(defaultConfig);
servletHolder = contextHandler.addServlet(RequestParameterServlet.class, "/requestSizeLimit");
servletHolder.getRegistration().setMultipartConfig(requestSizedConfig);
servletHolder = contextHandler.addServlet(MultiPartEchoServlet.class, "/echo");
servletHolder.getRegistration().setMultipartConfig(config);

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

@ParameterizedTest
@MethodSource("data")
public void testLargePart(MultiPartFormDataCompliance compliance) throws Exception
{
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
.setMultiPartFormDataCompliance(compliance);

OutputStreamContentProvider content = new OutputStreamContentProvider();
MultiPartContentProvider multiPart = new MultiPartContentProvider();
multiPart.addFieldPart("param", content, null);
multiPart.close();

InputStreamResponseListener listener = new InputStreamResponseListener();
client.newRequest("localhost", connector.getLocalPort())
.path("/defaultConfig")
.scheme(HttpScheme.HTTP.asString())
.method(HttpMethod.POST)
.content(multiPart)
.send(listener);

// Write large amount of content to the part.
byte[] byteArray = new byte[1024 * 1024];
Arrays.fill(byteArray, (byte)1);
for (int i = 0; i < 128 * 2; i++)
{
content.getOutputStream().write(byteArray);
}
content.close();

Response response = listener.get(2, TimeUnit.MINUTES);
assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
String responseContent = IO.toString(listener.getInputStream());
assertThat(responseContent, containsString("Unable to parse form content"));
assertThat(responseContent, containsString("Form is larger than max length"));
}

@ParameterizedTest
@MethodSource("data")
public void testManyParts(MultiPartFormDataCompliance compliance) throws Exception
{
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
.setMultiPartFormDataCompliance(compliance);

byte[] byteArray = new byte[1024];
Arrays.fill(byteArray, (byte)1);

MultiPartContentProvider multiPart = new MultiPartContentProvider();
for (int i = 0; i < 1024 * 1024; i++)
{
BytesContentProvider content = new BytesContentProvider(byteArray);
multiPart.addFieldPart("part" + i, content, null);
}
multiPart.close();

InputStreamResponseListener listener = new InputStreamResponseListener();
client.newRequest("localhost", connector.getLocalPort())
.path("/defaultConfig")
.scheme(HttpScheme.HTTP.asString())
.method(HttpMethod.POST)
.content(multiPart)
.send(listener);

Response response = listener.get(30, TimeUnit.SECONDS);
assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
String responseContent = IO.toString(listener.getInputStream());
assertThat(responseContent, containsString("Unable to parse form content"));
assertThat(responseContent, containsString("Form with too many parts"));
}

@ParameterizedTest
@MethodSource("data")
public void testMaxRequestSize(MultiPartFormDataCompliance compliance) throws Exception
{
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
.setMultiPartFormDataCompliance(compliance);

OutputStreamContentProvider content = new OutputStreamContentProvider();
MultiPartContentProvider multiPart = new MultiPartContentProvider();
multiPart.addFieldPart("param", content, null);
multiPart.close();

InputStreamResponseListener listener = new InputStreamResponseListener();
client.newRequest("localhost", connector.getLocalPort())
.path("/requestSizeLimit")
.scheme(HttpScheme.HTTP.asString())
.method(HttpMethod.POST)
.content(multiPart)
.send(listener);

Throwable writeError = null;
try
{
// Write large amount of content to the part.
byte[] byteArray = new byte[1024 * 1024];
Arrays.fill(byteArray, (byte)1);
for (int i = 0; i < 512; i++)
{
content.getOutputStream().write(byteArray);
}
}
catch (Exception e)
{
writeError = e;
}

if (writeError != null)
assertThat(writeError, instanceOf(EofException.class));

// We should get 400 response.
Response response = listener.get(30, TimeUnit.SECONDS);
assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
}

@ParameterizedTest
@MethodSource("data")
public void testTempFilesDeletedOnError(MultiPartFormDataCompliance compliance) throws Exception
Expand Down
23 changes: 22 additions & 1 deletion jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@
public class MultiPartInputStreamParser
{
private static final Logger LOG = Log.getLogger(MultiPartInputStreamParser.class);
private static final int DEFAULT_MAX_FORM_KEYS = 1000;
public static final MultipartConfigElement __DEFAULT_MULTIPART_CONFIG = new MultipartConfigElement(System.getProperty("java.io.tmpdir"));
public static final MultiMap<Part> EMPTY_MAP = new MultiMap(Collections.emptyMap());
public static final MultiMap<Part> EMPTY_MAP = new MultiMap<>(Collections.emptyMap());
private final int _maxParts;
private int _numParts;
protected InputStream _in;
protected MultipartConfigElement _config;
protected String _contentType;
Expand Down Expand Up @@ -394,10 +397,23 @@ public String getContentDispositionFilename()
* @param contextTmpDir javax.servlet.context.tempdir
*/
public MultiPartInputStreamParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir)
{
this(in, contentType, config, contextTmpDir, DEFAULT_MAX_FORM_KEYS);
}

/**
* @param in Request input stream
* @param contentType Content-Type header
* @param config MultipartConfigElement
* @param contextTmpDir javax.servlet.context.tempdir
* @param maxParts the maximum number of parts that can be parsed from the multipart content (0 for no parts allowed, -1 for unlimited parts).
*/
public MultiPartInputStreamParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, int maxParts)
{
_contentType = contentType;
_config = config;
_contextTmpDir = contextTmpDir;
_maxParts = maxParts;
if (_contextTmpDir == null)
_contextTmpDir = new File(System.getProperty("java.io.tmpdir"));

Expand Down Expand Up @@ -693,6 +709,11 @@ else if (tl.startsWith("filename="))
continue;
}

// Check if we can create a new part.
_numParts++;
if (_maxParts >= 0 && _numParts > _maxParts)
throw new IllegalStateException(String.format("Form with too many parts [%d > %d]", _numParts, _maxParts));

//Have a new Part
MultiPart part = new MultiPart(name, filename);
part.setHeaders(headers);
Expand Down

0 comments on commit c302159

Please sign in to comment.