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 authored May 12, 2022
1 parent be912d4 commit 9438e50
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import javax.servlet.MultipartConfigElement;
import javax.servlet.ServletInputStream;
Expand Down Expand Up @@ -62,6 +63,7 @@ public class MultiPartFormInputStream
private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
private static final MultiMap<Part> EMPTY_MAP = new MultiMap<>(Collections.emptyMap());
private final MultiMap<Part> _parts;
private final EnumSet<NonCompliance> _nonComplianceWarnings = EnumSet.noneOf(NonCompliance.class);
private InputStream _in;
private MultipartConfigElement _config;
private String _contentType;
Expand All @@ -72,6 +74,31 @@ public class MultiPartFormInputStream
private boolean _parsed;
private int _bufferSize = 16 * 1024;

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 @@ -610,7 +637,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
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Collection;
import java.util.EnumSet;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.MultipartConfigElement;
import javax.servlet.ReadListener;
import javax.servlet.ServletInputStream;
import javax.servlet.http.Part;

import org.eclipse.jetty.http.MultiPartFormInputStream.MultiPart;
import org.eclipse.jetty.http.MultiPartFormInputStream.NonCompliance;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.BlockingArrayQueue;
import org.eclipse.jetty.util.BufferUtil;
Expand All @@ -43,6 +45,7 @@
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 @@ -887,6 +890,8 @@ public void testBase64EncodedContent() throws Exception
baos = new ByteArrayOutputStream();
IO.copy(p3.getInputStream(), baos);
assertEquals("the end", baos.toString("US-ASCII"));

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

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

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

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,29 @@ class MultiPartsHttpParser implements MultiParts
{
private final MultiPartFormInputStream _httpParser;
private final ContextHandler.Context _context;
private final Request _request;

public MultiPartsHttpParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request) throws IOException
{
_httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir);
_context = request.getContext();
_request = request;
}

@Override
public Collection<Part> getParts() throws IOException
{
return _httpParser.getParts();
Collection<Part> parts = _httpParser.getParts();
setNonComplianceViolationsOnRequest();
return parts;
}

@Override
public Part getPart(String name) throws IOException
{
return _httpParser.getPart(name);
Part part = _httpParser.getPart(name);
setNonComplianceViolationsOnRequest();
return part;
}

@Override
Expand All @@ -90,6 +96,22 @@ public Context getContext()
{
return _context;
}

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

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

@SuppressWarnings("deprecation")
Expand Down

0 comments on commit 9438e50

Please sign in to comment.