Fix Regression on ee9 / ee8 MultiPart parsing

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java

@@ -632,7 +632,7 @@ else if (len == -1)
             if (parser.getState() != MultiPartParser.State.END)
             {
                 if (parser.getState() == MultiPartParser.State.PREAMBLE)
-                    _err = new IOException("Missing initial multi part boundary");
+                    _err = new IOException("Missing content for multipart request");
                 else
                     _err = new IOException("Incomplete Multipart");
             }

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java

@@ -598,7 +598,7 @@ else if ("".equals(_config.getLocation()))
             }
 
             if (line == null || line.length() == 0)
-                throw new IOException("Missing initial multi part boundary");
+                throw new IOException("Missing content for multipart request");
 
             // Empty multipart.
             if (line.equals(lastBoundary))
@@ -699,7 +699,7 @@ else if (tl.startsWith("filename="))
                 // 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));
+                    throw new IllegalStateException(String.format("Form with too many keys [%d > %d]", _numParts, _maxParts));
 
                 //Have a new Part
                 MultiPart part = new MultiPart(name, filename);
@@ -863,7 +863,7 @@ public int read() throws IOException
                         MultiPartCompliance.Violation.LF_LINE_TERMINATION, "0x10"));
             }
             else
-                throw new IOException("Incomplete parts");
+                throw new IOException("Incomplete Multipart");
         }
         catch (Exception e)
         {

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java

@@ -515,7 +515,7 @@ else if (MimeTypes.Type.MULTIPART_FORM_DATA.is(baseType) &&
                         String msg = "Unable to extract content parameters";
                         if (LOG.isDebugEnabled())
                             LOG.debug(msg, e);
-                        throw new RuntimeIOException(msg, e);
+                        throw new BadMessageException(msg, e);
                     }
                 }
             }
@@ -2044,7 +2044,16 @@ private Collection<Part> getParts(Fields params) throws IOException
             MultiPartCompliance multiPartCompliance = getHttpChannel().getHttpConfiguration().getMultiPartCompliance();
 
             _multiParts = newMultiParts(multiPartCompliance, config, maxFormKeys);
-            Collection<Part> parts = _multiParts.getParts();
+
+            Collection<Part> parts;
+            try
+            {
+                parts = _multiParts.getParts();
+            }
+            catch (IOException e)
+            {
+                throw new BadMessageException("Unable to parse form content", e);
+            }
             reportComplianceViolations();
 
             String formCharset = null;

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

@@ -24,6 +24,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.function.Consumer;
+import java.util.stream.Stream;
 import java.util.zip.GZIPInputStream;
 
 import jakarta.servlet.MultipartConfigElement;
@@ -49,15 +50,20 @@
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.MimeTypes;
 import org.eclipse.jetty.http.MultiPart;
+import org.eclipse.jetty.http.MultiPartCompliance;
 import org.eclipse.jetty.io.EofException;
 import org.eclipse.jetty.logging.StacklessLogging;
+import org.eclipse.jetty.server.HttpConfiguration;
+import org.eclipse.jetty.server.HttpConnectionFactory;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.handler.gzip.GzipHandler;
 import org.eclipse.jetty.util.IO;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+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;
@@ -130,14 +136,15 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S
         }
     }
 
-    @BeforeEach
-    public void start() throws Exception
+    private void startServer(MultiPartCompliance multiPartCompliance) throws Exception
     {
         tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName());
         assertNotNull(tmpDir);
 
         server = new Server();
-        connector = new ServerConnector(server);
+        HttpConfiguration httpConfiguration = new HttpConfiguration();
+        httpConfiguration.setMultiPartCompliance(multiPartCompliance);
+        connector = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration));
         server.addConnector(connector);
 
         MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
@@ -180,9 +187,50 @@ public void stop() throws Exception
         IO.delete(tmpDir.toFile());
     }
 
