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

MimeParser parses closing boundary as normal boundary #3968

Merged
Show file tree
Hide file tree
Changes from all 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
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2021 Oracle and/or its affiliates.
* Copyright (c) 2020, 2022 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -83,12 +83,14 @@ abstract static class ParserEvent {

/**
* Get the event type.
*
* @return EVENT_TYPE
*/
abstract EventType type();

/**
* Get this event as a {@link HeaderEvent}.
*
* @return HeaderEvent
*/
HeaderEvent asHeaderEvent() {
Expand All @@ -97,6 +99,7 @@ HeaderEvent asHeaderEvent() {

/**
* Get this event as a {@link BodyEvent}.
*
* @return HeaderEvent
*/
BodyEvent asBodyEvent() {
Expand Down Expand Up @@ -229,6 +232,7 @@ static final class ParsingException extends RuntimeException {

/**
* Create a new exception with the specified message.
*
* @param message exception message
*/
private ParsingException(String message) {
Expand All @@ -237,6 +241,7 @@ private ParsingException(String message) {

/**
* Create a new exception with the specified cause.
*
* @param cause exception cause
*/
private ParsingException(Throwable cause) {
Expand Down Expand Up @@ -344,8 +349,8 @@ private enum STATE {
* Push new data to the parsing buffer.
*
* @param data new data add to the parsing buffer
* @throws ParsingException if the parser state is not consistent
* @return buffer id
* @throws ParsingException if the parser state is not consistent
*/
int offer(ByteBuffer data) throws ParsingException {
if (closed) {
Expand All @@ -358,6 +363,9 @@ int offer(ByteBuffer data) throws ParsingException {
break;
case DATA_REQUIRED:
// resume the previous state
if (LOGGER.isLoggable(Level.FINER)) {
LOGGER.log(Level.FINER, "Resume state. resumeState={1}", resumeState);
}
state = resumeState;
resumeState = null;
id = buf.offer(data, position);
Expand Down Expand Up @@ -409,6 +417,7 @@ void cleanup() {

/**
* Advances parsing.
*
* @throws ParsingException if an error occurs during parsing
*/
Iterator<ParserEvent> parseIterator() {
Expand Down Expand Up @@ -574,9 +583,13 @@ private List<VirtualBuffer.BufferEntry> readBody() {
position = bufLen - (bl + 1);
return buf.slice(bodyBegin, position);
}
// remaining data can be an complete boundary, force it to be
// remaining data can be a complete boundary, force it to be
// processed during next iteration
return Collections.emptyList();
} else if (bndStart + bl + 1 >= bufLen) {
// found a boundary at the very end of the buffer
// it may be a "closing" boundary, require more data
return Collections.emptyList();
}

// Found boundary.
Expand Down Expand Up @@ -724,7 +737,7 @@ private String readHeaderLine() {
}
}
position = offset + hdrLen + lwsp;
if (hdrLen == 0){
if (hdrLen == 0) {
return "";
}
return new String(buf.getBytes(offset, hdrLen), HEADER_ENCODING);
Expand All @@ -733,7 +746,7 @@ private String readHeaderLine() {
/**
* Boyer-Moore search method.
* Copied from {@link java.util.regex.Pattern}
*
* <p>
* Pre calculates arrays needed to generate the bad character shift and the
* good suffix shift. Only the last seven bits are used to see if chars
* match; This keeps the tables small and covers the heavily used ASCII
Expand Down Expand Up @@ -809,6 +822,7 @@ private int match() {

/**
* Get the bytes representation of a string.
*
* @param str string to convert
* @return byte[]
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2021 Oracle and/or its affiliates.
* Copyright (c) 2020, 2022 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -59,7 +59,7 @@ public static void after() {

// TODO test mixed with nested boundaries
@Test
public void testSkipPreambule() {
public void testSkipPreamble() {
String boundary = "boundary";
final byte[] chunk1 = ("--" + boundary
+ " \t \t \t "
Expand All @@ -79,7 +79,7 @@ public void testSkipPreambule() {
}

@Test
public void testNoPreambule() {
public void testNoPreamble() {
String boundary = "boundary";
final byte[] chunk1 = ("--" + boundary
+ "Content-Id: part1\n"
Expand Down Expand Up @@ -431,6 +431,46 @@ public void testClosingBoundaryAcrossChunks() {
assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1")));
}

@Test
public void testIncompleteClosingBoundary() {
String boundary = "boundary";
final byte[] chunk1 = ("--" + boundary + "\n"
+ "Content-Id: part1\n"
+ "\n"
+ "this-is-the-body-of-part1\n"
+ "--" + boundary + "-").getBytes();
final byte[] chunk2 = "-".getBytes();

List<MimePart> parts = parse(boundary, List.of(chunk1, chunk2)).parts;
assertThat(parts.size(), is(equalTo(1)));

MimePart part1 = parts.get(0);
assertThat(part1.headers.size(), is(equalTo(1)));
assertThat(part1.headers.get("Content-Id"), hasItems("part1"));
assertThat(part1.content, is(notNullValue()));
assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1")));
}

@Test
public void testAmbiguousClosingBoundary() {
String boundary = "boundary";
final byte[] chunk1 = ("--" + boundary + "\n"
+ "Content-Id: part1\n"
+ "\n"
+ "this-is-the-body-of-part1\n"
+ "--" + boundary).getBytes();
final byte[] chunk2 = "--".getBytes();

List<MimePart> parts = parse(boundary, List.of(chunk1, chunk2)).parts;
assertThat(parts.size(), is(equalTo(1)));

MimePart part1 = parts.get(0);
assertThat(part1.headers.size(), is(equalTo(1)));
assertThat(part1.headers.get("Content-Id"), hasItems("part1"));
assertThat(part1.content, is(notNullValue()));
assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1")));
}

@Test
public void testPreamble() {
String boundary = "boundary";
Expand Down Expand Up @@ -688,12 +728,8 @@ static ParserResult parse(String boundary, List<byte[]> data) {
assertThat(name, notNullValue());
assertThat(name.length(), not(equalTo(0)));
assertThat(value, notNullValue());
List<String> values = partHeaders.get(name);
if (values == null) {
values = new ArrayList<>();
partHeaders.put(name, values);
}
values.add(value);
partHeaders.computeIfAbsent(name, k -> new ArrayList<>())
.add(value);
break;
case BODY:
for (VirtualBuffer.BufferEntry content : event.asBodyEvent().body()) {
Expand Down