Merge pull request #12072 from jetty/fix/12.0.x/ee9-form-regressions
Addressing more regressions on form handling in ee9 / ee8 environments
This commit is contained in:
commit
58d651a69a
|
@ -2050,7 +2050,12 @@ public class Request implements HttpServletRequest
|
|||
{
|
||||
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);
|
||||
}
|
||||
|
|
|
@ -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.AsyncRequestContent;
|
|||
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.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.junit.jupiter.api.Assertions.assertEquals;
|
||||
|
||||
public class FormTest
|
||||
|
@ -229,4 +235,98 @@ public class FormTest
|
|||
|
||||
assertEquals(HttpStatus.OK_200, response.getStatus());
|
||||
}
|
||||
|
||||
public static Stream<Arguments> 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<Arguments> utf8Form()
|
||||
{
|
||||
return Stream.of(
|
||||
Arguments.of("euro=%E2%82%AC", List.of("param[euro] = \"€\"")),
|
||||
Arguments.of("name=%AB%CD", List.of("param[name] = \"<EFBFBD><EFBFBD>\"")),
|
||||
Arguments.of("name=x%AB%CDz", List.of("param[name] = \"x<EFBFBD><EFBFBD>z\"")),
|
||||
Arguments.of("name=%FF%FF%FF%FF", List.of("param[name] = \"<EFBFBD><EFBFBD><EFBFBD><EFBFBD>\""))
|
||||
);
|
||||
}
|
||||
|
||||
@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));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -15,6 +15,7 @@ package org.eclipse.jetty.ee9.servlet;
|
|||
|
||||
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 class MultiPartServletTest
|
|||
@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 class MultiPartServletTest
|
|||
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 class MultiPartServletTest
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
|
@ -487,7 +623,7 @@ public class MultiPartServletTest
|
|||
.body(multiPart)
|
||||
.send();
|
||||
|
||||
assertEquals(500, response.getStatus());
|
||||
assertEquals(400, response.getStatus());
|
||||
assertThat(response.getContentAsString(),
|
||||
containsString("Multipart Mime part largePart exceeds max filesize"));
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue