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.

Sign up for GitHub

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

Jump to bottom

Add TRANSFER_ENCODING violation for MultiPart RFC7578 parser. (#7976) #7981

Merged
merged 1 commit into from May 16, 2022
Merged

Add TRANSFER_ENCODING violation for MultiPart RFC7578 parser. (#7976) #7981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 32 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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