From fd78e2b76c4a7b9c8ab9c00fd40e03f08e071168 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 11 Jul 2024 14:25:03 -0500 Subject: [PATCH 1/5] Regression on ee9 / ee8 MultiPart parsing behavior with empty request body --- .../ee9/nested/MultiPartFormInputStream.java | 9 +++ .../MultiPartInputStreamLegacyParser.java | 8 ++- .../ee9/servlet/MultiPartServletTest.java | 60 +++++++++++++++++-- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java index df32f184dd40..ae90621c1fb1 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java @@ -38,6 +38,7 @@ import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.Part; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.MultiPartCompliance; @@ -617,6 +618,14 @@ protected void parse() } else if (len == -1) { + if (total == 0) + { + throw new BadMessageException( + "No progress made on multipart/form-data", + new IOException("Missing content for multipart request") + ); + } + parser.parse(BufferUtil.EMPTY_BUFFER, true); break; } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java index f26ac5a62175..d3958560c3a6 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java @@ -41,6 +41,7 @@ import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.Part; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.MultiPartCompliance; import org.eclipse.jetty.util.ByteArrayOutputStream2; @@ -578,7 +579,12 @@ else if ("".equals(_config.getLocation())) } if (line == null) - throw new IOException("Missing content for multipart request"); + { + throw new BadMessageException( + "No progress made on multipart/form-data", + new IOException("Missing content for multipart request") + ); + } boolean badFormatLogged = false; diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java index b351e8bc3e22..6a91c4a33d45 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java +++ b/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,46 @@ public void stop() throws Exception IO.delete(tmpDir.toFile()); } + public static Stream 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)); + + String responseBody = IO.toString(listener.getInputStream()); + assertThat(responseBody, containsString("java.io.IOException: Missing content for multipart request")); + } + @Test public void testLargePart() throws Exception { + startServer(MultiPartCompliance.RFC7578); + OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content)); @@ -215,6 +259,8 @@ public void testLargePart() throws Exception @Test public void testManyParts() throws Exception { + startServer(MultiPartCompliance.RFC7578); + byte[] byteArray = new byte[1024]; Arrays.fill(byteArray, (byte)1); @@ -244,6 +290,8 @@ public void testManyParts() throws Exception @Test public void testMaxRequestSize() throws Exception { + startServer(MultiPartCompliance.RFC7578); + OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content)); @@ -304,6 +352,8 @@ private static void assert400orEof(InputStreamResponseListener listener, Consume @Test public void testTempFilesDeletedOnError() throws Exception { + startServer(MultiPartCompliance.RFC7578); + byte[] byteArray = new byte[LARGE_MESSAGE_SIZE]; Arrays.fill(byteArray, (byte)1); BytesRequestContent content = new BytesRequestContent(byteArray); @@ -333,6 +383,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); From 7e58921a5a4d60659d0c4a234079858350c53812 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 15 Jul 2024 16:00:50 -0500 Subject: [PATCH 2/5] More testing of regression. + More multipart parsing errors return 400, not 500 now. --- .../MultiPartInputStreamLegacyParser.java | 2 +- .../org/eclipse/jetty/ee9/nested/Request.java | 2 +- .../ee9/servlet/MultiPartServletTest.java | 147 ++++++++++++++++-- 3 files changed, 133 insertions(+), 18 deletions(-) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java index d3958560c3a6..a531fac72d83 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java @@ -705,7 +705,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); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java index 650857f0d215..9bb9d6189759 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java +++ b/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); } } } diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java index 6a91c4a33d45..fceb7daf96c6 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java @@ -60,7 +60,6 @@ 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.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -222,10 +221,11 @@ public void testEmptyBodyMultipartForm(MultiPartCompliance multiPartCompliance) assertThat(responseBody, containsString("java.io.IOException: Missing content for multipart request")); } - @Test - public void testLargePart() throws Exception + @ParameterizedTest + @MethodSource("multipartModes") + public void testLargePart(MultiPartCompliance multiPartCompliance) throws Exception { - startServer(MultiPartCompliance.RFC7578); + startServer(multiPartCompliance); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -256,10 +256,122 @@ public void testLargePart() throws Exception }); } - @Test - public void testManyParts() throws Exception + @ParameterizedTest + @MethodSource("multipartModes") + public void testIncompleteMultipart(MultiPartCompliance multiPartCompliance) throws Exception { - startServer(MultiPartCompliance.RFC7578); + 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 extract content parameters")); + 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 -> + { + if (multiPartCompliance == MultiPartCompliance.RFC7578) + { + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Illegal character ALPHA='s&apos")); + } + else if (multiPartCompliance == MultiPartCompliance.LEGACY) + { + assertThat(responseContent, containsString("Unable to extract content parameters")); + assertThat(responseContent, containsString("Incomplete parts")); + } + }); + } + + @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 extract content parameters")); + assertThat(responseContent, containsString("Missing initial multi part boundary")); + }); + } + + @ParameterizedTest + @MethodSource("multipartModes") + public void testManyParts(MultiPartCompliance multiPartCompliance) throws Exception + { + startServer(multiPartCompliance); byte[] byteArray = new byte[1024]; Arrays.fill(byteArray, (byte)1); @@ -287,10 +399,11 @@ public void testManyParts() throws Exception }); } - @Test - public void testMaxRequestSize() throws Exception + @ParameterizedTest + @MethodSource("multipartModes") + public void testMaxRequestSize(MultiPartCompliance multiPartCompliance) throws Exception { - startServer(MultiPartCompliance.RFC7578); + startServer(multiPartCompliance); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -349,10 +462,11 @@ 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.RFC7578); + startServer(multiPartCompliance); byte[] byteArray = new byte[LARGE_MESSAGE_SIZE]; Arrays.fill(byteArray, (byte)1); @@ -380,10 +494,11 @@ public void testTempFilesDeletedOnError() throws Exception assertThat(fileList.length, is(0)); } - @Test - public void testMultiPartGzip() throws Exception + @ParameterizedTest + @MethodSource("multipartModes") + public void testMultiPartGzip(MultiPartCompliance multiPartCompliance) throws Exception { - startServer(MultiPartCompliance.RFC7578); + startServer(multiPartCompliance); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; From 0b72d81dfd0be457dd4b9ca1da4c1ccf4bb02f11 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 15 Jul 2024 16:14:48 -0500 Subject: [PATCH 3/5] Fix test assumptions --- .../jetty/ee9/nested/MultiPartInputStreamLegacyParser.java | 2 +- .../org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java index a531fac72d83..01d88b010ceb 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java @@ -869,7 +869,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) { diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java index fceb7daf96c6..ca043fff251b 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java @@ -333,7 +333,7 @@ public void testLineFeedCarriageReturnEOL(MultiPartCompliance multiPartComplianc else if (multiPartCompliance == MultiPartCompliance.LEGACY) { assertThat(responseContent, containsString("Unable to extract content parameters")); - assertThat(responseContent, containsString("Incomplete parts")); + assertThat(responseContent, containsString("Incomplete Multipart")); } }); } From 7085a50ef128d131110a10fe8dfb667edcb51f43 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 15 Jul 2024 20:36:14 -0500 Subject: [PATCH 4/5] Reworked so that Request handles Bad Message from Multipart Parsing. * Request.getParts() catches IOExceptions from multiParts.getParts() and wraps them in a BadMessageException to ensure that a 400 Bad Request response shows up. --- .../ee9/nested/MultiPartFormInputStream.java | 11 +----- .../MultiPartInputStreamLegacyParser.java | 10 +---- .../org/eclipse/jetty/ee9/nested/Request.java | 11 +++++- .../ee9/servlet/MultiPartServletTest.java | 38 ++++++++++--------- 4 files changed, 34 insertions(+), 36 deletions(-) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java index ae90621c1fb1..10bc336b0e4e 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java @@ -38,7 +38,6 @@ import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.Part; -import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.MultiPartCompliance; @@ -618,14 +617,6 @@ protected void parse() } else if (len == -1) { - if (total == 0) - { - throw new BadMessageException( - "No progress made on multipart/form-data", - new IOException("Missing content for multipart request") - ); - } - parser.parse(BufferUtil.EMPTY_BUFFER, true); break; } @@ -641,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"); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java index 01d88b010ceb..cae02359dd17 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartInputStreamLegacyParser.java @@ -41,7 +41,6 @@ import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.Part; -import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.MultiPartCompliance; import org.eclipse.jetty.util.ByteArrayOutputStream2; @@ -579,12 +578,7 @@ else if ("".equals(_config.getLocation())) } if (line == null) - { - throw new BadMessageException( - "No progress made on multipart/form-data", - new IOException("Missing content for multipart request") - ); - } + throw new IOException("Missing content for multipart request"); boolean badFormatLogged = false; @@ -604,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)) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java index 9bb9d6189759..6a865ffc5f1c 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java @@ -2044,7 +2044,16 @@ private Collection getParts(Fields params) throws IOException MultiPartCompliance multiPartCompliance = getHttpChannel().getHttpConfiguration().getMultiPartCompliance(); _multiParts = newMultiParts(multiPartCompliance, config, maxFormKeys); - Collection parts = _multiParts.getParts(); + + Collection parts; + try + { + parts = _multiParts.getParts(); + } + catch (IOException e) + { + throw new BadMessageException("Unable to parse form content", e); + } reportComplianceViolations(); String formCharset = null; diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java index ca043fff251b..d278df3b1fe0 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java @@ -60,6 +60,7 @@ 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.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -217,8 +218,11 @@ public void testEmptyBodyMultipartForm(MultiPartCompliance multiPartCompliance) Response response = listener.get(60, TimeUnit.SECONDS); assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); - String responseBody = IO.toString(listener.getInputStream()); - assertThat(responseBody, containsString("java.io.IOException: Missing content for multipart request")); + assert400orEof(listener, responseContent -> + { + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Missing content for multipart request")); + }); } @ParameterizedTest @@ -286,7 +290,7 @@ public void testIncompleteMultipart(MultiPartCompliance multiPartCompliance) thr assert400orEof(listener, responseContent -> { - assertThat(responseContent, containsString("Unable to extract content parameters")); + assertThat(responseContent, containsString("Unable to parse form content")); assertThat(responseContent, containsString("Incomplete Multipart")); }); } @@ -325,14 +329,13 @@ public void testLineFeedCarriageReturnEOL(MultiPartCompliance multiPartComplianc assert400orEof(listener, responseContent -> { + assertThat(responseContent, containsString("Unable to parse form content")); if (multiPartCompliance == MultiPartCompliance.RFC7578) { - assertThat(responseContent, containsString("Unable to parse form content")); assertThat(responseContent, containsString("Illegal character ALPHA='s&apos")); } else if (multiPartCompliance == MultiPartCompliance.LEGACY) { - assertThat(responseContent, containsString("Unable to extract content parameters")); assertThat(responseContent, containsString("Incomplete Multipart")); } }); @@ -362,8 +365,8 @@ public void testAllWhitespaceForm(MultiPartCompliance multiPartCompliance) throw assert400orEof(listener, responseContent -> { - assertThat(responseContent, containsString("Unable to extract content parameters")); - assertThat(responseContent, containsString("Missing initial multi part boundary")); + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Missing content for multipart request")); }); } @@ -494,11 +497,10 @@ public void testTempFilesDeletedOnError(MultiPartCompliance multiPartCompliance) assertThat(fileList.length, is(0)); } - @ParameterizedTest - @MethodSource("multipartModes") - public void testMultiPartGzip(MultiPartCompliance multiPartCompliance) throws Exception + @Test + public void testMultiPartGzip() throws Exception { - startServer(multiPartCompliance); + startServer(MultiPartCompliance.RFC7578); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; @@ -524,12 +526,14 @@ public void testMultiPartGzip(MultiPartCompliance multiPartCompliance) throws Ex 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 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 parts = new ArrayList<>(mpis.getParts()); + assertThat(parts.size(), is(1)); + assertThat(IO.toString(parts.get(0).getInputStream()), is(contentString)); + } } } } From 68bc1fb3e07db67c8f05c5e3e0ab94a1129a997c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 15 Jul 2024 20:52:57 -0500 Subject: [PATCH 5/5] Fix checkstyle --- .../org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java index d278df3b1fe0..fbe745cee8bc 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java @@ -526,7 +526,7 @@ public void testMultiPartGzip() throws Exception assertThat(headers.get(HttpHeader.CONTENT_TYPE), startsWith("multipart/form-data")); assertThat(headers.get(HttpHeader.CONTENT_ENCODING), is("gzip")); - try(InputStream inputStream = new GZIPInputStream(responseStream.getInputStream())) + try (InputStream inputStream = new GZIPInputStream(responseStream.getInputStream())) { String contentType = headers.get(HttpHeader.CONTENT_TYPE); MultiPartFormInputStream mpis = new MultiPartFormInputStream(inputStream, contentType, null, null);