-    @Test
-    public void testLargePart() throws Exception
+    public static Stream<Arguments> multipartModes()
+    {
+        return Stream.of(
+            Arguments.of(MultiPartCompliance.RFC7578),
+            Arguments.of(MultiPartCompliance.LEGACY)
+        );
+    }
+
+    /**
+     * The request indicates that it is a multipart/form-data, but no body is sent.
+     */
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testEmptyBodyMultipartForm(MultiPartCompliance multiPartCompliance) throws Exception
+    {
+        startServer(multiPartCompliance);
+
+        String contentType = "multipart/form-data; boundary=---------------boundaryXYZ123";
+        StringRequestContent emptyContent = new StringRequestContent(contentType, "");
+
+        InputStreamResponseListener listener = new InputStreamResponseListener();
+        client.newRequest("localhost", connector.getLocalPort())
+            .path("/defaultConfig")
+            .scheme(HttpScheme.HTTP.asString())
+            .method(HttpMethod.POST)
+            .body(emptyContent)
+            .send(listener);
+
+        Response response = listener.get(60, TimeUnit.SECONDS);
+        assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400));
+
+        assert400orEof(listener, responseContent ->
+        {
+            assertThat(responseContent, containsString("Unable to parse form content"));
+            assertThat(responseContent, containsString("Missing content for multipart request"));
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testLargePart(MultiPartCompliance multiPartCompliance) throws Exception
     {
+        startServer(multiPartCompliance);
+
         OutputStreamRequestContent content = new OutputStreamRequestContent();
         MultiPartRequestContent multiPart = new MultiPartRequestContent();
         multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content));
@@ -212,9 +260,122 @@ public void testLargePart() throws Exception
         });
     }
 
