Skip to content

Commit

Permalink
Issue #11387: Reintroduce MultiPartCompliance.LEGACY in ee9/ee8 (#11388)
Browse files Browse the repository at this point in the history 
* Issue #11387: Reintroduce MultiPartCompliance.LEGACY in ee9/ee8
* Correcting javadoc
* Updating MultiPartCaptureTest to ...
  * Test with MultiPartFormData.Parser and MultiPart.Parser
 * Enable all test cases
    * base64 behaviors modified to not auto-decode base64 content
    * forms submitted without `_charset_` part (some using a different
       charset than UTF-8, like `Shift_JIS`)
* Fixing checkstyle warning
* Re-enable Part-ContainsContents expectations
* Rename MultiPartCompliance.NO_CRLF_AFTER_PREAMBLE to WHITESPACE_BEFORE_BOUNDARY to fit spec better
* Make ee9/ee8 legacy parser use legacy tokenization
* Testing ee9/ee8 legacy parser base64 auto-decoding behaviors
* Cleanup jetty-test-multipart class naming
* Adding ee10 tests against raw multipart examples
* Adding shorter whitespace multipart test
* Adding jetty-core version of failing ee10 tests
* Fixed missed notification for CR content in case of 1 chunk ending with CR and the next chunk ending with LF.
* Removed internal unused class MultiPartParser.
* Adding MultiPartCompliance.Violation events
  + in MultiPart.Parser
  + in MultiPartFormData.Parser
* lenient mode behavior
* new name fits violation better
+ adding violation to MultiPart.Parser.parseHeaderStart
* some simple cleanup of new ee9 code

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@joakime @sbordet
joakime and sbordet authored Feb 27, 2024
1 parent 2803f5a commit 98ceb73
Show file tree
Hide file tree
Showing 279 changed files with 3,285 additions and 214,447 deletions.
7 changes: 6 additions & 1 deletion jetty-core/jetty-http/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,17 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>

<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-slf4j-impl</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.tests</groupId>
<artifactId>jetty-test-multipart</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.toolchain</groupId>
<artifactId>jetty-test-helper</artifactId>
Expand Down
1 change: 1 addition & 0 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public enum HttpHeader
CONTENT_LOCATION("Content-Location"),
CONTENT_MD5("Content-MD5"),
CONTENT_RANGE("Content-Range"),
CONTENT_TRANSFER_ENCODING("Content-Transfer-Encoding"),
CONTENT_TYPE("Content-Type"),
EXPIRES("Expires"),
LAST_MODIFIED("Last-Modified"),
Expand Down
113 changes: 111 additions & 2 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.nio.file.StandardCopyOption;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -908,6 +909,7 @@ public static class Parser
private final Utf8StringBuilder text = new Utf8StringBuilder();
private final String boundary;
private final SearchPattern boundaryFinder;
private final MultiPartCompliance compliance;
private final Listener listener;
private int partHeadersLength;
private int partHeadersMaxLength = -1;
Expand All @@ -920,13 +922,33 @@ public static class Parser
private String fieldValue;
private long maxParts = 1000;
private int numParts;
private EnumSet<MultiPartCompliance.Violation> eols;

public Parser(String boundary, Listener listener)
{
this(boundary, MultiPartCompliance.RFC7578, listener);
}

public Parser(String boundary, MultiPartCompliance compliance, Listener listener)
{
this.boundary = boundary;
// While the spec mandates CRLF before the boundary, be more lenient and only require LF.
this.boundaryFinder = SearchPattern.compile("\n--" + boundary);
this.compliance = compliance;
this.listener = listener;

if (LOG.isDebugEnabled())
{
List.of(
MultiPartCompliance.Violation.CR_LINE_TERMINATION,
MultiPartCompliance.Violation.BASE64_TRANSFER_ENCODING,
MultiPartCompliance.Violation.WHITESPACE_BEFORE_BOUNDARY
).forEach(violation ->
{
if (compliance.allows(violation))
LOG.debug("{} ignoring violation {}: unable to allow it", getClass().getName(), violation.name());
});
}
reset();
}

