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.

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

Fix Regression on ee9 / ee8 MultiPart parsing #12031

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -617,6 +618,14 @@ protected void parse()
}
else if (len == -1)
{
if (total == 0)
{
throw new BadMessageException(
"No progress made on multipart/form-data",
joakime marked this conversation as resolved.
Show resolved Hide resolved
new IOException("Missing content for multipart request")
);
}

parser.parse(BufferUtil.EMPTY_BUFFER, true);
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -699,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);
Expand Down Expand Up @@ -863,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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -49,15 +50,19 @@
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;
Expand Down Expand Up @@ -130,14 +135,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(),
Expand Down Expand Up @@ -180,9 +186,47 @@ 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));

String responseBody = IO.toString(listener.getInputStream());
assertThat(responseBody, containsString("java.io.IOException: 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));
Expand Down Expand Up @@ -212,9 +256,123 @@ 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 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=&apos;s&apos"));
}
else if (multiPartCompliance == MultiPartCompliance.LEGACY)
{
assertThat(responseContent, containsString("Unable to extract content parameters"));
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 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);

Expand All @@ -241,9 +399,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));
Expand Down Expand Up @@ -301,9 +462,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);
Expand All @@ -330,9 +494,12 @@ 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);

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);
Expand Down
Loading