Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merged branch 'jetty-10.0.x' into 'jetty-11.0.x'. 

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2021-04-10 18:54:34 +02:00
parent 269d9b718c e163b001c3
commit 465154ae04
7 changed files with 169 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java
View File

@ -25,7 +25,9 @@ import org.slf4j.LoggerFactory;
import static java.util.Arrays.asList;
import static java.util.Collections.unmodifiableSet;
import static java.util.EnumSet.allOf;
import static java.util.EnumSet.complementOf;
import static java.util.EnumSet.noneOf;
import static java.util.EnumSet.of;
/**
* URI compliance modes for Jetty request handling.
@ -37,23 +39,29 @@ public final class UriCompliance implements ComplianceViolation.Mode
protected static final Logger LOG = LoggerFactory.getLogger(UriCompliance.class);
/**
* These are URI compliance violations, which may be allowed by the compliance mode. Currently all these
* violations are for additional criteria in excess of the strict requirements of rfc3986.
* These are URI compliance "violations", which may be allowed by the compliance mode. These are actual
* violations of the RFC, as they represent additional requirements in excess of the strict compliance of rfc3986.
* A compliance mode that contains one or more of these Violations, allows request to violate the corresponding
* additional requirement.
*/
public enum Violation implements ComplianceViolation
{
/**
* Ambiguous path segments e.g. <code>/foo/%2e%2e/bar</code>
* Allow ambiguous path segments e.g. <code>/foo/%2e%2e/bar</code>
*/
AMBIGUOUS_PATH_SEGMENT("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path segment"),
/**
* Ambiguous path separator within a URI segment e.g. <code>/foo/b%2fr</code>
* Allow ambiguous path separator within a URI segment e.g. <code>/foo/b%2fr</code>
*/
AMBIGUOUS_PATH_SEPARATOR("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path separator"),
/**
* Ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
* Allow ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
*/
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter");
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter"),
/**
* Allow Non canonical ambiguous paths. eg <code>/foo/x%2f%2e%2e%/bar</code> provided to applications as <code>/foo/x/../bar</code>
*/
NON_CANONICAL_AMBIGUOUS_PATHS("https://tools.ietf.org/html/rfc3986#section-3.3", "Non canonical ambiguous paths");
private final String _url;
private final String _description;
@ -84,19 +92,28 @@ public final class UriCompliance implements ComplianceViolation.Mode
}
/**
* The default compliance mode that extends RFC3986 compliance with additional violations to avoid ambiguous URIs
* The default compliance mode that extends RFC3986 compliance with additional violations to avoid most ambiguous URIs.
* This mode does allow {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, but disallows
* {@link Violation#AMBIGUOUS_PATH_PARAMETER} and {@link Violation#AMBIGUOUS_PATH_SEGMENT}.
* Ambiguous paths are not allowed by {@link Violation#NON_CANONICAL_AMBIGUOUS_PATHS}.
*/
public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", noneOf(Violation.class));
public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", of(Violation.AMBIGUOUS_PATH_SEPARATOR));
/**
* LEGACY compliance mode that disallows only ambiguous path parameters as per Jetty-9.4
* LEGACY compliance mode that models Jetty-9.4 behavior by allowing {@link Violation#AMBIGUOUS_PATH_SEGMENT} and {@link Violation#AMBIGUOUS_PATH_SEPARATOR}
*/
public static final UriCompliance LEGACY = new UriCompliance("LEGACY", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR));
public static final UriCompliance LEGACY = new UriCompliance("LEGACY", of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR));
/**
* Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations
* Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations,
* except {@link Violation#NON_CANONICAL_AMBIGUOUS_PATHS}, thus ambiguous paths are canonicalized for safety.
*/
public static final UriCompliance RFC3986 = new UriCompliance("RFC3986", allOf(Violation.class));
public static final UriCompliance RFC3986 = new UriCompliance("RFC3986", complementOf(of(Violation.NON_CANONICAL_AMBIGUOUS_PATHS)));
/**
* Compliance mode that allows all URI Violations, including allowing ambiguous paths in non canonicalized form.
*/
public static final UriCompliance UNSAFE = new UriCompliance("UNSAFE", allOf(Violation.class));
/**
* @deprecated equivalent to DEFAULT
@ -125,6 +142,17 @@ public final class UriCompliance implements ComplianceViolation.Mode
return null;
}
/**
* Create compliance set from a set of allowed Violations.
*
* @param violations A string of violations to allow:
* @return the compliance from the string spec
*/
public static UriCompliance from(Set<Violation> violations)
{
return new UriCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
}
/**
* Create compliance set from string.
* <p>
@ -151,22 +179,23 @@ public final class UriCompliance implements ComplianceViolation.Mode
*/
public static UriCompliance from(String spec)
{
Set<Violation> sections;
Set<Violation> violations;
String[] elements = spec.split("\\s*,\\s*");
switch (elements[0])
{
case "0":
sections = noneOf(Violation.class);
violations = noneOf(Violation.class);
break;
case "*":
sections = allOf(Violation.class);
violations = allOf(Violation.class);
break;
default:
{
UriCompliance mode = UriCompliance.valueOf(elements[0]);
sections = (mode == null) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
violations = (mode == null) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
break;
}
}
@ -178,12 +207,12 @@ public final class UriCompliance implements ComplianceViolation.Mode
element = element.substring(1);
Violation section = Violation.valueOf(element);
if (exclude)
sections.remove(section);
violations.remove(section);
else
sections.add(section);
violations.add(section);
}
UriCompliance compliance = new UriCompliance("CUSTOM" + __custom.getAndIncrement(), sections);
UriCompliance compliance = new UriCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
if (LOG.isDebugEnabled())
LOG.debug("UriCompliance from {}->{}", spec, compliance);
return compliance;
@ -192,7 +221,7 @@ public final class UriCompliance implements ComplianceViolation.Mode
private final String _name;
private final Set<Violation> _allowed;
private UriCompliance(String name, Set<Violation> violations)
public UriCompliance(String name, Set<Violation> violations)
{
Objects.requireNonNull(violations);
_name = name;

15
jetty-http/src/test/java/org/eclipse/jetty/http/SyntaxTest.java
View File

@ -18,6 +18,7 @@ import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;
public class SyntaxTest
@ -61,17 +62,11 @@ public class SyntaxTest
for (String token : tokens)
{
try
{
Syntax.requireValidRFC2616Token(token, "Test Based");
fail("RFC2616 Token [" + token + "] Should have thrown " + IllegalArgumentException.class.getName());
}
catch (IllegalArgumentException e)
{
assertThat("Testing Bad RFC2616 Token [" + token + "]", e.getMessage(),
Throwable e = assertThrows(IllegalArgumentException.class,
() -> Syntax.requireValidRFC2616Token(token, "Test Based"));
assertThat("Testing Bad RFC2616 Token [" + token + "]", e.getMessage(),
allOf(containsString("Test Based"),
containsString("RFC2616")));
}
containsString("RFC2616")));
}
}