Expand Down Expand Up @@ -1050,6 +1072,7 @@ else if (type != HttpTokens.Type.SPACE && type != HttpTokens.Type.HTAB)
HttpTokens.Token token = next(buffer);
if (token.getByte() != '-')
throw new BadMessageException("bad last boundary");
notifyEndOfLineViolations();
state = State.EPILOGUE;
}
case HEADER_START ->
Expand Down Expand Up @@ -1099,6 +1122,7 @@ else if (type != HttpTokens.Type.SPACE && type != HttpTokens.Type.HTAB)
if (LOG.isDebugEnabled())
LOG.debug("parse failure {} {}", state, BufferUtil.toDetailString(buffer), x);
buffer.position(buffer.limit());
notifyEndOfLineViolations();
notifyFailure(x);
}
}
Expand All @@ -1123,18 +1147,35 @@ private HttpTokens.Token next(ByteBuffer buffer)
}
case LF ->
{
if (!crFlag)
{
MultiPartCompliance.Violation violation = MultiPartCompliance.Violation.LF_LINE_TERMINATION;
addEndOfLineViolation(violation);
if (!compliance.allows(violation))
throw new BadMessageException("invalid LF-only EOL");
}
crFlag = false;
}
case CR ->
{
if (crFlag)
throw new BadMessageException("invalid EOL");
{
MultiPartCompliance.Violation violation = MultiPartCompliance.Violation.CR_LINE_TERMINATION;
addEndOfLineViolation(violation);
if (!compliance.allows(violation))
throw new BadMessageException("invalid CR-only EOL");
}
crFlag = true;
}
default ->
{
if (crFlag)
throw new BadMessageException("invalid EOL");
{
MultiPartCompliance.Violation violation = MultiPartCompliance.Violation.CR_LINE_TERMINATION;
addEndOfLineViolation(violation);
if (!compliance.allows(violation))
throw new BadMessageException("invalid CR-only EOL");
}
}
}
return t;
Expand Down Expand Up @@ -1331,6 +1372,13 @@ private boolean parseContent(Content.Chunk chunk)
{
// The boundary was fully matched, so the part is complete.
buffer.position(buffer.position() + boundaryMatch - partialBoundaryMatch);
if (!crContent)
{
MultiPartCompliance.Violation violation = MultiPartCompliance.Violation.LF_LINE_TERMINATION;
addEndOfLineViolation(violation);
if (!compliance.allows(violation))
throw new BadMessageException("invalid LF-only EOL");
}
partialBoundaryMatch = 0;
crContent = false;
notifyPartContent(Content.Chunk.EOF);
Expand Down Expand Up @@ -1373,7 +1421,16 @@ private boolean parseContent(Content.Chunk chunk)
if (boundaryOffset >= 0)
{
if (boundaryOffset == 0)
{
if (!crContent)
{
MultiPartCompliance.Violation violation = MultiPartCompliance.Violation.LF_LINE_TERMINATION;
addEndOfLineViolation(violation);
if (!compliance.allows(violation))
throw new BadMessageException("invalid LF-only EOL");
}
crContent = false;
}

// Emit as content the last CR byte of the previous chunk, if any.
notifyCRContent();
Expand All @@ -1384,7 +1441,16 @@ private boolean parseContent(Content.Chunk chunk)
// BoundaryFinder is configured to search for '\n--Boundary';
// if '\r\n--Boundary' is found, then the '\r' is not content.
if (length > 0 && buffer.get(position + length - 1) == '\r')
{
--length;
}
else
{
MultiPartCompliance.Violation violation = MultiPartCompliance.Violation.LF_LINE_TERMINATION;
addEndOfLineViolation(violation);
if (!compliance.allows(violation))
throw new BadMessageException("invalid LF-only EOL");
}
Content.Chunk content = asSlice(chunk, position, length, true);
buffer.position(position + boundaryOffset + boundaryFinder.getLength());
notifyPartContent(content);
Expand Down Expand Up @@ -1536,6 +1602,39 @@ private void notifyFailure(Throwable failure)
}
}

private void notifyEndOfLineViolations()
{
if (eols != null)
{
for (MultiPartCompliance.Violation violation: eols)
{
notifyViolation(violation);
}
eols = null;
}
}

private void addEndOfLineViolation(MultiPartCompliance.Violation violation)
{
if (eols == null)
eols = EnumSet.of(violation);
else
eols.add(violation);
}

private void notifyViolation(MultiPartCompliance.Violation violation)
{
try
{
listener.onViolation(violation);
}
catch (Throwable x)
{
if (LOG.isDebugEnabled())
LOG.debug("failure while notifying listener {}", listener, x);
}
}

/**
* <p>A listener for events emitted by a {@link Parser}.</p>
*/
Expand Down Expand Up @@ -1598,6 +1697,16 @@ default void onComplete()
default void onFailure(Throwable failure)
{
}

/**
* <p>Callback method invoked when the low level parser encounters
* a fundamental multipart violation</p>>
*
* @param violation the violation detected
*/
default void onViolation(MultiPartCompliance.Violation violation)
{
}
}

private enum State
Expand Down
Loading

0 comments on commit 98ceb73

Please sign in to comment.