Merge remote-tracking branch 'eclipse/jetty-9.4.x' into jetty-10.0.x

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

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

@@ -298,9 +298,8 @@ public class MultiPartFormInputStream
          */
         public void cleanUp() throws IOException
         {
-            if (_temporary && _file != null && _file.exists())
+            if (_temporary)
-                if (!_file.delete())
+                delete();
-                    throw new IOException("Could Not Delete File");
         }
         
         /**
@@ -376,31 +375,21 @@ public class MultiPartFormInputStream
      */
     public void deleteParts()
     {
-        if (!_parsed)
-            return;
-        
-        Collection<Part> parts;
-        try
-        {
-            parts = getParts();
-        }
-        catch (IOException e)
-        {
-            throw new RuntimeException(e);
-        }
-        
         MultiException err = null;
-        for (Part p : parts)
+        for (List<Part> parts : _parts.values())
         {
-            try
+            for (Part p : parts)
             {
-                ((MultiPart)p).cleanUp();
+                try
-            }
+                {
-            catch (Exception e)
+                    ((MultiPart)p).cleanUp();
-            {
+                }
-                if (err == null)
+                catch (Exception e)
-                    err = new MultiException();
+                {
-                err.add(e);
+                    if (err == null)
+                        err = new MultiException();
+                    err.add(e);
+                }
             }
         }
         _parts.clear();
@@ -455,6 +444,9 @@ public class MultiPartFormInputStream
     {
         if (_err != null)
         {
+            if (LOG.isDebugEnabled())
+                LOG.debug("MultiPart parsing failure ", _err);
+
             _err.addSuppressed(new Throwable());
             if (_err instanceof IOException)
                 throw (IOException)_err;
@@ -473,7 +465,9 @@ public class MultiPartFormInputStream
         if (_parsed)
             return;
         _parsed = true;
-        
+
+        MultiPartParser parser = null;
+        Handler handler = new Handler();
         try
         {
             // initialize
@@ -509,9 +503,7 @@ public class MultiPartFormInputStream
                 contentTypeBoundary = QuotedStringTokenizer.unquote(value(_contentType.substring(bstart, bend)).trim());
             }
             
-            Handler handler = new Handler();
+            parser = new MultiPartParser(handler, contentTypeBoundary);
-            MultiPartParser parser = new MultiPartParser(handler, contentTypeBoundary);
-            
             byte[] data = new byte[_bufferSize];
             int len;
             long total = 0;
@@ -523,7 +515,6 @@ public class MultiPartFormInputStream
                 
                 if (len > 0)
                 {
-                    
                     // keep running total of size of bytes read from input and throw an exception if exceeds MultipartConfigElement._maxRequestSize
                     total += len;
                     if (_config.getMaxRequestSize() > 0 && total > _config.getMaxRequestSize())
@@ -573,6 +564,10 @@ public class MultiPartFormInputStream
         catch (Throwable e)
         {
             _err = e;
+
+            // Notify parser if failure occurs
+            if (parser != null)
+                parser.parse(BufferUtil.EMPTY_BUFFER, true);
         }
     }
     
@@ -720,6 +715,16 @@ public class MultiPartFormInputStream
         {
             if (LOG.isDebugEnabled())
                 LOG.debug("Early EOF {}", MultiPartFormInputStream.this);
+
+            try
+            {
+                if (_part != null)
+                    _part.close();
+            }
+            catch (IOException e)
+            {
+                LOG.warn("part could not be closed", e);
+            }
         }
         
         public void reset()

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

@@ -131,12 +131,8 @@ public class MultiPartParser
     private HttpTokens.Token next(ByteBuffer buffer)
     {
         byte ch = buffer.get();
-
         HttpTokens.Token t = HttpTokens.TOKENS[0xff & ch];
         
-        if (DEBUG)
-            LOG.debug("token={}",t);
-        
         switch(t.getType())
         {
             case CNTL:
@@ -271,6 +267,9 @@ public class MultiPartParser
     /* ------------------------------------------------------------------------------- */
     private void parsePreamble(ByteBuffer buffer)
     {
+        if (LOG.isDebugEnabled())
+            LOG.debug("parsePreamble({})", BufferUtil.toDetailString(buffer));
+
         if (_partialBoundary > 0)
         {
             int partial = _delimiterSearch.startsWith(buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.remaining(), _partialBoundary);
@@ -307,6 +306,9 @@ public class MultiPartParser
     /* ------------------------------------------------------------------------------- */
     private void parseDelimiter(ByteBuffer buffer)
     {
+        if (LOG.isDebugEnabled())
+            LOG.debug("parseDelimiter({})", BufferUtil.toDetailString(buffer));
+
         while (__delimiterStates.contains(_state) && hasNextByte(buffer))
         {
             HttpTokens.Token t = next(buffer);
@@ -354,6 +356,9 @@ public class MultiPartParser
      */
     protected boolean parseMimePartHeaders(ByteBuffer buffer)
     {
+        if (LOG.isDebugEnabled())
+            LOG.debug("parseMimePartHeaders({})", BufferUtil.toDetailString(buffer));
+
         // Process headers
         while (_state == State.BODY_PART && hasNextByte(buffer))
         {
@@ -575,6 +580,8 @@ public class MultiPartParser
     
     protected boolean parseOctetContent(ByteBuffer buffer)
     {
+        if (LOG.isDebugEnabled())
+            LOG.debug("parseOctetContent({})", BufferUtil.toDetailString(buffer));
         
         // Starts With
         if (_partialBoundary > 0)

jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormInputStreamTest.java

@@ -217,6 +217,7 @@ public class MultiPartFormInputStreamTest
 
     @Test
     public void testNoBody()
+            throws Exception
     {
         String body = "";
 
@@ -277,6 +278,7 @@ public class MultiPartFormInputStreamTest
 
     @Test
     public void testWhitespaceBodyWithCRLF()
+            throws Exception
     {
         String whitespace = "              \n\n\n\r\n\r\n\r\n\r\n";
 
@@ -292,6 +294,7 @@ public class MultiPartFormInputStreamTest
 
     @Test
     public void testWhitespaceBody()
+            throws Exception
     {
         String whitespace = " ";
 
@@ -400,6 +403,7 @@ public class MultiPartFormInputStreamTest
 
     @Test
     public void testRequestTooBig ()
+            throws Exception
     {
         MultipartConfigElement config = new MultipartConfigElement(_dirname, 60, 100, 50);
         MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@@ -415,6 +419,7 @@ public class MultiPartFormInputStreamTest
     
     @Test
     public void testRequestTooBigThrowsErrorOnGetParts ()
+            throws Exception
     {
         MultipartConfigElement config = new MultipartConfigElement(_dirname, 60, 100, 50);
         MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@@ -434,6 +439,7 @@ public class MultiPartFormInputStreamTest
 
     @Test
     public void testFileTooBig()
+            throws Exception
     {
         MultipartConfigElement config = new MultipartConfigElement(_dirname, 40, 1024, 30);
         MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@@ -449,6 +455,7 @@ public class MultiPartFormInputStreamTest
     
     @Test
     public void testFileTooBigThrowsErrorOnGetParts()
+            throws Exception
     {
         MultipartConfigElement config = new MultipartConfigElement(_dirname, 40, 1024, 30);
         MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@@ -550,6 +557,7 @@ public class MultiPartFormInputStreamTest
     
     @Test
     public void testCROnlyRequest()
+            throws Exception
     {
         String str = "--AaB03x\r" +
                 "content-disposition: form-data; name=\"field1\"\r" +
@@ -576,6 +584,7 @@ public class MultiPartFormInputStreamTest
 
     @Test
     public void testCRandLFMixRequest()
+            throws Exception
     {
         String str = "--AaB03x\r" +
                 "content-disposition: form-data; name=\"field1\"\r" +

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

@@ -37,8 +37,8 @@ import org.eclipse.jetty.server.handler.ContextHandler.Context;
  */
 public interface MultiParts extends Closeable
 {   
-    Collection<Part> getParts();
+    Collection<Part> getParts() throws IOException;
-    Part getPart(String name);
+    Part getPart(String name) throws IOException;
     boolean isEmpty();
     ContextHandler.Context getContext();
 
@@ -51,32 +51,18 @@ public interface MultiParts extends Closeable
         {
             _httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir);
             _context = request.getContext();
-            _httpParser.getParts();
         }
 
         @Override
-        public Collection<Part> getParts() 
+        public Collection<Part> getParts() throws IOException
         {
-            try
+            return _httpParser.getParts();
-            {
-                return _httpParser.getParts();
-            }
-            catch (IOException e)
-            {
-                throw new RuntimeException(e);
-            }
         }
 
         @Override
-        public Part getPart(String name) {
+        public Part getPart(String name) throws IOException
-            try
+        {
-            {
+            return _httpParser.getPart(name);
-                return _httpParser.getPart(name);
-            }
-            catch (IOException e)
-            {
-                throw new RuntimeException(e);
-            }
         }
 
         @Override
@@ -96,6 +82,5 @@ public interface MultiParts extends Closeable
         {
             return _context;
         }
-
     }
 }

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

@@ -477,7 +477,7 @@ public class Request implements HttpServletRequest
                             throw new BadMessageException(HttpStatus.NOT_IMPLEMENTED_501,"Unsupported Content-Encoding");
                         getParts(_contentParameters);
                     }
-                    catch (IOException | ServletException e)
+                    catch (IOException e)
                     {
                         LOG.debug(e);
                         throw new RuntimeIOException(e);
@@ -2317,7 +2317,6 @@ public class Request implements HttpServletRequest
     public Part getPart(String name) throws IOException, ServletException
     {
         getParts();
-
         return _multiParts.getPart(name);
     }
 
@@ -2331,7 +2330,7 @@ public class Request implements HttpServletRequest
         return getParts(null);
     }
 
-    private Collection<Part> getParts(MultiMap<String> params) throws IOException, ServletException
+    private Collection<Part> getParts(MultiMap<String> params) throws IOException
     {        
         if (_multiParts == null)
             _multiParts = (MultiParts)getAttribute(__MULTIPARTS);
@@ -2342,12 +2341,9 @@ public class Request implements HttpServletRequest
             if (config == null)
                 throw new IllegalStateException("No multipart config for servlet");
 
-            _multiParts = newMultiParts(getInputStream(),
+            _multiParts = newMultiParts(config);
-                                       getContentType(), config,
-                                       (_context != null?(File)_context.getAttribute("javax.servlet.context.tempdir"):null));
-
             setAttribute(__MULTIPARTS, _multiParts);
-            Collection<Part> parts = _multiParts.getParts(); //causes parsing
+            Collection<Part> parts = _multiParts.getParts();
                        
             String _charset_ = null;
             Part charsetPart = _multiParts.getPart("_charset_");
@@ -2409,9 +2405,9 @@ public class Request implements HttpServletRequest
     }
 
 
-    private MultiParts newMultiParts(ServletInputStream inputStream, String contentType, MultipartConfigElement config, Object object) throws IOException
+    private MultiParts newMultiParts(MultipartConfigElement config) throws IOException
     {
-        return new MultiParts.MultiPartsHttpParser(getInputStream(), contentType, config,
+                return new MultiParts.MultiPartsHttpParser(getInputStream(), getContentType(), config,
                 (_context != null ? (File) _context.getAttribute("javax.servlet.context.tempdir") : null), this);
     }
 

jetty-servlet/pom.xml

@@ -65,6 +65,12 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+      <dependency>
+          <groupId>org.eclipse.jetty</groupId>
+          <artifactId>jetty-client</artifactId>
+          <version>${project.version}</version>
+          <scope>test</scope>
+      </dependency>
   </dependencies>
 
 </project>

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java

@@ -748,11 +748,14 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
         //cleaned up correctly
         if (((Registration)getRegistration()).getMultipartConfig() != null)
         {
+            if (LOG.isDebugEnabled())
+                LOG.debug("multipart cleanup listener added for {}", this);
+
             //Register a listener to delete tmp files that are created as a result of this
             //servlet calling Request.getPart() or Request.getParts()
-
             ContextHandler ch = ContextHandler.getContextHandler(getServletHandler().getServletContext());
-            ch.addEventListener(MultiPartCleanerListener.INSTANCE);
+            if(!Arrays.asList(ch.getEventListeners()).contains(MultiPartCleanerListener.INSTANCE))
+                ch.addEventListener(MultiPartCleanerListener.INSTANCE);
         }
     }
 

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

@@ -0,0 +1,164 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.servlet;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.stream.Stream;
+import javax.servlet.MultipartConfigElement;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.Part;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.util.BytesContentProvider;
+import org.eclipse.jetty.client.util.MultiPartContentProvider;
+import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.http.HttpScheme;
+import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.http.MultiPartFormInputStream;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.MultiPartFormDataCompliance;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.util.IO;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.log.StacklessLogging;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class MultiPartServletTest
+{
+    private static final Logger LOG = Log.getLogger(MultiPartServletTest.class);
+
+    private Server server;
+    private ServerConnector connector;
+    private HttpClient client;
+    private Path tmpDir;
+
+    private static final int MAX_FILE_SIZE = 512 * 1024;
+    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 MultiPartServlet extends HttpServlet
+    {
+        @Override
+        protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            if (!req.getContentType().contains(MimeTypes.Type.MULTIPART_FORM_DATA.asString()))
+            {
+                resp.setContentType("text/plain");
+                resp.getWriter().println("not content type " + MimeTypes.Type.MULTIPART_FORM_DATA);
+                resp.getWriter().println("contentType: " + req.getContentType());
+                return;
+            }
+
+            resp.setContentType("text/plain");
+            for (Part part : req.getParts())
+            {
+                resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize());
+            }
+        }
+    }
+
+    @BeforeEach
+    public void start() throws Exception
+    {
+        tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName());
+
+        server = new Server();
+        connector = new ServerConnector(server);
+        server.addConnector(connector);
+
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
+        contextHandler.setContextPath("/");
+        ServletHolder servletHolder = contextHandler.addServlet(MultiPartServlet.class, "/");
+
+        MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
+                MAX_FILE_SIZE, -1, 1);
+        servletHolder.getRegistration().setMultipartConfig(config);
+
+        server.setHandler(contextHandler);
+
+        server.start();
+
+        client = new HttpClient();
+        client.start();
+    }
+
+    @AfterEach
+    public void stop() throws Exception
+    {
+        client.stop();
+        server.stop();
+
+        IO.delete(tmpDir.toFile());
+    }
+
+    @ParameterizedTest
+    @MethodSource("data")
+    public void testTempFilesDeletedOnError(MultiPartFormDataCompliance compliance) throws Exception
+    {
+        connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration()
+                .setMultiPartFormDataCompliance(compliance);
+
+        byte[] byteArray = new byte[LARGE_MESSAGE_SIZE];
+        for (int i=0; i<byteArray.length; i++)
+            byteArray[i] = 1;
+        BytesContentProvider contentProvider = new BytesContentProvider(byteArray);
+
+        MultiPartContentProvider multiPart = new MultiPartContentProvider();
+        multiPart.addFieldPart("largePart", contentProvider, null);
+        multiPart.close();
+
+        try (StacklessLogging stacklessLogging = new StacklessLogging(HttpChannel.class, MultiPartFormInputStream.class))
+        {
+            ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
+                    .scheme(HttpScheme.HTTP.asString())
+                    .method(HttpMethod.POST)
+                    .content(multiPart)
+                    .send();
+
+            assertEquals(500, response.getStatus());
+            assertThat(response.getContentAsString(),
+                    containsString("Multipart Mime part largePart exceeds max filesize"));
+        }
+
+        assertThat(tmpDir.toFile().list().length, is(0));
+    }
+}