Fixes #10919 - EE10 multipart parsing may include '\r' at the front under certain conditions (#10921)

Setting MultiPart.Parser.crContent=false when the boundary is matched.
This is necessary when a read chunk ends with \r\n, and then matches the boundary on the next read chunk, and avoids to emit the CR as part content.

Fixed case of a chunk ending with \r.

Fixed case where a CR in a previous chunk is followed by a boundary at index 0 in the next chunk.
The CR should not be emitted as content, and crContent reset to false.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2023-11-29 09:17:04 +01:00 committed by GitHub
parent 0b252e59d2
commit a15a4b0a23
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 210 additions and 15 deletions

View File

@ -1326,6 +1326,7 @@ public class MultiPart
// The boundary was fully matched.
buffer.position(buffer.position() + boundaryMatch - partialBoundaryMatch);
partialBoundaryMatch = 0;
crContent = false;
notifyPartContent(Content.Chunk.EOF);
notifyPartEnd();
return true;
@ -1349,14 +1350,8 @@ public class MultiPart
return false;
}
// Must output as content the previous partial match.
if (crContent)
{
crContent = false;
Content.Chunk partContentChunk = Content.Chunk.from(CR.slice(), false);
notifyPartContent(partContentChunk);
partContentChunk.release();
}
// Output as content the previous partial match.
notifyCRContent();
ByteBuffer content = ByteBuffer.wrap(boundaryFinder.getPattern(), 0, partialBoundaryMatch);
partialBoundaryMatch = 0;
Content.Chunk partContentChunk = Content.Chunk.from(content, false);
@ -1368,8 +1363,12 @@ public class MultiPart
// Search for a full boundary.
int boundaryOffset = boundaryFinder.match(buffer);
if (boundaryOffset == 0)
crContent = false;
if (boundaryOffset >= 0)
{
// Output as content the previous partial match, if any.
notifyCRContent();
int position = buffer.position();
int length = boundaryOffset;
// BoundaryFinder is configured to search for '\n--Boundary';
@ -1406,11 +1405,9 @@ public class MultiPart
}
// There is normal content with no boundary.
if (crContent)
{
crContent = false;
notifyPartContent(Content.Chunk.from(CR.slice(), false));
}
// Output as content the previous partial match, if any.
notifyCRContent();
// If '\r' is found at the end of the buffer, it may
// not be content but the beginning of a '\r\n--Boundary';
// remember it in case it is truly normal content.
@ -1428,6 +1425,16 @@ public class MultiPart
return false;
}
private void notifyCRContent()
{
if (!crContent)
return;
crContent = false;
Content.Chunk partContentChunk = Content.Chunk.from(CR.slice(), false);
notifyPartContent(partContentChunk);
partContentChunk.release();
}
private Content.Chunk asSlice(Content.Chunk chunk, int position, int length, boolean last)
{
if (chunk.isLast() && !chunk.hasRemaining())

View File

@ -16,7 +16,9 @@ package org.eclipse.jetty.server.handler;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.SocketChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
@ -204,7 +206,7 @@ public class MultiPartFormDataHandlerTest
{
try
{
// Allow method process(...) to return.
// Allow method handle(...) to return.
Thread.sleep(1000);
// Add another part and close the multipart content.
@ -214,7 +216,7 @@ public class MultiPartFormDataHandlerTest
}
catch (InterruptedException x)
{
x.printStackTrace();
source.fail(x);
}
finally
{
@ -262,4 +264,190 @@ public class MultiPartFormDataHandlerTest
}
}
}
@Test
public void testMultiPartWithTransferEncodingChunked() throws Exception
{
start(new Handler.Abstract.NonBlocking()
{
@Override
public boolean handle(Request request, Response response, Callback callback) throws Exception
{
String boundary = MultiPart.extractBoundary(request.getHeaders().get(HttpHeader.CONTENT_TYPE));
MultiPartFormData.Parser parser = new MultiPartFormData.Parser(boundary);
parser.setMaxMemoryFileSize(-1);
MultiPartFormData.Parts parts = parser.parse(request).get(5, TimeUnit.SECONDS);
assertEquals(2, parts.size());
MultiPart.Part part0 = parts.get(0);
String part0Content = part0.getContentAsString(StandardCharsets.ISO_8859_1);
assertEquals("upload_file", part0Content);
MultiPart.Part part1 = parts.get(1);
String part1Content = part1.getContentAsString(StandardCharsets.US_ASCII);
assertEquals("abcde", part1Content);
callback.succeeded();
return true;
}
});
try (SocketChannel client = SocketChannel.open(new InetSocketAddress("localhost", connector.getLocalPort())))
{
String request = """
POST / HTTP/1.1\r
Host: localhost\r
Content-Type: multipart/form-data; boundary=908d442b-2c7d-401a-ab46-7c6ec6f89fe6\r
Transfer-Encoding: chunked\r
\r
90\r
--908d442b-2c7d-401a-ab46-7c6ec6f89fe6\r
Content-Disposition: form-data; name="az"\r
Content-Type: text/plain; charset=ISO-8859-1\r
\r
upload_file\r
\r
94\r
--908d442b-2c7d-401a-ab46-7c6ec6f89fe6\r
Content-Disposition: form-data; name="file_upload"; filename="testUpload.test"\r
Content-Type: text/plain\r
\r
\r
5\r
abcde\r
2\r
\r
\r
28\r
--908d442b-2c7d-401a-ab46-7c6ec6f89fe6--\r
0\r
\r
""";
client.write(UTF_8.encode(request));
HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(client));
assertNotNull(response);
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}
@Test
public void testMultiPartWithTransferEncodingChunkedContentEndingWithCR() throws Exception
{
start(new Handler.Abstract.NonBlocking()
{
@Override
public boolean handle(Request request, Response response, Callback callback) throws Exception
{
String boundary = MultiPart.extractBoundary(request.getHeaders().get(HttpHeader.CONTENT_TYPE));
MultiPartFormData.Parser parser = new MultiPartFormData.Parser(boundary);
parser.setMaxMemoryFileSize(-1);
MultiPartFormData.Parts parts = parser.parse(request).get(5, TimeUnit.SECONDS);
assertEquals(2, parts.size());
MultiPart.Part part0 = parts.get(0);
String part0Content = part0.getContentAsString(StandardCharsets.ISO_8859_1);
assertEquals("text_one\r", part0Content);
MultiPart.Part part1 = parts.get(1);
String part1Content = part1.getContentAsString(StandardCharsets.US_ASCII);
assertEquals("text_two", part1Content);
callback.succeeded();
return true;
}
});
try (SocketChannel client = SocketChannel.open(new InetSocketAddress("localhost", connector.getLocalPort())))
{
String request = """
POST / HTTP/1.1\r
Host: localhost\r
Content-Type: multipart/form-data; boundary=A1B2C3\r
Transfer-Encoding: chunked\r
\r
6A\r
--A1B2C3\r
Content-Disposition: form-data; name="one"\r
Content-Type: text/plain; charset=UTF-8\r
\r
text_one\r\r
61\r
\r
--A1B2C3\r
Content-Disposition: form-data; name="two"\r
Content-Type: text/plain; charset=UTF-8\r
\r
C\r
\r
text_two\r
\r
A\r
--A1B2C3--\r
0\r
\r
""";
client.write(UTF_8.encode(request));
HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(client));
assertNotNull(response);
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}
@Test
public void testMultiPartWithTransferEncodingChunkedBoundaryOnNewChunk() throws Exception
{
start(new Handler.Abstract.NonBlocking()
{
@Override
public boolean handle(Request request, Response response, Callback callback) throws Exception
{
String boundary = MultiPart.extractBoundary(request.getHeaders().get(HttpHeader.CONTENT_TYPE));
MultiPartFormData.Parser parser = new MultiPartFormData.Parser(boundary);
parser.setMaxMemoryFileSize(-1);
MultiPartFormData.Parts parts = parser.parse(request).get(5, TimeUnit.SECONDS);
assertEquals(2, parts.size());
MultiPart.Part part0 = parts.get(0);
String part0Content = part0.getContentAsString(StandardCharsets.ISO_8859_1);
assertEquals("text_one", part0Content);
MultiPart.Part part1 = parts.get(1);
String part1Content = part1.getContentAsString(StandardCharsets.US_ASCII);
assertEquals("text_two", part1Content);
callback.succeeded();
return true;
}
});
try (SocketChannel client = SocketChannel.open(new InetSocketAddress("localhost", connector.getLocalPort())))
{
String request = """
POST / HTTP/1.1\r
Host: localhost\r
Content-Type: multipart/form-data; boundary=A1B2C3\r
Transfer-Encoding: chunked\r
\r
6A\r
--A1B2C3\r
Content-Disposition: form-data; name="one"\r
Content-Type: text/plain; charset=UTF-8\r
\r
text_one\r\r
76\r
\n--A1B2C3\r
Content-Disposition: form-data; name="two"\r
Content-Type: text/plain; charset=UTF-8\r
\r
text_two\r
--A1B2C3--\r
0\r
\r
""";
client.write(UTF_8.encode(request));
HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(client));
assertNotNull(response);
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}
}