-    @Test
-    public void testManyParts() throws Exception
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testIncompleteMultipart(MultiPartCompliance multiPartCompliance) throws Exception
+    {
+        startServer(multiPartCompliance);
+
+        String contentType = "multipart/form-data; boundary=-------------------------7e21c038151054";
+        String incompleteForm = """
+            ---------------------------7e21c038151054
+            Content-Disposition: form-data; name="description"
+
+            Some data, but incomplete
+            ---------------------------7e21c038151054
+            Content-Disposition: form-d"""; // intentionally incomplete
+
+        StringRequestContent incomplete = new StringRequestContent(
+            contentType,
+            incompleteForm
+        );
+
+        InputStreamResponseListener listener = new InputStreamResponseListener();
+        client.newRequest("localhost", connector.getLocalPort())
+            .path("/defaultConfig")
+            .scheme(HttpScheme.HTTP.asString())
+            .method(HttpMethod.POST)
+            .body(incomplete)
+            .send(listener);
+
+        assert400orEof(listener, responseContent ->
+        {
+            assertThat(responseContent, containsString("Unable to parse form content"));
+            assertThat(responseContent, containsString("Incomplete Multipart"));
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testLineFeedCarriageReturnEOL(MultiPartCompliance multiPartCompliance) throws Exception
+    {
+        startServer(multiPartCompliance);
+
+        String contentType = "multipart/form-data; boundary=---------------------------7e25e1e151054";
+        String rawForm = """
+            -----------------------------7e25e1e151054\r
+            Content-Disposition: form-data; name="user"\r
+                        \r
+            anotheruser\r
+            -----------------------------7e25e1e151054\r
+            Content-Disposition: form-data; name="comment"\r
+                        \r
+            with something to say\r
+            -----------------------------7e25e1e151054--\r
+            """;
+
+        StringRequestContent form = new StringRequestContent(
+            contentType,
+            rawForm
+        );
+
+        InputStreamResponseListener listener = new InputStreamResponseListener();
+        client.newRequest("localhost", connector.getLocalPort())
+            .path("/defaultConfig")
+            .scheme(HttpScheme.HTTP.asString())
+            .method(HttpMethod.POST)
+            .body(form)
+            .send(listener);
+
+        assert400orEof(listener, responseContent ->
+        {
+            assertThat(responseContent, containsString("Unable to parse form content"));
+            if (multiPartCompliance == MultiPartCompliance.RFC7578)
+            {
+                assertThat(responseContent, containsString("Illegal character ALPHA=&apos;s&apos"));
+            }
+            else if (multiPartCompliance == MultiPartCompliance.LEGACY)
+            {
+                assertThat(responseContent, containsString("Incomplete Multipart"));
+            }
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testAllWhitespaceForm(MultiPartCompliance multiPartCompliance) throws Exception
     {
+        startServer(multiPartCompliance);
+
+        String contentType = "multipart/form-data; boundary=----WebKitFormBoundaryjwqONTsAFgubfMZc";
+        String rawForm = " \n \n \n \n \n \n \n \n \n ";
+
+        StringRequestContent form = new StringRequestContent(
+            contentType,
+            rawForm
+        );
+
+        InputStreamResponseListener listener = new InputStreamResponseListener();
+        client.newRequest("localhost", connector.getLocalPort())
+            .path("/defaultConfig")
+            .scheme(HttpScheme.HTTP.asString())
+            .method(HttpMethod.POST)
+            .body(form)
+            .send(listener);
+
+        assert400orEof(listener, responseContent ->
+        {
+            assertThat(responseContent, containsString("Unable to parse form content"));
+            assertThat(responseContent, containsString("Missing content for multipart request"));
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testManyParts(MultiPartCompliance multiPartCompliance) throws Exception
+    {
+        startServer(multiPartCompliance);
+
         byte[] byteArray = new byte[1024];
         Arrays.fill(byteArray, (byte)1);
 
@@ -241,9 +402,12 @@ public void testManyParts() throws Exception
         });
     }
 
-    @Test
-    public void testMaxRequestSize() throws Exception
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testMaxRequestSize(MultiPartCompliance multiPartCompliance) throws Exception
     {
+        startServer(multiPartCompliance);
+
         OutputStreamRequestContent content = new OutputStreamRequestContent();
         MultiPartRequestContent multiPart = new MultiPartRequestContent();
         multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content));
@@ -301,9 +465,12 @@ private static void assert400orEof(InputStreamResponseListener listener, Consume
             checkbody.accept(responseContent);
     }
 
-    @Test
-    public void testTempFilesDeletedOnError() throws Exception
+    @ParameterizedTest
+    @MethodSource("multipartModes")
+    public void testTempFilesDeletedOnError(MultiPartCompliance multiPartCompliance) throws Exception
     {
+        startServer(multiPartCompliance);
+
         byte[] byteArray = new byte[LARGE_MESSAGE_SIZE];
         Arrays.fill(byteArray, (byte)1);
         BytesRequestContent content = new BytesRequestContent(byteArray);
@@ -333,6 +500,8 @@ public void testTempFilesDeletedOnError() throws Exception
     @Test
     public void testMultiPartGzip() throws Exception
     {
+        startServer(MultiPartCompliance.RFC7578);
+
         String contentString = "the quick brown fox jumps over the lazy dog, " +
             "the quick brown fox jumps over the lazy dog";
         StringRequestContent content = new StringRequestContent(contentString);
@@ -357,12 +526,14 @@ public void testMultiPartGzip() throws Exception
             assertThat(headers.get(HttpHeader.CONTENT_TYPE), startsWith("multipart/form-data"));
             assertThat(headers.get(HttpHeader.CONTENT_ENCODING), is("gzip"));
 
-            InputStream inputStream = new GZIPInputStream(responseStream.getInputStream());
-            String contentType = headers.get(HttpHeader.CONTENT_TYPE);
-            MultiPartFormInputStream mpis = new MultiPartFormInputStream(inputStream, contentType, null, null);
-            List<Part> parts = new ArrayList<>(mpis.getParts());
-            assertThat(parts.size(), is(1));
-            assertThat(IO.toString(parts.get(0).getInputStream()), is(contentString));
+            try (InputStream inputStream = new GZIPInputStream(responseStream.getInputStream()))
+            {
+                String contentType = headers.get(HttpHeader.CONTENT_TYPE);
+                MultiPartFormInputStream mpis = new MultiPartFormInputStream(inputStream, contentType, null, null);
+                List<Part> parts = new ArrayList<>(mpis.getParts());
+                assertThat(parts.size(), is(1));
+                assertThat(IO.toString(parts.get(0).getInputStream()), is(contentString));
+            }
         }
     }
 }