Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Addressing more regressions on form handling in ee9 / ee8 environments #12072

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Addressing more regressions on form handling in ee9 / ee8 environments #12072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2050,7 +2050,12 @@ private Collection<Part> 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);
}
Expand Down
100 changes: 100 additions & 0 deletions jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/FormTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -229,4 +235,98 @@ protected void service(HttpServletRequest request, HttpServletResponse response)

assertEquals(HttpStatus.OK_200, response.getStatus());
}

public static Stream<Arguments> invalidForm()
{
return Stream.of(
Arguments.of("%A", "java.lang.IllegalArgumentException: Not valid encoding &apos;%A?&apos;"),
Arguments.of("name%", "java.lang.IllegalArgumentException: Not valid encoding &apos;%??&apos;"),
Arguments.of("name%A", "java.lang.IllegalArgumentException: Not valid encoding &apos;%A?&apos;"),
Arguments.of("name%A&", "java.lang.IllegalArgumentException: Not valid encoding &apos;%A&amp;&apos;"),
Arguments.of("name=%", "java.lang.IllegalArgumentException: Not valid encoding &apos;%??&apos;"),
Arguments.of("name=A%%A", "java.lang.IllegalArgumentException: Not valid encoding &apos;%%A&apos;"),
Arguments.of("name=A%%3D", "java.lang.IllegalArgumentException: Not valid encoding &apos;%%3&apos;"),
Arguments.of("%=", "java.lang.IllegalArgumentException: Not valid encoding &apos;%=?&apos;"),
Arguments.of("name=%A", "java.lang.IllegalArgumentException: Not valid encoding &apos;%A?&apos;"),
Arguments.of("name=value%A", "ava.lang.IllegalArgumentException: Not valid encoding &apos;%A?&apos;"),
Arguments.of("n%AH=v", "java.lang.IllegalArgumentException: Not valid encoding &apos;%AH&apos;"),
Arguments.of("n=v%AH", "java.lang.IllegalArgumentException: Not valid encoding &apos;%AH&apos;")
);
}

@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<Arguments> 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<String> 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<String, String[]> paramMap = request.getParameterMap();
List<String> 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));
}
}
148 changes: 142 additions & 6 deletions ...9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: <foo@example.org>
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: <foo@example.org>
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: <foo@example.org>
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
Expand Down Expand Up @@ -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"));
}
Expand Down
Loading