Skip to content

Commit

Permalink
bring back some multipart improvements from #9287
Browse files Browse the repository at this point in the history 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Feb 9, 2023
1 parent bd4cf3c commit a5344d7
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 9 deletions.
19 changes: 19 additions & 0 deletions jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java
View file
Expand Up @@ -38,6 +38,7 @@
import javax.servlet.http.Part;

import org.eclipse.jetty.server.MultiParts.NonCompliance;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.ByteArrayOutputStream2;
import org.eclipse.jetty.util.MultiException;
Expand Down Expand Up @@ -98,6 +99,8 @@ private enum State
private final MultipartConfigElement _config;
private final File _contextTmpDir;
private final String _contentType;
private final int _maxParts;
private int _numParts;
private volatile Throwable _err;
private volatile Path _tmpDir;
private volatile boolean _deleteOnExit;
Expand Down Expand Up @@ -367,6 +370,18 @@ public String getContentDispositionFilename()
* @param contextTmpDir javax.servlet.context.tempdir
*/
public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir)
{
this(in, contentType, config, contextTmpDir, ContextHandler.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)
{
// Must be a multipart request.
_contentType = contentType;
Expand All @@ -375,6 +390,7 @@ public MultiPartFormInputStream(InputStream in, String contentType, MultipartCon

_contextTmpDir = (contextTmpDir != null) ? contextTmpDir : new File(System.getProperty("java.io.tmpdir"));
_config = (config != null) ? config : new MultipartConfigElement(_contextTmpDir.getAbsolutePath());
_maxParts = maxParts;

if (in instanceof ServletInputStream)
{
Expand Down Expand Up @@ -797,6 +813,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 keys [%d > %d]", _numParts, _maxParts));
}

@Override
Expand Down
23 changes: 22 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartInputStreamParser.java
View file
Expand Up @@ -41,6 +41,7 @@
import javax.servlet.http.Part;

import org.eclipse.jetty.server.MultiParts.NonCompliance;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.ByteArrayOutputStream2;
import org.eclipse.jetty.util.LazyList;
import org.eclipse.jetty.util.MultiException;
Expand All @@ -67,7 +68,9 @@ public class MultiPartInputStreamParser
{
private static final Logger LOG = LoggerFactory.getLogger(MultiPartInputStreamParser.class);
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 @@ -375,10 +378,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, ContextHandler.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 @@ -674,6 +690,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 keys [%d > %d]", _numParts, _maxParts));

//Have a new Part
MultiPart part = new MultiPart(name, filename);
part.setHeaders(headers);
Expand Down
14 changes: 12 additions & 2 deletions jetty-server/src/main/java/org/eclipse/jetty/server/MultiParts.java
View file
Expand Up @@ -74,7 +74,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 @@ -145,7 +150,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
31 changes: 25 additions & 6 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View file
Expand Up @@ -2305,7 +2305,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);
Collection<Part> parts = _multiParts.getParts();

String formCharset = null;
Expand All @@ -2316,7 +2330,7 @@ private Collection<Part> getParts(MultiMap<String> params) throws IOException
{
ByteArrayOutputStream os = new ByteArrayOutputStream();
IO.copy(is, os);
formCharset = new String(os.toByteArray(), StandardCharsets.UTF_8);
formCharset = os.toString(StandardCharsets.UTF_8);
}
}

Expand All @@ -2338,11 +2352,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 @@ -2354,7 +2373,7 @@ else if (getCharacterEncoding() != null)
os = new ByteArrayOutputStream();
IO.copy(is, os);

String content = new String(os.toByteArray(), charset == null ? defaultCharset : Charset.forName(charset));
String content = os.toString(charset == null ? defaultCharset : Charset.forName(charset));
if (_contentParameters == null)
_contentParameters = params == null ? new MultiMap<>() : params;
_contentParameters.add(p.getName(), content);
Expand All @@ -2367,7 +2386,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 @@ -2377,12 +2396,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
128 changes: 128 additions & 0 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/MultiPartServletTest.java
View file
Expand Up @@ -35,12 +35,15 @@
import org.eclipse.jetty.client.util.BytesRequestContent;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.client.util.MultiPartRequestContent;
import org.eclipse.jetty.client.util.OutputStreamRequestContent;
import org.eclipse.jetty.client.util.StringRequestContent;
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.io.EofException;
import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.MultiPartFormInputStream;
Expand All @@ -54,6 +57,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 @@ -67,8 +72,22 @@ 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 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 @@ -118,11 +137,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 @@ -148,6 +175,107 @@ public void stop() throws Exception
IO.delete(tmpDir.toFile());
}

@Test
public void testLargePart() throws Exception
{
OutputStreamRequestContent content = new OutputStreamRequestContent();
MultiPartRequestContent multiPart = new MultiPartRequestContent();
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)
.body(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 < 1024 * 2; i++)
{
content.getOutputStream().write(byteArray);
}
content.close();

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 is larger than max length"));
}

@Test
public void testManyParts() throws Exception
{
byte[] byteArray = new byte[1024];
Arrays.fill(byteArray, (byte)1);

MultiPartRequestContent multiPart = new MultiPartRequestContent();
for (int i = 0; i < 1024 * 1024; i++)
{
BytesRequestContent content = new BytesRequestContent(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)
.body(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 keys"));
}

@Test
public void testMaxRequestSize() throws Exception
{
OutputStreamRequestContent content = new OutputStreamRequestContent();
MultiPartRequestContent multiPart = new MultiPartRequestContent();
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)
.body(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));
}

@Test
public void testTempFilesDeletedOnError() throws Exception
{
Expand Down

0 comments on commit a5344d7

Please sign in to comment.