9
jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
View File

@ -263,9 +263,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
{
// Fill the request buffer (if needed).
int filled = fillRequestBuffer();
if (filled > 0)
bytesIn.add(filled);
else if (filled == -1 && getEndPoint().isOutputShutdown())
if (filled < 0 && getEndPoint().isOutputShutdown())
close();
// Parse the request buffer.
@ -352,8 +350,9 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
if (filled == 0) // Do a retry on fill 0 (optimization for SSL connections)
filled = getEndPoint().fill(_requestBuffer);
// tell parser
if (filled < 0)
if (filled > 0)
bytesIn.add(filled);
else if (filled < 0)
_parser.atEOF();
if (LOG.isDebugEnabled())

10
jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View File

@ -67,7 +67,6 @@ import jakarta.servlet.http.WebConnection;
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;
@ -1692,10 +1691,11 @@ public class Request implements HttpServletRequest
_httpFields = request.getFields();
final HttpURI uri = request.getURI();
UriCompliance compliance = null;
boolean ambiguous = uri.isAmbiguous();
if (ambiguous)
{
UriCompliance compliance = _channel == null || _channel.getHttpConfiguration() == null ? null : _channel.getHttpConfiguration().getUriCompliance();
compliance = _channel == null || _channel.getHttpConfiguration() == null ? null : _channel.getHttpConfiguration().getUriCompliance();
if (uri.hasAmbiguousSegment() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEGMENT)))
throw new BadMessageException("Ambiguous segment in URI");
if (uri.hasAmbiguousSeparator() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR)))
@ -1746,9 +1746,9 @@ public class Request implements HttpServletRequest
path = (encoded.length() == 1) ? "/" : _uri.getDecodedPath();
// Strictly speaking if a URI is legal and encodes ambiguous segments, then they should be
// reflected in the decoded string version. However, it can be ambiguous to provide a decoded path as
// a string, so we normalize again. If an application wishes to see ambiguous URIs, then they can look
// at the encoded form of the URI
if (ambiguous)
// a string, so we normalize again. If an application wishes to see ambiguous URIs, then they must
// set the {@link UriCompliance.Violation#NON_CANONICAL_AMBIGUOUS_PATHS} compliance.
if (ambiguous && (compliance == null || !compliance.allows(UriCompliance.Violation.NON_CANONICAL_AMBIGUOUS_PATHS)))
path = URIUtil.canonicalPath(path);
}
else if ("*".equals(encoded) || HttpMethod.CONNECT.is(getMethod()))

68
jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
View File

