multipart cleanups jetty 9

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

@@ -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;
@@ -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())
@@ -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 keys [%d > %d]", _numParts, _maxParts));
         }
 
         @Override

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

@@ -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;
         }
@@ -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;
         }

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

@@ -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();
 
@@ -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)
@@ -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())
@@ -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);
         }
     }
 

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

@@ -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;
@@ -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;
@@ -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
@@ -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();
@@ -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);
 
@@ -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 keys"));
+    }
+
+    @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

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

@@ -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;
@@ -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"));
 
@@ -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 keys [%d > %d]", _numParts, _maxParts));
+
                 //Have a new Part
                 MultiPart part = new MultiPart(name, filename);
                 part.setHeaders(headers);