Skip to content

Commit

Permalink
Add TRANSFER_ENCODING violation for MultiPart RFC7578 parser. (#7976)
Browse files Browse the repository at this point in the history 
* Add TRANSFER_ENCODING violation for MultiPart RFC7578 parser.
* Ignore TRANSFER_ENCODING violation for 8bit and binary.

Signed-off-by: Lachlan Roberts <[email protected]>
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed May 12, 2022
1 parent b47d9c4 commit 2093f38
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 2 deletions.
33 changes: 32 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.nio.file.StandardOpenOption;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.stream.Collectors;
import javax.servlet.MultipartConfigElement;
Expand Down Expand Up @@ -91,6 +92,7 @@ private enum State

private final AutoLock _lock = new AutoLock();
private final MultiMap<Part> _parts = new MultiMap<>();
private final EnumSet<NonCompliance> _nonComplianceWarnings = EnumSet.noneOf(NonCompliance.class);
private final InputStream _in;
private final MultipartConfigElement _config;
private final File _contextTmpDir;
Expand All @@ -102,6 +104,31 @@ private enum State
private volatile int _bufferSize = 16 * 1024;
private State state = State.UNPARSED;

public enum NonCompliance
{
TRANSFER_ENCODING("https://tools.ietf.org/html/rfc7578#section-4.7");

final String _rfcRef;

NonCompliance(String rfcRef)
{
_rfcRef = rfcRef;
}

public String getURL()
{
return _rfcRef;
}
}

/**
* @return an EnumSet of non compliances with the RFC that were accepted by this parser
*/
public EnumSet<NonCompliance> getNonComplianceWarnings()
{
return _nonComplianceWarnings;
}

public class MultiPart implements Part
{
protected String _name;
Expand Down Expand Up @@ -671,7 +698,11 @@ else if (key.equalsIgnoreCase("content-type"))

// Transfer encoding is not longer considers as it is deprecated as per
// https://tools.ietf.org/html/rfc7578#section-4.7

if (key.equalsIgnoreCase("content-transfer-encoding"))
{
if (!"8bit".equalsIgnoreCase(value) && !"binary".equalsIgnoreCase(value))
_nonComplianceWarnings.add(NonCompliance.TRANSFER_ENCODING);
}
}

@Override
Expand Down
18 changes: 18 additions & 0 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.ComplianceViolation;
import org.eclipse.jetty.http.HostPortHttpField;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpCookie;
import org.eclipse.jetty.http.HttpCookie.SetCookieHttpField;
import org.eclipse.jetty.http.HttpField;
Expand Down Expand Up @@ -2295,6 +2296,7 @@ private Collection<Part> getParts(MultiMap<String> params) throws IOException

_multiParts = newMultiParts(config);
Collection<Part> parts = _multiParts.getParts();
setNonComplianceViolationsOnRequest();

String formCharset = null;
Part charsetPart = _multiParts.getPart("_charset_");
Expand Down Expand Up @@ -2355,6 +2357,22 @@ else if (getCharacterEncoding() != null)
return _multiParts.getParts();
}

private void setNonComplianceViolationsOnRequest()
{
@SuppressWarnings("unchecked")
List<String> violations = (List<String>)getAttribute(HttpCompliance.VIOLATIONS_ATTR);
if (violations != null)
return;

EnumSet<MultiPartFormInputStream.NonCompliance> nonComplianceWarnings = _multiParts.getNonComplianceWarnings();
violations = new ArrayList<>();
for (MultiPartFormInputStream.NonCompliance nc : nonComplianceWarnings)
{
violations.add(nc.name() + ": " + nc.getURL());
}
setAttribute(HttpCompliance.VIOLATIONS_ATTR, violations);
}

