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 6a865ffc5f1c..dbfd90c7f4aa 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 @@ -2050,7 +2050,12 @@ private Collection getParts(Fields params) throws IOException { parts = _multiParts.getParts(); } - catch (IOException e) + catch (BadMessageException e) + { + throw e; + } + // Catch RuntimeException to handle IllegalStateException, IllegalArgumentException, CharacterEncodingException, etc .. (long list) + catch (RuntimeException | IOException e) { throw new BadMessageException("Unable to parse form content", e); } diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/FormTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/FormTest.java index f929b264987d..48f08bf0f9ed 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/FormTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/FormTest.java @@ -13,9 +13,12 @@ package org.eclipse.jetty.ee9.servlet; +import java.io.IOException; +import java.io.PrintWriter; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -28,6 +31,7 @@ import org.eclipse.jetty.client.ContentResponse; import org.eclipse.jetty.client.FormRequestContent; import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.StringRequestContent; import org.eclipse.jetty.ee9.nested.ContextHandler; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; @@ -42,6 +46,8 @@ 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.junit.jupiter.api.Assertions.assertEquals; public class FormTest @@ -229,4 +235,98 @@ protected void service(HttpServletRequest request, HttpServletResponse response) assertEquals(HttpStatus.OK_200, response.getStatus()); } + + public static Stream invalidForm() + { + return Stream.of( + Arguments.of("%A", "java.lang.IllegalArgumentException: Not valid encoding '%A?'"), + Arguments.of("name%", "java.lang.IllegalArgumentException: Not valid encoding '%??'"), + Arguments.of("name%A", "java.lang.IllegalArgumentException: Not valid encoding '%A?'"), + Arguments.of("name%A&", "java.lang.IllegalArgumentException: Not valid encoding '%A&'"), + Arguments.of("name=%", "java.lang.IllegalArgumentException: Not valid encoding '%??'"), + Arguments.of("name=A%%A", "java.lang.IllegalArgumentException: Not valid encoding '%%A'"), + Arguments.of("name=A%%3D", "java.lang.IllegalArgumentException: Not valid encoding '%%3'"), + Arguments.of("%=", "java.lang.IllegalArgumentException: Not valid encoding '%=?'"), + Arguments.of("name=%A", "java.lang.IllegalArgumentException: Not valid encoding '%A?'"), + Arguments.of("name=value%A", "ava.lang.IllegalArgumentException: Not valid encoding '%A?'"), + Arguments.of("n%AH=v", "java.lang.IllegalArgumentException: Not valid encoding '%AH'"), + Arguments.of("n=v%AH", "java.lang.IllegalArgumentException: Not valid encoding '%AH'") + ); + } + + @ParameterizedTest + @MethodSource("invalidForm") + public void testContentTypeWithoutCharsetDecodeBadUTF8(String rawForm, String expectedCauseMessage) throws Exception + { + start(handler -> new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) + { + // This is expected to throw an exception due to the bad form input + request.getParameterMap(); + } + }); + + StringRequestContent requestContent = new StringRequestContent("application/x-www-form-urlencoded", rawForm); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .method(HttpMethod.POST) + .path(contextPath + servletPath) + .body(requestContent) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.BAD_REQUEST_400, response.getStatus(), response::getContentAsString); + String responseContent = response.getContentAsString(); + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString(expectedCauseMessage)); + } + + public static Stream utf8Form() + { + return Stream.of( + Arguments.of("euro=%E2%82%AC", List.of("param[euro] = \"€\"")), + Arguments.of("name=%AB%CD", List.of("param[name] = \"��\"")), + Arguments.of("name=x%AB%CDz", List.of("param[name] = \"x��z\"")), + Arguments.of("name=%FF%FF%FF%FF", List.of("param[name] = \"����\"")) + ); + } + + @ParameterizedTest + @MethodSource("utf8Form") + public void testUtf8Decoding(String rawForm, List expectedParams) throws Exception + { + start(handler -> new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setCharacterEncoding("utf-8"); + response.setContentType("text/plain"); + PrintWriter out = response.getWriter(); + + Map paramMap = request.getParameterMap(); + List names = paramMap.keySet().stream().sorted().toList(); + for (String name: names) + { + out.printf("param[%s] = \"%s\"%n", name, String.join(",", paramMap.get(name))); + } + } + }); + + StringRequestContent requestContent = new StringRequestContent("application/x-www-form-urlencoded", rawForm); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .method(HttpMethod.POST) + .path(contextPath + servletPath) + .body(requestContent) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus(), response::getContentAsString); + String responseContent = response.getContentAsString(); + for (String expectedParam: expectedParams) + assertThat(responseContent, containsString(expectedParam)); + } } 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 fbe745cee8bc..e9434e139bbe 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 @@ -15,6 +15,7 @@ import java.io.IOException; import java.io.InputStream; +import java.io.PrintWriter; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -104,18 +105,27 @@ public static class MultiPartServlet extends HttpServlet @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.setCharacterEncoding("utf-8"); + resp.setContentType("text/plain"); + + PrintWriter out = resp.getWriter(); + 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()); + out.println("not content type " + MimeTypes.Type.MULTIPART_FORM_DATA); + out.println("contentType: " + req.getContentType()); return; } - resp.setContentType("text/plain"); for (Part part : req.getParts()) { - resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize()); + out.printf("Part: name=%s, size=%s", part.getName(), part.getSize()); + if (part.getSize() <= 100) + { + String content = IO.toString(part.getInputStream()); + out.printf(", content=%s", content); + } + out.println(); } } } @@ -302,6 +312,7 @@ public void testLineFeedCarriageReturnEOL(MultiPartCompliance multiPartComplianc startServer(multiPartCompliance); String contentType = "multipart/form-data; boundary=---------------------------7e25e1e151054"; + // NOTE: The extra `\r` here are intentional, do not remove. String rawForm = """ -----------------------------7e25e1e151054\r Content-Disposition: form-data; name="user"\r @@ -370,6 +381,131 @@ public void testAllWhitespaceForm(MultiPartCompliance multiPartCompliance) throw }); } + /** + * A part with Content-Transfer-Encoding: base64, and the content is valid Base64 encoded. + * + * MultiPartCompliance mode set to allow MultiPartCompliance.Violation.BASE64_TRANSFER_ENCODING + */ + @Test + public void testLegacyContentTransferEncodingBase64Allowed() throws Exception + { + MultiPartCompliance legacyBase64 = MultiPartCompliance.from("LEGACY,BASE64_TRANSFER_ENCODING"); + + startServer(legacyBase64); + + String contentType = "multipart/form-data; boundary=8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp"; + String rawForm = """ + --8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp + Content-ID: + Content-Disposition: form-data; name="quote" + Content-Transfer-Encoding: base64 + + IkJvb2tzIGFyZSB0aGUgbGliZXJhdGVkIHNwaXJpdHMgb2YgbWVuLiIgLS0gTWFyayBUd2Fpbg== + --8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp-- + + """; + + StringRequestContent form = new StringRequestContent( + contentType, + rawForm + ); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .path("/") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(form) + .send(); + + assertEquals(200, response.getStatus()); + assertThat(response.getContentAsString(), containsString("Part: name=quote, size=55, content=\"Books are the liberated spirits of men.\" -- Mark Twain")); + } + + /** + * A part with Content-Transfer-Encoding: base64, but the content is not actually encoded in Base 64. + * + * MultiPartCompliance mode set to allow MultiPartCompliance.Violation.BASE64_TRANSFER_ENCODING + */ + @Test + public void testLegacyContentTransferEncodingBadBase64Allowed() throws Exception + { + MultiPartCompliance legacyBase64 = MultiPartCompliance.from("LEGACY,BASE64_TRANSFER_ENCODING"); + + startServer(legacyBase64); + + String contentType = "multipart/form-data; boundary=8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp"; + String rawForm = """ + --8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp + Content-ID: + Content-Disposition: form-data; name="quote" + Content-Transfer-Encoding: base64 + + "Travel is fatal to prejudice." -- Mark Twain + --8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp-- + + """; + + StringRequestContent form = new StringRequestContent( + contentType, + rawForm + ); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/") + .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("java.lang.IllegalArgumentException: Last unit does not have enough valid bits")); + }); + } + + /** + * A part with Content-Transfer-Encoding: base64, and the content is valid Base64 encoded. + * + * MultiPartCompliance mode set to allow MultiPartCompliance.LEGACY, which does not perform + * base64 decoding. + */ + @Test + public void testLegacyContentTransferEncodingBase64() throws Exception + { + MultiPartCompliance legacyBase64 = MultiPartCompliance.LEGACY; + + startServer(legacyBase64); + + String contentType = "multipart/form-data; boundary=8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp"; + String rawForm = """ + --8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp + Content-ID: + Content-Disposition: form-data; name="quote" + Content-Transfer-Encoding: base64 + + IkJvb2tzIGFyZSB0aGUgbGliZXJhdGVkIHNwaXJpdHMgb2YgbWVuLiIgLS0gTWFyayBUd2Fpbg== + --8GbcZNTauFWYMt7GeM9BxFMdlNBJ6aLJhGdXp-- + + """; + + StringRequestContent form = new StringRequestContent( + contentType, + rawForm + ); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .path("/") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(form) + .send(); + + assertEquals(200, response.getStatus()); + assertThat(response.getContentAsString(), containsString("Part: name=quote, size=76, content=IkJvb2tzIGFyZSB0aGUgbGliZXJhdGVkIHNwaXJpdHMgb2YgbWVuLiIgLS0gTWFyayBUd2Fpbg==")); + } + @ParameterizedTest @MethodSource("multipartModes") public void testManyParts(MultiPartCompliance multiPartCompliance) throws Exception @@ -487,7 +623,7 @@ public void testTempFilesDeletedOnError(MultiPartCompliance multiPartCompliance) .body(multiPart) .send(); - assertEquals(500, response.getStatus()); + assertEquals(400, response.getStatus()); assertThat(response.getContentAsString(), containsString("Multipart Mime part largePart exceeds max filesize")); }