Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixes #11310 - multipart parser dropping some relevant CR bytes in parts (#11409) 

Fixed case in MultiPart.Parser where a small chunk contains part of the boundary.

Added and fixed related tests.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Joakim Erdfelt 2024-02-21 14:43:26 -06:00 committed by GitHub
parent f8601750f3
commit 2d51170be2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 325 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java
View File

@ -405,7 +405,7 @@ public class MultiPart
@Override
public Content.Source newContentSource()
{
try (AutoLock l = lock.lock())
try (AutoLock ignored = lock.lock())
{
if (closed)
return null;
@ -431,7 +431,7 @@ public class MultiPart
public void fail(Throwable t)
{
List<Content.Source> contentSourcesToFail;
try (AutoLock l = lock.lock())
try (AutoLock ignored = lock.lock())
{
closed = true;
content.forEach(Content.Chunk::release);
@ -1329,7 +1329,7 @@ public class MultiPart
{
if (boundaryMatch == boundaryFinder.getLength())
{
// The boundary was fully matched.
// The boundary was fully matched, so the part is complete.
buffer.position(buffer.position() + boundaryMatch - partialBoundaryMatch);
partialBoundaryMatch = 0;
crContent = false;
@ -1339,7 +1339,8 @@ public class MultiPart
}
else
{
// The boundary was partially matched.
// The boundary was partially matched, but it
// is not clear yet if it is content or boundary.
buffer.position(buffer.limit());
partialBoundaryMatch = boundaryMatch;
return false;
@ -1347,8 +1348,8 @@ public class MultiPart
}
else
{
// There was a partial boundary match, but a mismatch was found.
// Handle the special case of parts with no content.
// There was a partial boundary match in the previous chunk, but now a
// mismatch was found. Handle the special case of parts with no content.
if (state == State.CONTENT_START)
{
// There is some content, reset the boundary match.
@ -1369,16 +1370,19 @@ 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.
if (boundaryOffset == 0)
crContent = false;
// Emit as content the last CR byte of the previous chunk, if any.
notifyCRContent();
// Emit as content the bytes of this chunk that are before the boundary.
int position = buffer.position();
int length = boundaryOffset;
// BoundaryFinder is configured to search for '\n--Boundary';
// if we found '\r\n--Boundary' then the '\r' is not content.
// if '\r\n--Boundary' is found, then the '\r' is not content.
if (length > 0 && buffer.get(position + length - 1) == '\r')
--length;
Content.Chunk content = asSlice(chunk, position, length, true);
@ -1390,34 +1394,25 @@ public class MultiPart
// Search for a partial boundary at the end of the buffer.
partialBoundaryMatch = boundaryFinder.endsWith(buffer);
if (partialBoundaryMatch > 0)
if (partialBoundaryMatch > 0 && partialBoundaryMatch == buffer.remaining())
{
int limit = buffer.limit();
int sliceLimit = limit - partialBoundaryMatch;
// BoundaryFinder is configured to search for '\n--Boundary';
// if we found '\r\n--Bo' then the '\r' may not be content,
// but remember it in case there is a boundary mismatch.
if (sliceLimit > 0 && buffer.get(sliceLimit - 1) == '\r')
{
crContent = true;
--sliceLimit;
}
int position = buffer.position();
Content.Chunk content = asSlice(chunk, position, sliceLimit - position, false);
buffer.position(limit);
if (content.hasRemaining())
notifyPartContent(content);
// The boundary was partially matched, but it
// is not clear yet if it is content or boundary.
buffer.position(buffer.limit());
return false;
}
// There is normal content with no boundary.
// Here, either the boundary was not found, or it was partially found at the end.
// Output as content the previous partial match, if any.
// Emit as content the last CR byte of the previous chunk, 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.
int sliceLimit = buffer.limit();
// Emit as content the bytes of this chunk, until the partial boundary match (if any).
int limit = buffer.limit();
int sliceLimit = limit - partialBoundaryMatch;
// BoundaryFinder is configured to search for '\n--Boundary';
// if '\r\n--Bo' is found, then the '\r' may not be content,
// but remember it in case there is a boundary mismatch.
if (buffer.get(sliceLimit - 1) == '\r')
{
crContent = true;
@ -1425,7 +1420,7 @@ public class MultiPart
}
int position = buffer.position();
Content.Chunk content = asSlice(chunk, position, sliceLimit - position, false);
buffer.position(buffer.limit());
buffer.position(limit);
if (content.hasRemaining())
notifyPartContent(content);
return false;
@ -1669,7 +1664,7 @@ public class MultiPart
if (value.matches(".??[a-zA-Z]:\\\\[^\\\\].*"))
{
// Matched incorrectly escaped IE filenames that have the whole
// path, as in C:\foo, we just strip any leading & trailing quotes
// path, as in C:\foo, just strip any leading & trailing quotes.
char first = value.charAt(0);
if (first == '"' || first == '\'')
value = value.substring(1);

296
jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java
View File

@ -663,6 +663,302 @@ public class MultiPartFormDataTest
}
}
@Test
public void testChunkEndsWithCRNextChunkEndsWithLF() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// First chunk must end with CR.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Second chunk must end with LF.
String chunk2 = """
DEF
""";
String terminator = """
\r
--AaB03x--\r
""";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, false, chunk2, Callback.NOOP);
Content.Sink.write(source, true, terminator, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n\rDEF\n", content);
}
}
@Test
public void testChunkEndsWithCRNextChunkHasPartialBoundaryMatch() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// First chunk must end with CR.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Second chunk must have a partial boundary that is actually content.
String chunk2 = "\n--AaB0";
String terminator = """
\r
--AaB03x--\r
""";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, false, chunk2, Callback.NOOP);
Content.Sink.write(source, true, terminator, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n\r\n--AaB0", content);
}
}
@Test
public void testChunkEndsWithCRNextChunkHasPartialBoundaryMatchNextChunkHasRemainingBoundary() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// First chunk must end with CR.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Second chunk must have a partial boundary that is actually content.
String chunk2 = "\n--AaB0";
String chunk3 = "3x--\r\n";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, false, chunk2, Callback.NOOP);
Content.Sink.write(source, true, chunk3, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n", content);
}
}
@Test
public void testChunkEndsWithCRNextChunkHasPartialBoundaryMatchNextChunksHaveRemainingBoundary() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// First chunk must end with CR.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Second chunk must have a partial boundary that is actually content.
String chunk2 = "\n--AaB0";
String chunk3 = "3x";
String chunk4 = "--\r\n";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, false, chunk2, Callback.NOOP);
Content.Sink.write(source, false, chunk3, Callback.NOOP);
Content.Sink.write(source, true, chunk4, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n", content);
}
}
@Test
public void testChunkEndsWithCRNextChunkHasPartialBoundaryMatchNextChunkHasContent() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// First chunk must end with CR.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Second chunk must have a partial boundary that is actually content.
String chunk2 = "\n--AaB0";
String chunk3 = """
-CONTENT\r
--AaB03x--\r
""";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, false, chunk2, Callback.NOOP);
Content.Sink.write(source, true, chunk3, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n\r\n--AaB0-CONTENT", content);
}
}
@Test
public void testChunkEndsWithCRNextChunkHasPartialBoundaryMatchNextChunksHaveContent() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// First chunk must end with CR.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Second chunk must have a partial boundary that is actually content.
String chunk2 = "\n--AaB";
String chunk3 = "03";
String chunk4 = """
-CONTENT\r
--AaB03x--\r
""";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, false, chunk2, Callback.NOOP);
Content.Sink.write(source, false, chunk3, Callback.NOOP);
Content.Sink.write(source, true, chunk4, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n\r\n--AaB03-CONTENT", content);
}
}
@Test
public void testOneByteChunks() throws Exception
{
String boundary = "AaB03x";
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary);
formData.setMaxMemoryFileSize(-1);
String form = """
--$B\r
Content-Disposition: form-data; name="spaces"\r
Content-Type: text/plain\r
\r
ABC\n\rDEF\n
--$B--
""".replace("$B", boundary);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
new Thread(() ->
{
ByteBuffer buf = UTF_8.encode(form);
while (buf.hasRemaining())
{
source.write(false, ByteBuffer.wrap(new byte[]{buf.get()}), Callback.NOOP);
}
source.write(true, BufferUtil.EMPTY_BUFFER, Callback.NOOP);
}).start();
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part part = parts.get(0);
String content = part.getContentAsString(UTF_8);
assertEquals("ABC\n\rDEF\n", content);
}
}
@Test
public void testSecondChunkIsTerminator() throws Exception
{
AsyncContent source = new TestContent();
MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x");
formData.setMaxMemoryFileSize(-1);
// This chunk must end with \r.
String chunk1 = """
--AaB03x\r
Content-Disposition: form-data; name="j"\r
Content-Type: text/plain\r
\r
ABC
""" + "\r";
// Terminator must start with \n--Boundary.
String terminator = """
\n--AaB03x--\r
""";
Content.Sink.write(source, false, chunk1, Callback.NOOP);
Content.Sink.write(source, true, terminator, Callback.NOOP);
CompletableFuture<MultiPartFormData.Parts> futureParts = formData.parse(source);
try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS))
{
assertEquals(1, parts.size());
MultiPart.Part spaces = parts.get(0);
String content = spaces.getContentAsString(UTF_8);
assertEquals("ABC\n", content);
}
}
@Test
public void testPartWithBackSlashInFileName() throws Exception
{