private MultiPartFormInputStream newMultiParts(MultipartConfigElement config) throws IOException
{
return new MultiPartFormInputStream(getInputStream(), getContentType(), config,
Expand Down
101 changes: 101 additions & 0 deletions jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Base64;
import java.util.Collection;
import java.util.EnumSet;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
Expand All @@ -32,13 +34,17 @@
import javax.servlet.http.Part;

import org.eclipse.jetty.server.MultiPartFormInputStream.MultiPart;
import org.eclipse.jetty.server.MultiPartFormInputStream.NonCompliance;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.BlockingArrayQueue;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.junit.jupiter.api.Test;

import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
Expand Down Expand Up @@ -1024,6 +1030,99 @@ public void testBase64EncodedContent() throws Exception
baos = new ByteArrayOutputStream();
IO.copy(p3.getInputStream(), baos);
assertEquals("the end", baos.toString(StandardCharsets.US_ASCII));

assertThat(mpis.getNonComplianceWarnings(), equalTo(EnumSet.of(NonCompliance.TRANSFER_ENCODING)));
}

@Test
public void testFragmentation() throws IOException
{
String contentType = "multipart/form-data, boundary=----WebKitFormBoundaryhXfFAMfUnUKhmqT8";
String payload1 =
"------WebKitFormBoundaryhXfFAMfUnUKhmqT8\r\n" +
"Content-Disposition: form-data; name=\"field1\"\r\n\r\n" +
"value1" +
"\r\n--";
String payload2 = "----WebKitFormBoundaryhXfFAMfUnUKhmqT8\r\n" +
"Content-Disposition: form-data; name=\"field2\"\r\n\r\n" +
"value2" +
"\r\n------WebKitFormBoundaryhXfFAMfUnUKhmqT8--\r\n";

// Split the content into separate reads, with the content broken up on the boundary string.
AppendableInputStream stream = new AppendableInputStream();
stream.append(payload1);
stream.append("");
stream.append(payload2);
stream.endOfContent();

MultipartConfigElement config = new MultipartConfigElement(_dirname);
MultiPartFormInputStream mpis = new MultiPartFormInputStream(stream, contentType, config, _tmpDir);
mpis.setDeleteOnExit(true);

// Check size.
Collection<Part> parts = mpis.getParts();
assertThat(parts.size(), is(2));

// Check part content.
assertThat(IO.toString(mpis.getPart("field1").getInputStream()), is("value1"));
assertThat(IO.toString(mpis.getPart("field2").getInputStream()), is("value2"));
}

static class AppendableInputStream extends InputStream
{
private static final ByteBuffer EOF = ByteBuffer.allocate(0);
private final BlockingArrayQueue<ByteBuffer> buffers = new BlockingArrayQueue<>();
private ByteBuffer current;

public void append(String data)
{
append(data.getBytes(StandardCharsets.US_ASCII));
}

public void append(byte[] data)
{
buffers.add(BufferUtil.toBuffer(data));
}

public void endOfContent()
{
buffers.add(EOF);
}

@Override
public int read() throws IOException
{
byte[] buf = new byte[1];
while (true)
{
int len = read(buf, 0, 1);
if (len < 0)
return -1;
if (len > 0)
return buf[0];
}
}

@Override
public int read(byte[] b, int off, int len) throws IOException
{
if (current == null)
current = buffers.poll();
if (current == EOF)
return -1;
if (BufferUtil.isEmpty(current))
{
current = null;
return 0;
}

ByteBuffer buffer = ByteBuffer.wrap(b, off, len);
buffer.flip();
int read = BufferUtil.append(buffer, current);
if (BufferUtil.isEmpty(current))
current = buffers.poll();
return read;
}
}

@Test
Expand Down Expand Up @@ -1062,6 +1161,8 @@ public void testQuotedPrintableEncoding() throws Exception
baos = new ByteArrayOutputStream();
IO.copy(p2.getInputStream(), baos);
assertEquals("truth=3Dbeauty", baos.toString(StandardCharsets.US_ASCII));

assertThat(mpis.getNonComplianceWarnings(), equalTo(EnumSet.of(NonCompliance.TRANSFER_ENCODING)));
}

@Test
Expand Down
5 changes: 4 additions & 1 deletion jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ public void testMultiPart() throws Exception
"--AaB03x\r\n" +
"content-disposition: form-data; name=\"stuff\"; filename=\"foo.upload\"\r\n" +
"Content-Type: text/plain;charset=ISO-8859-1\r\n" +
"Content-Transfer-Encoding: something\r\n" +
"\r\n" +
"000000000000000000000000000000000000000000000000000\r\n" +
"--AaB03x--\r\n";
Expand All @@ -514,7 +515,9 @@ public void testMultiPart() throws Exception

LocalEndPoint endPoint = _connector.connect();
endPoint.addInput(request);
assertTrue(endPoint.getResponse().startsWith("HTTP/1.1 200"));
String response = endPoint.getResponse();
assertTrue(response.startsWith("HTTP/1.1 200"));
assertThat(response, containsString("Violation: TRANSFER_ENCODING"));

// We know the previous request has completed if another request can be processed on the same connection.
String cleanupRequest = "GET /foo/cleanup HTTP/1.1\r\n" +
Expand Down

0 comments on commit 2093f38

Please sign in to comment.