@ -32,6 +32,8 @@ import java.util.List;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import jakarta.servlet.ServletException;
@ -40,6 +42,7 @@ import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpParser;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.logging.StacklessLogging;
@ -47,6 +50,7 @@ import org.eclipse.jetty.server.LocalConnector.LocalEndPoint;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@ -59,9 +63,11 @@ import org.slf4j.LoggerFactory;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpConnectionTest
@ -1353,6 +1359,68 @@ public class HttpConnectionTest
}
}
@Test
public void testBytesIn() throws Exception
{
String chunk1 = "0123456789ABCDEF";
String chunk2 = IntStream.range(0, 64).mapToObj(i -> chunk1).collect(Collectors.joining());
long dataLength = chunk1.length() + chunk2.length();
server.stop();
server.setHandler(new AbstractHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
jettyRequest.setHandled(true);
IO.copy(request.getInputStream(), IO.getNullStream());
HttpConnection connection = HttpConnection.getCurrentConnection();
long bytesIn = connection.getBytesIn();
assertThat(bytesIn, greaterThan(dataLength));
}
});
server.start();
LocalEndPoint localEndPoint = connector.executeRequest("" +
"POST / HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Content-Length: " + dataLength + "\r\n" +
"\r\n" +
chunk1);
// Wait for the server to block on the read().
Thread.sleep(500);
// Send more content.
localEndPoint.addInput(chunk2);
HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse());
assertEquals(response.getStatus(), HttpStatus.OK_200);
localEndPoint.close();
localEndPoint = connector.executeRequest("" +
"POST / HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Transfer-Encoding: chunked\r\n" +
"\r\n" +
Integer.toHexString(chunk1.length()) + "\r\n" +
chunk1 + "\r\n");
// Wait for the server to block on the read().
Thread.sleep(500);
// Send more content.
localEndPoint.addInput("" +
Integer.toHexString(chunk2.length()) + "\r\n" +
chunk2 + "\r\n" +
"0\r\n" +
"\r\n");
response = HttpTester.parseResponse(localEndPoint.getResponse());
assertEquals(response.getStatus(), HttpStatus.OK_200);
localEndPoint.close();
}
private int checkContains(String s, int offset, String c)
{
assertThat(s.substring(offset), Matchers.containsString(c));

34
jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
View File

@ -28,6 +28,7 @@ import java.security.Principal;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.Enumeration;
import java.util.List;
import java.util.Locale;
@ -1733,12 +1734,45 @@ public class RequestTest
"Host: whatever\r\n" +
"\r\n";
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(new UriCompliance("Test", EnumSet.noneOf(UriCompliance.Violation.class)));
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400"));
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
}
@Test
public void testAmbiguousPaths() throws Exception
{
_handler._checker = (request, response) ->
{
response.getOutputStream().println("servletPath=" + request.getServletPath());
response.getOutputStream().println("pathInfo=" + request.getPathInfo());
return true;
};
String request = "GET /unnormal/.././path/ambiguous%2f%2e%2e/%2e;/info HTTP/1.0\r\n" +
"Host: whatever\r\n" +
"\r\n";
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.from(EnumSet.of(
UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR,
UriCompliance.Violation.AMBIGUOUS_PATH_SEGMENT,
UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER)));
assertThat(_connector.getResponse(request), Matchers.allOf(
startsWith("HTTP/1.1 200"),
containsString("pathInfo=/path/info")));
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.from(EnumSet.of(
UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR,
UriCompliance.Violation.AMBIGUOUS_PATH_SEGMENT,
UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER,
UriCompliance.Violation.NON_CANONICAL_AMBIGUOUS_PATHS)));
assertThat(_connector.getResponse(request), Matchers.allOf(
startsWith("HTTP/1.1 200"),
containsString("pathInfo=/path/ambiguous/.././info")));
}
@Test
public void testPushBuilder()

6
tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java
View File

@ -251,7 +251,8 @@ public class GzipWithSendErrorTest
assertThat("Request Input Content Received", inputContentReceived.get(), is(0L));
assertThat("Request Input Content Received less then initial buffer", inputContentReceived.get(), lessThanOrEqualTo((long)sizeActuallySent));
assertThat("Request Connection BytesIn should have some minimal data", inputBytesIn.get(), greaterThanOrEqualTo(1024L));
assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo((long)sizeActuallySent));
long requestBytesSent = sizeActuallySent + 512; // Take into account headers and chunked metadata.
assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo(requestBytesSent));
// Now provide rest
content.offer(ByteBuffer.wrap(compressedRequest, sizeActuallySent, compressedRequest.length - sizeActuallySent));
@ -351,7 +352,8 @@ public class GzipWithSendErrorTest
assertThat("Request Input Content Received", inputContentReceived.get(), read ? greaterThan(0L) : is(0L));
assertThat("Request Input Content Received less then initial buffer", inputContentReceived.get(), lessThanOrEqualTo((long)sizeActuallySent));
assertThat("Request Connection BytesIn should have some minimal data", inputBytesIn.get(), greaterThanOrEqualTo(1024L));
assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo((long)sizeActuallySent));
long requestBytesSent = sizeActuallySent + 512; // Take into account headers and chunked metadata.
assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo(requestBytesSent));
// Now provide rest
content.offer(ByteBuffer.wrap(compressedRequest, sizeActuallySent, compressedRequest.length - sizeActuallySent));