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

Merged
merged 2 commits into from
May 12, 2022
Merged

Add TRANSFER_ENCODING violation for MultiPart RFC7578 parser. #7976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97e6aae
Add TRANSFER_ENCODING violation for MultiPart RFC7578 parser.
lachlan-roberts May 11, 2022
c15350d
Ignore TRANSFER_ENCODING violation for 8bit and binary.
lachlan-roberts May 11, 2022
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
33 changes: 32 additions & 1 deletion jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java
View file
Open in desktop
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"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to be more subtle about this?

There are 3 Content-Transfer-Encoding scenarios that are not a violation (as they basically mean nothing more than "identity" or "no encoding")

  • Content-Transfer-Encoding: 8bit (common enough in older httpclients)
  • Content-Transfer-Encoding: binary (also common enough, Apache HttpClient will use this for example)
  • Content-Transfer-Encoding: 7bit (this one is iffy)

So that any other value is a violation

{
if (!"8bit".equalsIgnoreCase(value) && !"binary".equalsIgnoreCase(value))
_nonComplianceWarnings.add(NonCompliance.TRANSFER_ENCODING);
}
}

@Override
Expand Down
7 changes: 7 additions & 0 deletions jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormInputStreamTest.java
View file
Open in desktop
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
26 changes: 24 additions & 2 deletions jetty-server/src/main/java/org/eclipse/jetty/server/MultiParts.java
View file
Open in desktop
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