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

Decode safe Path for #9444 (#9455) 

Added URIUtil.decodeSafePath for EE10, to allow for %2F and %25 to remain encoded in the servlet API.
Fixed async dispatch to also safeDecode
Updated tests to expect decoded space
Apply suggestions from code review

Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Greg Wilkins 2023-03-04 00:36:19 +11:00 committed by GitHub
parent 474136de57
commit 916cd9894d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 143 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View File

@ -34,15 +34,15 @@ import org.eclipse.jetty.util.UrlEncoded;
* via the static methods such as {@link #build()} and {@link #from(String)}.
*
* A URI such as
* {@code http://user@host:port/path;param1/%2e/f%6fo%2fbar;param2?query#fragment}
* {@code http://user@host:port/path;param1/%2e/f%6fo%2fbar%20bob;param2?query#fragment}
* is split into the following optional elements:<ul>
* <li>{@link #getScheme()} - http:</li>
* <li>{@link #getAuthority()} - //name@host:port</li>
* <li>{@link #getHost()} - host</li>
* <li>{@link #getPort()} - port</li>
* <li>{@link #getPath()} - /path;param1/%2e/f%6fo%2fbar;param2</li>
* <li>{@link #getCanonicalPath()} - /path/foo%2Fbar</li>
* <li>{@link #getDecodedPath()} - /path/foo/bar</li>
* <li>{@link #getPath()} - /path;param1/%2e/f%6fo%2fbar%20bob;param2</li>
* <li>{@link #getCanonicalPath()} - /path/foo%2Fbar%20bob</li>
* <li>{@link #getDecodedPath()} - /path/foo/bar bob</li>
* <li>{@link #getParam()} - param2</li>
* <li>{@link #getQuery()} - query</li>
* <li>{@link #getFragment()} - fragment</li>
@ -712,7 +712,7 @@ public interface HttpURI
{
_uri = null;
_path = URIUtil.encodePath(path);
_canonicalPath = path;
_canonicalPath = URIUtil.canonicalPath(_path);
return this;
}

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

@ -35,7 +35,7 @@ import static java.util.EnumSet.of;
*/
public final class UriCompliance implements ComplianceViolation.Mode
{
protected static final Logger LOG = LoggerFactory.getLogger(UriCompliance.class);
private static final Logger LOG = LoggerFactory.getLogger(UriCompliance.class);
/**
* These are URI compliance "violations", which may be allowed by the compliance mode. These are actual
@ -137,22 +137,8 @@ public final class UriCompliance implements ComplianceViolation.Mode
*/
public static final UriCompliance UNSAFE = new UriCompliance("UNSAFE", allOf(Violation.class));
/**
* @deprecated equivalent to DEFAULT
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated
public static final UriCompliance SAFE = new UriCompliance("SAFE", DEFAULT.getAllowed());
/**
* @deprecated equivalent to RFC3986
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated
public static final UriCompliance STRICT = new UriCompliance("STRICT", RFC3986.getAllowed());
private static final AtomicInteger __custom = new AtomicInteger();
private static final List<UriCompliance> KNOWN_MODES = List.of(DEFAULT, LEGACY, RFC3986, RFC3986_UNAMBIGUOUS, UNSAFE, SAFE, STRICT);
private static final List<UriCompliance> KNOWN_MODES = List.of(DEFAULT, LEGACY, RFC3986, RFC3986_UNAMBIGUOUS, UNSAFE);
public static UriCompliance valueOf(String name)
{

2
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java
View File

@ -14,7 +14,6 @@
package org.eclipse.jetty.http.pathmap;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -195,7 +194,6 @@ public class ServletPathSpec extends AbstractPathSpec
servletPathSpec = "";
if (servletPathSpec.startsWith("servlet|"))
servletPathSpec = servletPathSpec.substring("servlet|".length());
servletPathSpec = URIUtil.canonicalPath(servletPathSpec);
assertValidServletPathSpec(servletPathSpec);
// The Root Path Spec

2
jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
View File

@ -79,7 +79,7 @@ public class HttpURITest
assertThat(uri.getHost(), is("[::1]"));
assertThat(uri.getPort(), is(8080));
assertThat(uri.getPath(), is("/some%20encoded/evening;id=12345"));
assertThat(uri.getCanonicalPath(), is("/some encoded/evening"));
assertThat(uri.getCanonicalPath(), is("/some%20encoded/evening"));
assertThat(uri.getParam(), is("id=12345"));
assertThat(uri.getQuery(), nullValue());
assertThat(uri.getAuthority(), is("[::1]:8080"));

61
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
View File

@ -28,6 +28,7 @@ import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.StringTokenizer;
import java.util.function.BiConsumer;
import java.util.stream.Stream;
import org.eclipse.jetty.util.Utf8Appendable.NotUtf8Exception;
@ -459,7 +460,8 @@ public final class URIUtil
}
/**
* Decode a URI path and strip parameters
* Decodes a percent-encoded URI path (assuming UTF-8 characters) and strips path parameters.
* @param path The URI path to decode
* @see #canonicalPath(String)
* @see #normalizePath(String)
*/
@ -469,11 +471,59 @@ public final class URIUtil
}
/**
* Decode a URI path and strip parameters of UTF-8 path
* Decodes a percent-encoded URI path (assuming UTF-8 characters) and strips path parameters.
* @param path A String holding the URI path to decode
* @param offset The start of the URI within the path string
* @param length The length of the URI within the path string
* @see #canonicalPath(String)
* @see #normalizePath(String)
*/
public static String decodePath(String path, int offset, int length)
{
return decodePath(path, offset, length, Utf8StringBuilder::append);
}
/**
* Decodes a percent-encoded URI path (assuming UTF-8 characters) and strips path parameters, but leaves reserved path characters percent-encoded.
* @param path A String holding the URI path to decode
* @see #canonicalPath(String)
* @see #normalizePath(String)
*/
public static String safeDecodePath(String path)
{
return safeDecodePath(path, 0, path.length());
}
/**
* Decodes a percent-encoded URI path (assuming UTF-8 characters) and strips path parameters, but leaves reserved path characters percent-encoded.
* @param path A String holding the URI path to decode
* @param offset The start of the URI within the path string
* @param length The length of the URI within the path string
* @see #canonicalPath(String)
* @see #normalizePath(String)
*/
public static String safeDecodePath(String path, int offset, int length)
{
return decodePath(path, offset, length, URIUtil::safePathAppend);
}
private static void safePathAppend(Utf8StringBuilder builder, byte b)
{
switch (b)
{
case '/' -> builder.append("%2F");
case '%' -> builder.append("%25");
case '?' -> builder.append("%3F");
default -> builder.append(b);
}
}
/**
* Decode a URI path and strip parameters of UTF-8 path
* @see #canonicalPath(String)
* @see #normalizePath(String)
*/
public static String decodePath(String path, int offset, int length, BiConsumer<Utf8StringBuilder, Byte> decoder)
{
try
{
@ -487,7 +537,7 @@ public final class URIUtil
case '%':
if (builder == null)
{
builder = new Utf8StringBuilder(path.length());
builder = new Utf8StringBuilder(length);
builder.append(path, offset, i - offset);
}
if ((i + 2) < end)
@ -500,12 +550,13 @@ public final class URIUtil
String str = new String(codePoints, 0, 1);
byte[] bytes = str.getBytes(StandardCharsets.UTF_8);
for (byte b: bytes)
builder.append(b);
decoder.accept(builder, b);
i += 5;
}
else
{
builder.append((byte)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2)))));
byte b = (byte)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2))));
decoder.accept(builder, b);
i += 2;
}
}

2
jetty-ee10/jetty-ee10-proxy/src/test/java/org/eclipse/jetty/ee10/proxy/BalancerServletTest.java
View File

@ -180,7 +180,7 @@ public class BalancerServletTest
assertThat(response.getStatus(), is(200));
assertThat(response.getContentAsString(), containsString("requestURI='/context/mapping/test/%0A'"));
assertThat(response.getContentAsString(), containsString("servletPath='/mapping'"));
assertThat(response.getContentAsString(), containsString("pathInfo='/test/%0A'"));
assertThat(response.getContentAsString(), containsString("pathInfo='/test/\n'"));
}
private String readFirstLine(byte[] responseBytes) throws IOException

5
jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java
View File

@ -433,7 +433,7 @@ public class ServletChannel
dispatch(() ->
{
HttpURI uri;
String pathInContext = Request.getPathInContext(_servletContextRequest);
String pathInContext;
AsyncContextEvent asyncContextEvent = _state.getAsyncContextEvent();
String dispatchString = asyncContextEvent.getDispatchPath();
if (dispatchString != null)
@ -451,6 +451,7 @@ public class ServletChannel
if (uri == null)
{
uri = _servletContextRequest.getHttpURI();
pathInContext = Request.getPathInContext(_servletContextRequest);
}
else
{
@ -459,6 +460,8 @@ public class ServletChannel
pathInContext = pathInContext.substring(_context.getContextPath().length());
}
}
// We first worked with the core pathInContext above, but now need to convert to servlet style
pathInContext = URIUtil.safeDecodePath(pathInContext);
Dispatcher dispatcher = new Dispatcher(getContextHandler(), uri, pathInContext);
dispatcher.async(asyncContextEvent.getSuppliedRequest(), asyncContextEvent.getSuppliedResponse());

3
jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java
View File

@ -1161,7 +1161,8 @@ public class ServletContextHandler extends ContextHandler implements Graceful
{
// Need to ask directly to the Context for the pathInContext, rather than using
// Request.getPathInContext(), as the request is not yet wrapped in this Context.
String pathInContext = getContext().getPathInContext(request.getHttpURI().getCanonicalPath());
String pathInContext = URIUtil.safeDecodePath(getContext().getPathInContext(request.getHttpURI().getCanonicalPath()));
MatchedResource<ServletHandler.MappedServlet> matchedResource = _servletHandler.getMatchedServlet(pathInContext);
if (matchedResource == null)
return null;

8
jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContextTest.java
View File

@ -245,7 +245,7 @@ public class AsyncContextTest
// async run attributes
assertThat("async run attr servlet path is original", responseBody, containsString("async:run:attr:servletPath:/test"));
assertThat("async run attr path info has correct encoding", responseBody, containsString("async:run:attr:pathInfo:/hello%20there"));
assertThat("async run attr path info has correct encoding", responseBody, containsString("async:run:attr:pathInfo:/hello there"));
assertThat("async run attr query string", responseBody, containsString("async:run:attr:queryString:dispatch=true"));
assertThat("async run context path", responseBody, containsString("async:run:attr:contextPath:/ctx"));
assertThat("async run request uri has correct encoding", responseBody, containsString("async:run:attr:requestURI:/ctx/test/hello%20there"));
@ -287,13 +287,13 @@ public class AsyncContextTest
assertThat("servlet gets right path", responseBody, containsString("doGet:getServletPath:/servletPath2"));
assertThat("async context gets right path in get", responseBody, containsString("doGet:async:getServletPath:/servletPath2"));
assertThat("servlet path attr is original", responseBody, containsString("async:run:attr:servletPath:/path%20with%20spaces/servletPath"));
assertThat("servlet path attr is original", responseBody, containsString("async:run:attr:servletPath:/path with spaces/servletPath"));
assertThat("path info attr is correct", responseBody, containsString("async:run:attr:pathInfo:null"));
assertThat("query string attr is correct", responseBody, containsString("async:run:attr:queryString:dispatch=true&queryStringWithEncoding=space%20space"));
assertThat("context path attr is correct", responseBody, containsString("async:run:attr:contextPath:/ctx"));
assertThat("request uri attr is correct", responseBody, containsString("async:run:attr:requestURI:/ctx/path%20with%20spaces/servletPath"));
assertThat("http servlet mapping matchValue is correct", responseBody, containsString("async:run:attr:mapping:matchValue:path%20with%20spaces/servletPath"));
assertThat("http servlet mapping pattern is correct", responseBody, containsString("async:run:attr:mapping:pattern:/path%20with%20spaces/servletPath"));
assertThat("http servlet mapping matchValue is correct", responseBody, containsString("async:run:attr:mapping:matchValue:path with spaces/servletPath"));
assertThat("http servlet mapping pattern is correct", responseBody, containsString("async:run:attr:mapping:pattern:/path with spaces/servletPath"));
assertThat("http servlet mapping servletName is correct", responseBody, containsString("async:run:attr:mapping:servletName:"));
assertThat("http servlet mapping mappingMatch is correct", responseBody, containsString("async:run:attr:mapping:mappingMatch:EXACT"));
}

26
jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/EncodedURITest.java
View File

@ -35,6 +35,7 @@ import org.eclipse.jetty.util.URIUtil;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
@ -90,8 +91,8 @@ public class EncodedURITest
assertThat(response, startsWith("HTTP/1.1 200 "));
assertThat(response, Matchers.containsString("requestURI=/c%6Fntext%20path/test%20servlet/path%20info"));
assertThat(response, Matchers.containsString("contextPath=/context%20path"));
assertThat(response, Matchers.containsString("servletPath=/test%20servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path%20info"));
assertThat(response, Matchers.containsString("servletPath=/test servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path info"));
}
@Test
@ -101,8 +102,8 @@ public class EncodedURITest
assertThat(response, startsWith("HTTP/1.1 200 "));
assertThat(response, Matchers.containsString("requestURI=/context%20path/test%20servlet/path%20info"));
assertThat(response, Matchers.containsString("contextPath=/context%20path"));
assertThat(response, Matchers.containsString("servletPath=/test%20servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path%20info"));
assertThat(response, Matchers.containsString("servletPath=/test servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path info"));
}
@Test
@ -112,8 +113,8 @@ public class EncodedURITest
assertThat(response, startsWith("HTTP/1.1 200 "));
assertThat(response, Matchers.containsString("requestURI=/context%20path/test%20servlet/path%20info"));
assertThat(response, Matchers.containsString("contextPath=/context%20path"));
assertThat(response, Matchers.containsString("servletPath=/test%20servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path%20info"));
assertThat(response, Matchers.containsString("servletPath=/test servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path info"));
}
@Test
@ -123,19 +124,20 @@ public class EncodedURITest
assertThat(response, startsWith("HTTP/1.1 200 "));
assertThat(response, Matchers.containsString("requestURI=/context%20path/test%20servlet/path%20info"));
assertThat(response, Matchers.containsString("contextPath=/context%20path"));
assertThat(response, Matchers.containsString("servletPath=/test%20servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path%20info"));
assertThat(response, Matchers.containsString("servletPath=/test servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path info"));
}
@Test
@Test // TODO Need to check spec if encoded async dispatch is really supported
@Disabled
public void testAsyncServletTestServletEncoded() throws Exception
{
String response = _connector.getResponse("GET /context%20path/async%20servlet/path%20info?encode=true HTTP/1.0\n\n");
assertThat(response, startsWith("HTTP/1.1 200 "));
assertThat(response, Matchers.containsString("requestURI=/context%20path/test%20servlet/path%2520info"));
assertThat(response, Matchers.containsString("contextPath=/context%20path"));
assertThat(response, Matchers.containsString("servletPath=/test%20servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path%2520info"));
assertThat(response, Matchers.containsString("servletPath=/test servlet"));
assertThat(response, Matchers.containsString("pathInfo=/path info"));
}
@ParameterizedTest
@ -182,7 +184,7 @@ public class EncodedURITest
if (Boolean.parseBoolean(request.getParameter("encode")))
async.dispatch("/test%20servlet" + URIUtil.encodePath(request.getPathInfo()));
else
async.dispatch("/test servlet/path info" + request.getPathInfo());
async.dispatch("/test servlet" + request.getPathInfo());
return;
}
}

54
jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java
View File

@ -20,6 +20,7 @@ import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.server.ForwardedRequestCustomizer;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.LocalConnector;
@ -121,4 +122,57 @@ public class RequestTest
assertThat("request.getRequestURL", resultRequestURL.get(), is("http://myhost:9999/"));
assertThat("request.getRequestURI", resultRequestURI.get(), is("/"));
}
@Test
public void testSafeURI() throws Exception
{
startServer(new HttpServlet()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse resp)
{
}
});
_connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986_UNAMBIGUOUS);
String rawResponse = _connector.getResponse(
"""
GET /test/foo%2fbar HTTP/1.1\r
Host: localhost\r
Connection: close\r
\r
""");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(HttpStatus.BAD_REQUEST_400));
}
@Test
public void testGetWithEncodedURI() throws Exception
{
final AtomicReference<String> resultRequestURI = new AtomicReference<>();
final AtomicReference<String> resultPathInfo = new AtomicReference<>();
startServer(new HttpServlet()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse resp)
{
resultRequestURI.set(request.getRequestURI());
resultPathInfo.set(request.getPathInfo());
}
});
String rawResponse = _connector.getResponse(
"""
GET /test/path%20info/foo%2fbar HTTP/1.1\r
Host: localhost\r
Connection: close\r
\r
""");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat("request.getRequestURI", resultRequestURI.get(), is("/test/path%20info/foo%2fbar"));
assertThat("request.getPathInfo", resultPathInfo.get(), is("/test/path info/foo%2Fbar"));
}
}