diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java index ed2554f9050..2dbdc5e29d6 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java @@ -99,19 +99,29 @@ public final class UriCompliance implements ComplianceViolation.Mode } } + public static final Set AMBIGUOUS_VIOLATIONS = EnumSet.of( + Violation.AMBIGUOUS_EMPTY_SEGMENT, + Violation.AMBIGUOUS_PATH_ENCODING, + Violation.AMBIGUOUS_PATH_PARAMETER, + Violation.AMBIGUOUS_PATH_SEGMENT, + Violation.AMBIGUOUS_PATH_SEPARATOR); + /** - * 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 all out {@link Violation}s. + * Compliance mode that exactly follows RFC3986, + * excluding all URI Violations. */ - public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", - of(Violation.AMBIGUOUS_PATH_SEPARATOR, - Violation.AMBIGUOUS_PATH_ENCODING)); + public static final UriCompliance RFC3986 = new UriCompliance("RFC3986", noneOf(Violation.class)); + + /** + * The default compliance mode allows no violations from RFC3986 + * and is equivalent to {@link #RFC3986} compliance. + */ + public static final UriCompliance DEFAULT = RFC3986; /** * LEGACY compliance mode that models Jetty-9.4 behavior by allowing {@link Violation#AMBIGUOUS_PATH_SEGMENT}, * {@link Violation#AMBIGUOUS_EMPTY_SEGMENT}, {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, {@link Violation#AMBIGUOUS_PATH_ENCODING} - * and {@link Violation#UTF16_ENCODINGS} + * and {@link Violation#UTF16_ENCODINGS}. */ public static final UriCompliance LEGACY = new UriCompliance("LEGACY", of(Violation.AMBIGUOUS_PATH_SEGMENT, @@ -121,24 +131,12 @@ public final class UriCompliance implements ComplianceViolation.Mode Violation.UTF16_ENCODINGS)); /** - * Compliance mode that exactly follows RFC3986, - * including allowing all additional ambiguous URI Violations. - */ - public static final UriCompliance RFC3986 = new UriCompliance("RFC3986", allOf(Violation.class)); - - /** - * Compliance mode that follows RFC3986 - * plus it does not allow any ambiguous URI {@link Violation}s. - */ - public static final UriCompliance RFC3986_UNAMBIGUOUS = new UriCompliance("RFC3986_UNAMBIGUOUS", noneOf(Violation.class)); - - /** - * Compliance mode that allows all URI Violations, including allowing ambiguous paths in non canonicalized form. + * Compliance mode that allows all URI Violations, including allowing ambiguous paths in non-canonical form. */ public static final UriCompliance UNSAFE = new UriCompliance("UNSAFE", allOf(Violation.class)); private static final AtomicInteger __custom = new AtomicInteger(); - private static final List KNOWN_MODES = List.of(DEFAULT, LEGACY, RFC3986, RFC3986_UNAMBIGUOUS, UNSAFE); + private static final List KNOWN_MODES = List.of(DEFAULT, LEGACY, RFC3986, UNSAFE); public static UriCompliance valueOf(String name) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Context.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Context.java index 54cd6331536..05321644783 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Context.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Context.java @@ -38,7 +38,7 @@ import org.eclipse.jetty.util.resource.Resource; public interface Context extends Attributes, Decorator, Executor { /** - * @return the context path of this Context + * @return the encoded context path of this Context */ String getContextPath(); @@ -94,18 +94,46 @@ public interface Context extends Attributes, Decorator, Executor void run(Runnable task, Request request); /** - *

Returns a URI path scoped to this Context.

- *

For example, if the context path is {@code /ctx} then a - * full path of {@code /ctx/foo/bar} will return {@code /foo/bar}.

- * - * @param fullPath a full URI path + *

Returns the URI path scoped to this Context.

+ * @see #getPathInContext(String, String) + * @param canonicallyEncodedPath a full URI path that should be canonically encoded as + * per {@link org.eclipse.jetty.util.URIUtil#canonicalPath(String)} * @return the URI path scoped to this Context, or {@code null} if the full path does not match this Context. - * The empty string is returned if the full path is exactly the context path. + * The empty string is returned if the full path is exactly the context path. */ - String getPathInContext(String fullPath); + default String getPathInContext(String canonicallyEncodedPath) + { + return getPathInContext(getContextPath(), canonicallyEncodedPath); + } /** * @return a non-{@code null} temporary directory, configured either for the context, the server or the JVM */ File getTempDirectory(); + + /** + *

Returns the URI path scoped to the passed context path.

+ *

For example, if the context path passed is {@code /ctx} then a + * path of {@code /ctx/foo/bar} will return {@code /foo/bar}.

+ * + * @param encodedContextPath The context path that should be canonically encoded as + * per {@link org.eclipse.jetty.util.URIUtil#canonicalPath(String)}. + * @param encodedPath a full URI path that should be canonically encoded as + * per {@link org.eclipse.jetty.util.URIUtil#canonicalPath(String)}. + * @return the URI {@code encodedPath} scoped to the {@code encodedContextPath}, + * or {@code null} if the {@code encodedPath} does not match the context. + * The empty string is returned if the {@code encodedPath} is exactly the {@code encodedContextPath}. + */ + static String getPathInContext(String encodedContextPath, String encodedPath) + { + if (encodedContextPath.length() == 0 || "/".equals(encodedContextPath)) + return encodedPath; + if (encodedContextPath.length() > encodedPath.length() || !encodedPath.startsWith(encodedContextPath)) + return null; + if (encodedPath.length() == encodedContextPath.length()) + return ""; + if (encodedPath.charAt(encodedContextPath.length()) != '/') + return null; + return encodedPath.substring(encodedContextPath.length()); + } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 47108db886e..8af3097cb68 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -35,7 +35,6 @@ import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.Trailers; import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.server.internal.HttpChannelState; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.Callback; @@ -726,14 +725,14 @@ public interface Request extends Attributes, Content.Source * {@code newPathInContext=/newPath}, the returned HttpURI is {@code http://host/ctx/newPath?a=b}.

* * @param request The request to base the new HttpURI on. - * @param newPathInContext The new path in context for the new HttpURI + * @param newEncodedPathInContext The new path in context for the new HttpURI * @return A new immutable HttpURI with the path in context replaced, but query string and path * parameters retained. */ - static HttpURI newHttpURIFrom(Request request, String newPathInContext) + static HttpURI newHttpURIFrom(Request request, String newEncodedPathInContext) { return HttpURI.build(request.getHttpURI()) - .path(URIUtil.addPaths(getContextPath(request), newPathInContext)) + .path(URIUtil.addPaths(getContextPath(request), newEncodedPathInContext)) .asImmutable(); } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 56ced241817..2d2045bdc38 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -914,9 +914,9 @@ public class Server extends Handler.Wrapper implements Attributes } @Override - public String getPathInContext(String fullPath) + public String getPathInContext(String canonicallyEncodedPath) { - return fullPath; + return canonicallyEncodedPath; } @Override diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 932ee10e703..284a80f442a 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -1297,17 +1297,9 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace } @Override - public String getPathInContext(String fullPath) + public String getPathInContext(String canonicallyEncodedPath) { - if (_rootContext) - return fullPath; - if (!fullPath.startsWith(_contextPath)) - return null; - if (fullPath.length() == _contextPath.length()) - return ""; - if (fullPath.charAt(_contextPath.length()) != '/') - return null; - return fullPath.substring(_contextPath.length()); + return _rootContext ? canonicallyEncodedPath : Context.getPathInContext(_contextPath, canonicallyEncodedPath); } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java index 4d64d9a57b2..6944d49026b 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java @@ -1390,7 +1390,7 @@ public class HttpConnectionTest Host: whatever """; - _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986_UNAMBIGUOUS); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); @@ -1409,6 +1409,10 @@ public class HttpConnectionTest _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.from(EnumSet.of(UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER))); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); } @@ -1425,7 +1429,7 @@ public class HttpConnectionTest _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")); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); } @Test @@ -1437,13 +1441,13 @@ public class HttpConnectionTest \r """; _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.from("DEFAULT,AMBIGUOUS_PATH_SEPARATOR")); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); _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")); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); } @Test @@ -1476,11 +1480,13 @@ public class HttpConnectionTest \r """; _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.from(EnumSet.of(UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING))); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); _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")); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); @@ -1500,12 +1506,10 @@ public class HttpConnectionTest """; _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); - _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986_UNAMBIGUOUS); - 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")); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 007be864626..fa9f93a2048 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -88,9 +88,7 @@ public class RequestTest \r """; HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(request)); - assertEquals(HttpStatus.OK_200, response.getStatus()); - assertThat(response.getContent(), containsString("httpURI.path=/fo%6f%2fbar")); - assertThat(response.getContent(), containsString("pathInContext=/foo%2Fbar")); + assertEquals(HttpStatus.BAD_REQUEST_400, response.getStatus()); } @Test diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index 498704092a0..94e206a82e4 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -28,7 +28,6 @@ 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; @@ -479,51 +478,6 @@ public final class URIUtil * @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 decoder) { try { @@ -550,13 +504,13 @@ public final class URIUtil String str = new String(codePoints, 0, 1); byte[] bytes = str.getBytes(StandardCharsets.UTF_8); for (byte b: bytes) - decoder.accept(builder, b); + builder.append(b); i += 5; } else { byte b = (byte)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2)))); - decoder.accept(builder, b); + builder.append(b); i += 2; } } @@ -706,40 +660,41 @@ public final class URIUtil *

* For example the path {@code /fo %2fo/b%61r} will be normalized to {@code /fo%20%2Fo/bar}, * whilst {@link #decodePath(String)} would result in the ambiguous and URI illegal {@code /fo /o/bar}. + * @param encodedPath An encoded URI path * @return the canonical path or null if it is non-normal * @see #decodePath(String) * @see #normalizePath(String) * @see URI */ - public static String canonicalPath(String path) + public static String canonicalPath(String encodedPath) { - if (path == null) + if (encodedPath == null) return null; try { Utf8StringBuilder builder = null; - int end = path.length(); + int end = encodedPath.length(); boolean slash = true; boolean normal = true; for (int i = 0; i < end; i++) { - char c = path.charAt(i); + char c = encodedPath.charAt(i); switch (c) { case '%': if (builder == null) { - builder = new Utf8StringBuilder(path.length()); - builder.append(path, 0, i); + builder = new Utf8StringBuilder(encodedPath.length()); + builder.append(encodedPath, 0, i); } if ((i + 2) < end) { - char u = path.charAt(i + 1); + char u = encodedPath.charAt(i + 1); if (u == 'u') { // UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. - int code = TypeUtil.parseInt(path, i + 2, 4, 16); + int code = TypeUtil.parseInt(encodedPath, i + 2, 4, 16); if (isSafeElseEncode(code, builder)) { char[] chars = Character.toChars(code); @@ -755,7 +710,7 @@ public final class URIUtil } else { - int code = TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2)); + int code = TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(encodedPath.charAt(i + 2)); if (isSafeElseEncode(code, builder)) { builder.append((byte)(0xff & code)); @@ -774,13 +729,13 @@ public final class URIUtil case ';': if (builder == null) { - builder = new Utf8StringBuilder(path.length()); - builder.append(path, 0, i); + builder = new Utf8StringBuilder(encodedPath.length()); + builder.append(encodedPath, 0, i); } while (++i < end) { - if (path.charAt(i) == '/') + if (encodedPath.charAt(i) == '/') { builder.append('/'); break; @@ -803,8 +758,8 @@ public final class URIUtil default: if (builder == null && !isSafe(c)) { - builder = new Utf8StringBuilder(path.length()); - builder.append(path, 0, i); + builder = new Utf8StringBuilder(encodedPath.length()); + builder.append(encodedPath, 0, i); } if (builder != null && isSafeElseEncode(c, builder)) @@ -815,13 +770,13 @@ public final class URIUtil slash = c == '/'; } - String canonical = (builder != null) ? builder.toString() : path; + String canonical = (builder != null) ? builder.toString() : encodedPath; return normal ? canonical : normalizePath(canonical); } catch (NotUtf8Exception e) { if (LOG.isDebugEnabled()) - LOG.debug("{} {}", path, e.toString()); + LOG.debug("{} {}", encodedPath, e.toString()); throw e; } catch (IllegalArgumentException e) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java index 15c1483fc5e..5820fba36e3 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java @@ -16,7 +16,6 @@ package org.eclipse.jetty.util; import java.io.File; import java.io.IOException; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.text.Normalizer; @@ -37,8 +36,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; @@ -95,60 +92,79 @@ public class URIUtilTest public static Stream decodePathSource() { - List arguments = new ArrayList<>(); - arguments.add(Arguments.of("/foo/bar", "/foo/bar", "/foo/bar")); + return Stream.of( + Arguments.of("/foo/bar", "/foo/bar", "/foo/bar"), - // Simple encoding - arguments.add(Arguments.of("/f%20%6f/b%20r", "/f%20o/b%20r", "/f o/b r")); + // Simple encoding + Arguments.of("/f%20%6f/b%20r", "/f%20o/b%20r", "/f o/b r"), - // UTF8 and unicode handling - // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck - arguments.add(Arguments.of("/foo/b\u00e4\u00e4", "/foo/b\u00e4\u00e4", "/foo/b\u00e4\u00e4")); - arguments.add(Arguments.of("/f%d8%a9%D8%A9/bar", "/f\u0629\u0629/bar", "/f\u0629\u0629/bar")); + // UTF8 and unicode handling + // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck + Arguments.of("/foo/b\u00e4\u00e4", "/foo/bää", "/foo/bää"), + Arguments.of("/f\u20ac\u20ac/bar", "/f€€/bar", "/f€€/bar"), + Arguments.of("/f%d8%a9%D8%A9/bar", "/f\u0629\u0629/bar", "/f\u0629\u0629/bar"), - // Encoded delimiters - arguments.add(Arguments.of("/foo%2fbar", "/foo%2Fbar", "/foo/bar")); - arguments.add(Arguments.of("/foo%252fbar", "/foo%252fbar", "/foo%2fbar")); - arguments.add(Arguments.of("/foo%3bbar", "/foo%3Bbar", "/foo;bar")); - arguments.add(Arguments.of("/foo%3fbar", "/foo%3Fbar", "/foo?bar")); + // Encoded UTF-8 unicode (euro) + Arguments.of("/f%e2%82%ac%E2%82%AC/bar", "/f€€/bar", "/f€€/bar"), - // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck - arguments.add(Arguments.of("/f%20o/b%20r", "/f%20o/b%20r", "/f o/b r")); - arguments.add(Arguments.of("f\u00e4\u00e4%2523%3b%2c:%3db%20a%20r%3D", "f\u00e4\u00e4%2523%3B,:=b%20a%20r=", "f\u00e4\u00e4%23;,:=b a r=")); - arguments.add(Arguments.of("f%d8%a9%D8%A9%2523%3b%2c:%3db%20a%20r", "f\u0629\u0629%2523%3B,:=b%20a%20r", "f\u0629\u0629%23;,:=b a r")); + // Encoded delimiters + Arguments.of("/foo%2fbar", "/foo%2Fbar", "/foo/bar"), + Arguments.of("/foo%252fbar", "/foo%252fbar", "/foo%2fbar"), + Arguments.of("/foo%3bbar", "/foo%3Bbar", "/foo;bar"), + Arguments.of("/foo%3fbar", "/foo%3Fbar", "/foo?bar"), - // path parameters should be ignored - arguments.add(Arguments.of("/foo;ignore/bar;ignore", "/foo/bar", "/foo/bar")); - arguments.add(Arguments.of("/f\u00e4\u00e4;ignore/bar;ignore", "/fää/bar", "/fää/bar")); - arguments.add(Arguments.of("/f%d8%a9%d8%a9%2523;ignore/bar;ignore", "/f\u0629\u0629%2523/bar", "/f\u0629\u0629%23/bar")); - arguments.add(Arguments.of("foo%2523%3b%2c:%3db%20a%20r;rubbish", "foo%2523%3B,:=b%20a%20r", "foo%23;,:=b a r")); + // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck + Arguments.of("/f%20o/b%20r", "/f%20o/b%20r", "/f o/b r"), + Arguments.of("f\u00e4\u00e4%2523%3b%2c:%3db%20a%20r%3D", "f\u00e4\u00e4%2523%3B,:=b%20a%20r=", "f\u00e4\u00e4%23;,:=b a r="), + Arguments.of("f%d8%a9%D8%A9%2523%3b%2c:%3db%20a%20r", "f\u0629\u0629%2523%3B,:=b%20a%20r", "f\u0629\u0629%23;,:=b a r"), - // test for chars that are somehow already decoded, but shouldn't be - arguments.add(Arguments.of("/foo bar\n", "/foo%20bar%0A", "/foo bar\n")); - arguments.add(Arguments.of("/foo\u0000bar", "/foo%00bar", "/foo\u0000bar")); - arguments.add(Arguments.of("/foo/bär", "/foo/bär", "/foo/bär")); - arguments.add(Arguments.of("/foo/€/bar", "/foo/€/bar", "/foo/€/bar")); - arguments.add(Arguments.of("/fo %2fo/b%61r", "/fo%20%2Fo/bar", "/fo /o/bar")); + // path parameters should be ignored + Arguments.of("/foo;ignore/bar;ignore", "/foo/bar", "/foo/bar"), + Arguments.of("/f\u00e4\u00e4;ignore/bar;ignore", "/fää/bar", "/fää/bar"), + Arguments.of("/f%d8%a9%d8%a9%2523;ignore/bar;ignore", "/f\u0629\u0629%2523/bar", "/f\u0629\u0629%23/bar"), + Arguments.of("foo%2523%3b%2c:%3db%20a%20r;rubbish", "foo%2523%3B,:=b%20a%20r", "foo%23;,:=b a r"), - // Test for null character (real world ugly test case) - byte[] oddBytes = {'/', 0x00, '/'}; - String odd = new String(oddBytes, StandardCharsets.ISO_8859_1); - arguments.add(Arguments.of("/%00/", "/%00/", odd)); + // test for chars that are somehow already decoded, but shouldn't be + Arguments.of("/foo bar\n", "/foo%20bar%0A", "/foo bar\n"), + Arguments.of("/foo\u0000bar", "/foo%00bar", "/foo\u0000bar"), + Arguments.of("/foo/bär", "/foo/bär", "/foo/bär"), + Arguments.of("/foo/€/bar", "/foo/€/bar", "/foo/€/bar"), + Arguments.of("/fo %2fo/b%61r", "/fo%20%2Fo/bar", "/fo /o/bar"), - // Deprecated Microsoft Percent-U encoding - arguments.add(Arguments.of("abc%u3040", "abc\u3040", "abc\u3040")); + // Test for null character (real world ugly test case) + Arguments.of("/%00/", "/%00/", "/\u0000/"), - // Canonical paths are also normalized - arguments.add(Arguments.of("./bar", "bar", "./bar")); - arguments.add(Arguments.of("/foo/./bar", "/foo/bar", "/foo/./bar")); - arguments.add(Arguments.of("/foo/../bar", "/bar", "/foo/../bar")); - arguments.add(Arguments.of("/foo/.../bar", "/foo/.../bar", "/foo/.../bar")); - arguments.add(Arguments.of("/foo/%2e/bar", "/foo/bar", "/foo/./bar")); // Not by the RFC, but safer - arguments.add(Arguments.of("/foo/%2e%2e/bar", "/bar", "/foo/../bar")); // Not by the RFC, but safer - arguments.add(Arguments.of("/foo/%2e%2e%2e/bar", "/foo/.../bar", "/foo/.../bar")); + // Deprecated Microsoft Percent-U encoding + Arguments.of("abc%u3040", "abc\u3040", "abc\u3040"), - return arguments.stream(); + // Invalid UTF-8 - replacement characters should be present on invalid sequences + // URI paths do not support ISO-8859-1, so this should not be a fallback of our decodePath implementation + /* TODO: remove ISO-8859-1 fallback mode in decodePath - Issue #9489 + Arguments.of("/a%D8%2fbar", "/a�%2Fbar", "/a�%2Fbar"), // invalid 2 octet sequence + Arguments.of("/abc%C3%28", "/abc�", "/abc�"), // invalid 2 octet sequence + Arguments.of("/abc%A0%A1", "/abc��", "/abc��"), // invalid 2 octet sequence + Arguments.of("/abc%e2%28%a1", "/abc��", "/abc��"), // invalid 3 octet sequence + Arguments.of("/abc%e2%82%28", "/abc�", "/abc�"), // invalid 3 octet sequence + Arguments.of("/abc%f0%28%8c%bc", "/abc���", "/abc���"), // invalid 4 octet sequence + Arguments.of("/abc%f0%90%28%bc", "/abc��", "/abc��"), // invalid 4 octet sequence + Arguments.of("/abc%f0%28%8c%28", "/abc��(", "/abc��("), // invalid 4 octet sequence + Arguments.of("/abc%f8%a1%a1%a1%a1", "/abc�����", "/abc�����"), // valid sequence, but not unicode + Arguments.of("/abc%fc%a1%a1%a1%a1%a1", "/abc������", "/abc������"), // valid sequence, but not unicode + Arguments.of("/abc%f8%a1%a1%a1", "/abc����", "/abc����"), // incomplete sequence + */ + // Deprecated Microsoft Percent-U encoding + Arguments.of("/abc%u3040", "/abc\u3040", "/abc\u3040"), + + // Canonical paths are also normalized + Arguments.of("./bar", "bar", "./bar"), + Arguments.of("/foo/./bar", "/foo/bar", "/foo/./bar"), + Arguments.of("/foo/../bar", "/bar", "/foo/../bar"), + Arguments.of("/foo/.../bar", "/foo/.../bar", "/foo/.../bar"), + Arguments.of("/foo/%2e/bar", "/foo/bar", "/foo/./bar"), // Not by the RFC, but safer + Arguments.of("/foo/%2e%2e/bar", "/bar", "/foo/../bar"), // Not by the RFC, but safer + Arguments.of("/foo/%2e%2e%2e/bar", "/foo/.../bar", "/foo/.../bar") + ); } @ParameterizedTest(name = "[{index}] {0}") @@ -171,14 +187,6 @@ public class URIUtilTest { List arguments = new ArrayList<>(); - // Test for null character (real world ugly test case) - // TODO is this a bad decoding or a bad URI ? - // arguments.add(Arguments.of("/%00/")); - - // Deprecated Microsoft Percent-U encoding - // TODO still supported for now ? - // arguments.add(Arguments.of("abc%u3040")); - // Bad %## encoding arguments.add(Arguments.of("abc%xyz")); @@ -196,21 +204,6 @@ public class URIUtilTest arguments.add(Arguments.of("abc%uA")); arguments.add(Arguments.of("abc%u")); - // Invalid UTF-8 and ISO8859-1 - // TODO currently ISO8859 is too forgiving to detect these - /* - arguments.add(Arguments.of("abc%C3%28")); // invalid 2 octext sequence - arguments.add(Arguments.of("abc%A0%A1")); // invalid 2 octext sequence - arguments.add(Arguments.of("abc%e2%28%a1")); // invalid 3 octext sequence - arguments.add(Arguments.of("abc%e2%82%28")); // invalid 3 octext sequence - arguments.add(Arguments.of("abc%f0%28%8c%bc")); // invalid 4 octext sequence - arguments.add(Arguments.of("abc%f0%90%28%bc")); // invalid 4 octext sequence - arguments.add(Arguments.of("abc%f0%28%8c%28")); // invalid 4 octext sequence - arguments.add(Arguments.of("abc%f8%a1%a1%a1%a1")); // valid sequence, but not unicode - arguments.add(Arguments.of("abc%fc%a1%a1%a1%a1%a1")); // valid sequence, but not unicode - arguments.add(Arguments.of("abc%f8%a1%a1%a1")); // incomplete sequence - */ - return arguments.stream(); } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 4a0bd0dc7e8..1541ba46afa 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -359,18 +359,22 @@ public class DefaultServlet extends HttpServlet { String includedServletPath = (String)req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); boolean included = includedServletPath != null; - String pathInContext; + String encodedPathInContext; if (included) - pathInContext = getIncludedPathInContext(req, includedServletPath, isPathInfoOnly()); + encodedPathInContext = URIUtil.encodePath(getIncludedPathInContext(req, includedServletPath, isPathInfoOnly())); + else if (isPathInfoOnly()) + encodedPathInContext = URIUtil.encodePath(req.getPathInfo()); + else if (req instanceof ServletApiRequest apiRequest) + encodedPathInContext = Context.getPathInContext(req.getContextPath(), apiRequest.getServletContextRequest().getHttpURI().getCanonicalPath()); else - pathInContext = URIUtil.addPaths(isPathInfoOnly() ? "/" : req.getServletPath(), req.getPathInfo()); - + encodedPathInContext = Context.getPathInContext(req.getContextPath(), URIUtil.canonicalPath(req.getRequestURI())); + if (LOG.isDebugEnabled()) - LOG.debug("doGet(req={}, resp={}) pathInContext={}, included={}", req, resp, pathInContext, included); + LOG.debug("doGet(req={}, resp={}) pathInContext={}, included={}", req, resp, encodedPathInContext, included); try { - HttpContent content = _resourceService.getContent(pathInContext, ServletContextRequest.getServletContextRequest(req)); + HttpContent content = _resourceService.getContent(encodedPathInContext, ServletContextRequest.getServletContextRequest(req)); if (LOG.isDebugEnabled()) LOG.debug("content = {}", content); @@ -384,7 +388,7 @@ public class DefaultServlet extends HttpServlet * If the exception isn’t caught and handled, and the response * hasn’t been committed, the status code MUST be set to 500. */ - throw new FileNotFoundException(pathInContext); + throw new FileNotFoundException(encodedPathInContext); } // no content @@ -432,9 +436,9 @@ public class DefaultServlet extends HttpServlet catch (InvalidPathException e) { if (LOG.isDebugEnabled()) - LOG.debug("InvalidPathException for pathInContext: {}", pathInContext, e); + LOG.debug("InvalidPathException for pathInContext: {}", encodedPathInContext, e); if (included) - throw new FileNotFoundException(pathInContext); + throw new FileNotFoundException(encodedPathInContext); resp.setStatus(404); } } @@ -481,9 +485,9 @@ public class DefaultServlet extends HttpServlet if (request.getDispatcherType() == DispatcherType.REQUEST) _uri = getWrapped().getHttpURI(); else if (included) - _uri = Request.newHttpURIFrom(getWrapped(), getIncludedPathInContext(request, includedServletPath, false)); + _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.encodePath(getIncludedPathInContext(request, includedServletPath, false))); else - _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.addPaths(_servletRequest.getServletPath(), _servletRequest.getPathInfo())); + _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.encodePath(URIUtil.addPaths(_servletRequest.getServletPath(), _servletRequest.getPathInfo()))); } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java index e62ba6a4a9e..dc03681a9b2 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java @@ -35,7 +35,6 @@ import jakarta.servlet.http.HttpServletResponseWrapper; import org.eclipse.jetty.ee10.servlet.util.ServletOutputStreamWrapper; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.pathmap.MatchedResource; -import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; @@ -64,30 +63,30 @@ public class Dispatcher implements RequestDispatcher private final ServletContextHandler _contextHandler; private final HttpURI _uri; - private final String _pathInContext; + private final String _decodedPathInContext; private final String _named; private final ServletHandler.MappedServlet _mappedServlet; private final ServletHandler _servletHandler; private final ServletPathMapping _servletPathMapping; - public Dispatcher(ServletContextHandler contextHandler, HttpURI uri, String pathInContext) + public Dispatcher(ServletContextHandler contextHandler, HttpURI uri, String decodedPathInContext) { _contextHandler = contextHandler; _uri = uri.asImmutable(); - _pathInContext = pathInContext; + _decodedPathInContext = decodedPathInContext; _named = null; _servletHandler = _contextHandler.getServletHandler(); - MatchedResource matchedServlet = _servletHandler.getMatchedServlet(pathInContext); + MatchedResource matchedServlet = _servletHandler.getMatchedServlet(decodedPathInContext); _mappedServlet = matchedServlet.getResource(); - _servletPathMapping = _mappedServlet.getServletPathMapping(_pathInContext, matchedServlet.getMatchedPath()); + _servletPathMapping = _mappedServlet.getServletPathMapping(_decodedPathInContext, matchedServlet.getMatchedPath()); } public Dispatcher(ServletContextHandler contextHandler, String name) throws IllegalStateException { _contextHandler = contextHandler; _uri = null; - _pathInContext = null; + _decodedPathInContext = null; _named = name; _servletHandler = _contextHandler.getServletHandler(); @@ -100,7 +99,7 @@ public class Dispatcher implements RequestDispatcher HttpServletRequest httpRequest = (request instanceof HttpServletRequest) ? (HttpServletRequest)request : new ServletRequestHttpWrapper(request); HttpServletResponse httpResponse = (response instanceof HttpServletResponse) ? (HttpServletResponse)response : new ServletResponseHttpWrapper(response); - _mappedServlet.handle(_servletHandler, _pathInContext, new ErrorRequest(httpRequest), httpResponse); + _mappedServlet.handle(_servletHandler, _decodedPathInContext, new ErrorRequest(httpRequest), httpResponse); } @Override @@ -111,7 +110,7 @@ public class Dispatcher implements RequestDispatcher ServletContextRequest servletContextRequest = ServletContextRequest.getServletContextRequest(request); servletContextRequest.getResponse().resetForForward(); - _mappedServlet.handle(_servletHandler, _pathInContext, new ForwardRequest(httpRequest), httpResponse); + _mappedServlet.handle(_servletHandler, _decodedPathInContext, new ForwardRequest(httpRequest), httpResponse); // If we are not async and not closed already, then close via the possibly wrapped response. if (!servletContextRequest.getState().isAsync() && !servletContextRequest.getHttpOutput().isClosed()) @@ -136,7 +135,7 @@ public class Dispatcher implements RequestDispatcher try { - _mappedServlet.handle(_servletHandler, _pathInContext, new IncludeRequest(httpRequest), new IncludeResponse(httpResponse)); + _mappedServlet.handle(_servletHandler, _decodedPathInContext, new IncludeRequest(httpRequest), new IncludeResponse(httpResponse)); } finally { @@ -149,7 +148,7 @@ public class Dispatcher implements RequestDispatcher HttpServletRequest httpRequest = (request instanceof HttpServletRequest) ? (HttpServletRequest)request : new ServletRequestHttpWrapper(request); HttpServletResponse httpResponse = (response instanceof HttpServletResponse) ? (HttpServletResponse)response : new ServletResponseHttpWrapper(response); - _mappedServlet.handle(_servletHandler, _pathInContext, new AsyncRequest(httpRequest), httpResponse); + _mappedServlet.handle(_servletHandler, _decodedPathInContext, new AsyncRequest(httpRequest), httpResponse); } public class ParameterRequestWrapper extends HttpServletRequestWrapper diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java index d4e21360f0e..af5b77796f1 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java @@ -60,6 +60,7 @@ import org.eclipse.jetty.ee10.servlet.security.UserIdentity; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; @@ -1117,7 +1118,7 @@ public class ServletApiRequest implements HttpServletRequest // handle relative path if (!path.startsWith("/")) { - String relTo = _request.getPathInContext(); + String relTo = _request.getDecodedPathInContext(); int slash = relTo.lastIndexOf("/"); if (slash > 1) relTo = relTo.substring(0, slash + 1); @@ -1195,7 +1196,7 @@ public class ServletApiRequest implements HttpServletRequest @Override public HttpServletMapping getHttpServletMapping() { - return _request._mappedServlet.getServletPathMapping(_request.getPathInContext()); + return _request._mappedServlet.getServletPathMapping(_request.getDecodedPathInContext()); } @Override @@ -1246,4 +1247,24 @@ public class ServletApiRequest implements HttpServletRequest } return trailersMap; } + + static class AmbiguousURI extends ServletApiRequest + { + protected AmbiguousURI(ServletContextRequest servletContextRequest) + { + super(servletContextRequest); + } + + @Override + public String getPathInfo() + { + throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding"); + } + + @Override + public String getServletPath() + { + throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding"); + } + } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index b96672d309e..0ca08507cb2 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -461,9 +461,9 @@ public class ServletChannel } } // We first worked with the core pathInContext above, but now need to convert to servlet style - pathInContext = URIUtil.safeDecodePath(pathInContext); + String decodedPathInContext = URIUtil.decodePath(pathInContext); - Dispatcher dispatcher = new Dispatcher(getContextHandler(), uri, pathInContext); + Dispatcher dispatcher = new Dispatcher(getContextHandler(), uri, decodedPathInContext); dispatcher.async(asyncContextEvent.getSuppliedRequest(), asyncContextEvent.getSuppliedResponse()); }); break; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index a8d7310a43f..e62311087fd 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -1150,10 +1150,10 @@ public class ServletContextHandler extends ContextHandler implements Graceful protected ServletContextRequest newServletContextRequest(ServletChannel servletChannel, Request request, Response response, - String pathInContext, + String decodedPathInContext, MatchedResource matchedResource) { - return new ServletContextRequest(_servletContext, servletChannel, request, response, pathInContext, matchedResource, getSessionHandler()); + return new ServletContextRequest(_servletContext, servletChannel, request, response, decodedPathInContext, matchedResource, getSessionHandler()); } @Override @@ -1161,9 +1161,9 @@ 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 = URIUtil.safeDecodePath(getContext().getPathInContext(request.getHttpURI().getCanonicalPath())); + String decodedPathInContext = URIUtil.decodePath(getContext().getPathInContext(request.getHttpURI().getCanonicalPath())); - MatchedResource matchedResource = _servletHandler.getMatchedServlet(pathInContext); + MatchedResource matchedResource = _servletHandler.getMatchedServlet(decodedPathInContext); if (matchedResource == null) return null; ServletHandler.MappedServlet mappedServlet = matchedResource.getResource(); @@ -1179,7 +1179,7 @@ public class ServletContextHandler extends ContextHandler implements Graceful cache.setAttribute(ServletChannel.class.getName(), servletChannel); } - ServletContextRequest servletContextRequest = newServletContextRequest(servletChannel, request, response, pathInContext, matchedResource); + ServletContextRequest servletContextRequest = newServletContextRequest(servletChannel, request, response, decodedPathInContext, matchedResource); servletChannel.associate(servletContextRequest); return servletContextRequest; } @@ -1197,7 +1197,7 @@ public class ServletContextHandler extends ContextHandler implements Graceful { ServletContextRequest scopedRequest = Request.as(request, ServletContextRequest.class); DispatcherType dispatch = scopedRequest.getHttpServletRequest().getDispatcherType(); - if (dispatch == DispatcherType.REQUEST && isProtectedTarget(scopedRequest.getPathInContext())) + if (dispatch == DispatcherType.REQUEST && isProtectedTarget(scopedRequest.getDecodedPathInContext())) { Response.writeError(request, response, callback, HttpServletResponse.SC_NOT_FOUND, null); return true; @@ -2824,16 +2824,16 @@ public class ServletContextHandler extends ContextHandler implements Graceful String contextPath = getContextPath(); // uriInContext is canonicalized by HttpURI. HttpURI.Mutable uri = HttpURI.build(uriInContext); - String pathInfo = uri.getCanonicalPath(); - if (StringUtil.isEmpty(pathInfo)) + String encodedPathInContext = uri.getCanonicalPath(); + if (StringUtil.isEmpty(encodedPathInContext)) return null; if (!StringUtil.isEmpty(contextPath)) { uri.path(URIUtil.addPaths(contextPath, uri.getPath())); - pathInfo = uri.getCanonicalPath().substring(contextPath.length()); + encodedPathInContext = uri.getCanonicalPath().substring(contextPath.length()); } - return new Dispatcher(ServletContextHandler.this, uri, pathInfo); + return new Dispatcher(ServletContextHandler.this, uri, URIUtil.decodePath(encodedPathInContext)); } catch (Exception e) { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index df1d9e7e667..fd25ca3e639 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java @@ -27,6 +27,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.http.pathmap.MatchedPath; import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.http.pathmap.PathSpec; @@ -39,13 +40,10 @@ import org.eclipse.jetty.session.AbstractSessionManager; import org.eclipse.jetty.session.ManagedSession; import org.eclipse.jetty.session.SessionManager; import org.eclipse.jetty.util.Fields; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class ServletContextRequest extends ContextRequest { public static final String MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig"; - private static final Logger LOG = LoggerFactory.getLogger(ServletContextRequest.class); static final int INPUT_NONE = 0; static final int INPUT_STREAM = 1; static final int INPUT_READER = 2; @@ -78,7 +76,7 @@ public class ServletContextRequest extends ContextRequest private final ServletContextResponse _response; final ServletHandler.MappedServlet _mappedServlet; private final HttpInput _httpInput; - private final String _pathInContext; + private final String _decodedPathInContext; private final ServletChannel _servletChannel; private final PathSpec _pathSpec; private final SessionManager _sessionManager; @@ -93,7 +91,7 @@ public class ServletContextRequest extends ContextRequest ServletChannel servletChannel, Request request, Response response, - String pathInContext, + String decodedPathInContext, MatchedResource matchedResource, SessionManager sessionManager) { @@ -102,7 +100,7 @@ public class ServletContextRequest extends ContextRequest _httpServletRequest = newServletApiRequest(); _mappedServlet = matchedResource.getResource(); _httpInput = _servletChannel.getHttpInput(); - _pathInContext = pathInContext; + _decodedPathInContext = decodedPathInContext; _pathSpec = matchedResource.getPathSpec(); _matchedPath = matchedResource.getMatchedPath(); _response = newServletContextResponse(response); @@ -111,6 +109,15 @@ public class ServletContextRequest extends ContextRequest protected ServletApiRequest newServletApiRequest() { + if (getHttpURI().hasViolations() && !getServletChannel().getContextHandler().getServletHandler().isDecodeAmbiguousURIs()) + { + for (UriCompliance.Violation violation : getHttpURI().getViolations()) + { + if (UriCompliance.AMBIGUOUS_VIOLATIONS.contains(violation)) + return new ServletApiRequest.AmbiguousURI(this); + } + } + return new ServletApiRequest(this); } @@ -119,9 +126,9 @@ public class ServletContextRequest extends ContextRequest return new ServletContextResponse(_servletChannel, this, response); } - public String getPathInContext() + public String getDecodedPathInContext() { - return _pathInContext; + return _decodedPathInContext; } public PathSpec getPathSpec() @@ -209,8 +216,8 @@ public class ServletContextRequest extends ContextRequest { case "o.e.j.s.s.ServletScopedRequest.request" -> _httpServletRequest; case "o.e.j.s.s.ServletScopedRequest.response" -> _response.getHttpServletResponse(); - case "o.e.j.s.s.ServletScopedRequest.servlet" -> _mappedServlet.getServletPathMapping(getPathInContext()).getServletName(); - case "o.e.j.s.s.ServletScopedRequest.url-pattern" -> _mappedServlet.getServletPathMapping(getPathInContext()).getPattern(); + case "o.e.j.s.s.ServletScopedRequest.servlet" -> _mappedServlet.getServletPathMapping(getDecodedPathInContext()).getServletName(); + case "o.e.j.s.s.ServletScopedRequest.url-pattern" -> _mappedServlet.getServletPathMapping(getDecodedPathInContext()).getPattern(); default -> super.getAttribute(name); }; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java index 0dc96df0172..6d1abc14dfc 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java @@ -116,6 +116,7 @@ public class ServletHandler extends Handler.Wrapper @SuppressWarnings("unchecked") protected final ConcurrentMap[] _chainCache = new ConcurrentMap[FilterMapping.ALL]; + private boolean _decodeAmbiguousURIs = false; /** * Constructor. @@ -124,6 +125,21 @@ public class ServletHandler extends Handler.Wrapper { } + @ManagedAttribute(value = "True if URIs with violations are decoded") + public boolean isDecodeAmbiguousURIs() + { + return _decodeAmbiguousURIs; + } + + /** + * @param decodeAmbiguousURIs {@code True} if ambiguous URIs are decoded by {@link ServletApiRequest#getServletPath()} + * and {@link ServletApiRequest#getPathInfo()}. + */ + public void setDecodeAmbiguousURIs(boolean decodeAmbiguousURIs) + { + _decodeAmbiguousURIs = decodeAmbiguousURIs; + } + AutoLock lock() { return _lock.lock(); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/SecurityHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/SecurityHandler.java index 70f36321a61..ef78680ebdf 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/SecurityHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/SecurityHandler.java @@ -471,10 +471,10 @@ public abstract class SecurityHandler extends Handler.Wrapper implements Authent if (authenticator != null) authenticator.prepareRequest(request); - RoleInfo roleInfo = prepareConstraintInfo(servletContextRequest.getPathInContext(), servletApiRequest); + RoleInfo roleInfo = prepareConstraintInfo(servletContextRequest.getDecodedPathInContext(), servletApiRequest); // Check data constraints - if (!checkUserDataPermissions(servletContextRequest.getPathInContext(), servletContextRequest, response, callback, roleInfo)) + if (!checkUserDataPermissions(servletContextRequest.getDecodedPathInContext(), servletContextRequest, response, callback, roleInfo)) return true; // is Auth mandatory? diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/FormAuthenticator.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/FormAuthenticator.java index b085357ac8e..fa2abeed389 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/FormAuthenticator.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/FormAuthenticator.java @@ -225,7 +225,7 @@ public class FormAuthenticator extends LoginAuthenticator ServletContextRequest servletContextRequest = Request.as(req, ServletContextRequest.class); ServletApiRequest servletApiRequest = servletContextRequest.getServletApiRequest(); - String pathInContext = servletContextRequest.getPathInContext(); + String pathInContext = servletContextRequest.getDecodedPathInContext(); boolean jSecurityCheck = isJSecurityCheck(pathInContext); mandatory |= jSecurityCheck; if (!mandatory) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContextTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContextTest.java index 5662f411287..57a2ce63455 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContextTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContextTest.java @@ -70,11 +70,15 @@ public class AsyncContextTest _contextHandler.addServlet(new ServletHolder(new TestServlet()), "/path with spaces/servletPath"); _contextHandler.addServlet(new ServletHolder(new TestServlet2()), "/servletPath2"); - ServletHolder testHolder = new ServletHolder(new TestServlet()); - testHolder.setInitParameter("dispatchPath", "/test2/something%2felse"); - _contextHandler.addServlet(testHolder, "/test/*"); + ServletHolder encodedTestHolder = new ServletHolder(new TestServlet()); + encodedTestHolder.setInitParameter("dispatchPath", "/test2/something%25else"); + _contextHandler.addServlet(encodedTestHolder, "/encoded/*"); _contextHandler.addServlet(new ServletHolder(new TestServlet2()), "/test2/*"); + ServletHolder ambiguousTestHolder = new ServletHolder(new TestServlet()); + ambiguousTestHolder.setInitParameter("dispatchPath", "/test2/something%2Felse"); + _contextHandler.addServlet(ambiguousTestHolder, "/ambiguous/*"); + _contextHandler.addServlet(new ServletHolder(new SelfDispatchingServlet()), "/self/*"); _contextHandler.addServlet(new ServletHolder(new TestStartThrowServlet()), "/startthrow/*"); @@ -221,11 +225,52 @@ public class AsyncContextTest @Test public void testDispatchAsyncContextEncodedUrl() throws Exception { - String request = "GET /ctx/test/hello%20there?dispatch=true HTTP/1.1\r\n" + - "Host: localhost\r\n" + - "Content-Type: application/x-www-form-urlencoded\r\n" + - "Connection: close\r\n" + - "\r\n"; + String request = """ + GET /ctx/encoded/hello%20there?dispatch=true HTTP/1.1\r + Host: localhost\r + Content-Type: application/x-www-form-urlencoded\r + Connection: close\r + \r + """; + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request)); + assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_OK)); + + String responseBody = response.getContent(); + + // initial values + assertThat("servlet gets right path", responseBody, containsString("doGet:getServletPath:/test2")); + assertThat("request uri has correct encoding", responseBody, containsString("doGet:getRequestURI:/ctx/test2/something%25else")); + assertThat("request url has correct encoding", responseBody, containsString("doGet:getRequestURL:http://localhost/ctx/test2/something%25else")); + assertThat("path info has correct encoding", responseBody, containsString("doGet:getPathInfo:/something%else")); + + // async values + assertThat("async servlet gets right path", responseBody, containsString("doGet:async:getServletPath:/test2")); + assertThat("async request uri has correct encoding", responseBody, containsString("doGet:async:getRequestURI:/ctx/test2/something%25else")); + assertThat("async request url has correct encoding", responseBody, containsString("doGet:async:getRequestURL:http://localhost/ctx/test2/something%25else")); + assertThat("async path info has correct encoding", responseBody, containsString("doGet:async:getPathInfo:/something%else")); + + // async run attributes + assertThat("async run attr servlet path is original", responseBody, containsString("async:run:attr:servletPath:/encoded")); + 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/encoded/hello%20there")); + assertThat("http servlet mapping matchValue is correct", responseBody, containsString("async:run:attr:mapping:matchValue:encoded")); + assertThat("http servlet mapping pattern is correct", responseBody, containsString("async:run:attr:mapping:pattern:/encoded/*")); + 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:PATH")); + } + + @Test + public void testDispatchAsyncAmbiguousUrl() throws Exception + { + String request = """ + GET /ctx/ambiguous/hello%20there?dispatch=true HTTP/1.1\r + Host: localhost\r + Content-Type: application/x-www-form-urlencoded\r + Connection: close\r + \r + """; HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request)); assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_OK)); @@ -235,22 +280,22 @@ public class AsyncContextTest assertThat("servlet gets right path", responseBody, containsString("doGet:getServletPath:/test2")); assertThat("request uri has correct encoding", responseBody, containsString("doGet:getRequestURI:/ctx/test2/something%2Felse")); assertThat("request url has correct encoding", responseBody, containsString("doGet:getRequestURL:http://localhost/ctx/test2/something%2Felse")); - assertThat("path info has correct encoding", responseBody, containsString("doGet:getPathInfo:/something%2Felse")); + assertThat("path info has correct encoding", responseBody, containsString("doGet:getPathInfo:/something/else")); // async values assertThat("async servlet gets right path", responseBody, containsString("doGet:async:getServletPath:/test2")); assertThat("async request uri has correct encoding", responseBody, containsString("doGet:async:getRequestURI:/ctx/test2/something%2Felse")); assertThat("async request url has correct encoding", responseBody, containsString("doGet:async:getRequestURL:http://localhost/ctx/test2/something%2Felse")); - assertThat("async path info has correct encoding", responseBody, containsString("doGet:async:getPathInfo:/something%2Felse")); + assertThat("async path info has correct encoding", responseBody, containsString("doGet:async:getPathInfo:/something/else")); // async run attributes - assertThat("async run attr servlet path is original", responseBody, containsString("async:run:attr:servletPath:/test")); + assertThat("async run attr servlet path is original", responseBody, containsString("async:run:attr:servletPath:/ambiguous")); 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")); - assertThat("http servlet mapping matchValue is correct", responseBody, containsString("async:run:attr:mapping:matchValue:test")); - assertThat("http servlet mapping pattern is correct", responseBody, containsString("async:run:attr:mapping:pattern:/test/*")); + assertThat("async run request uri has correct encoding", responseBody, containsString("async:run:attr:requestURI:/ctx/ambiguous/hello%20there")); + assertThat("http servlet mapping matchValue is correct", responseBody, containsString("async:run:attr:mapping:matchValue:ambiguous")); + assertThat("http servlet mapping pattern is correct", responseBody, containsString("async:run:attr:mapping:pattern:/ambiguous/*")); 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:PATH")); } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java index ffac9d8e166..542e4ae1686 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java @@ -48,6 +48,7 @@ import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.http.content.ResourceHttpContentFactory; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.AllowedResourceAliasChecker; @@ -207,6 +208,8 @@ public class DefaultServletTest @Test public void testGetPercent2F() throws Exception { + connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); + Path file = docRoot.resolve("file.txt"); Files.writeString(file, "How now brown cow", UTF_8); @@ -256,6 +259,7 @@ public class DefaultServletTest assertThat(response.toString(), response.getContent(), is("In a while")); // Attempt access of content in sub-dir of context, using "%2F" instead of "/", should be a 404 + // as neither getServletPath and getPathInfo are used and thus they don't throw. rawResponse = connector.getResponse(""" GET /context/dirFoo%2Fother.txt HTTP/1.1\r Host: local\r diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DispatcherTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DispatcherTest.java index 7792cc3f0ac..50af4a83705 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DispatcherTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DispatcherTest.java @@ -212,12 +212,12 @@ public class DispatcherTest String expected = """ HTTP/1.1 200 OK\r Content-Type: text/plain\r - Content-Length: 56\r + Content-Length: 54\r Connection: close\r \r /context\r /EchoURI\r - /x%20x\r + /x x\r /context/EchoURI/x%20x;a=1\r """; assertEquals(expected, responses); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/EncodedURITest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/EncodedURITest.java index 0df7bf6dd15..f9ff2014f4b 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/EncodedURITest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/EncodedURITest.java @@ -27,6 +27,7 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; @@ -148,15 +149,24 @@ public class EncodedURITest ServletContextHandler context2 = new ServletContextHandler(); context2.setContextPath("/context_path".replace("_", separator)); _contextCollection.addHandler(context2); - context2.addServlet(TestServlet.class, "/test_servlet/*".replace("_", separator)); + context2.addServlet(TestServlet.class, URIUtil.decodePath("/test_servlet/*".replace("_", separator))); + _connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); _server.start(); String response = _connector.getResponse("GET /context_path/test_servlet/path_info HTTP/1.0\n\n".replace("_", separator)); assertThat(response, startsWith("HTTP/1.1 200 ")); assertThat(response, Matchers.containsString("requestURI=/context_path/test_servlet/path_info".replace("_", separator))); assertThat(response, Matchers.containsString("contextPath=/context_path".replace("_", separator))); - assertThat(response, Matchers.containsString("servletPath=/test_servlet".replace("_", separator))); - assertThat(response, Matchers.containsString("pathInfo=/path_info".replace("_", separator))); + if ("%2F".equals(separator)) + { + assertThat(response, Matchers.containsString("servletPath=org.eclipse.jetty.http.HttpException$IllegalArgumentException: 400: Ambiguous URI encoding")); + assertThat(response, Matchers.containsString("pathInfo=org.eclipse.jetty.http.HttpException$IllegalArgumentException: 400: Ambiguous URI encoding")); + } + else + { + assertThat(response, Matchers.containsString("servletPath=/test_servlet".replace("_", "?"))); + assertThat(response, Matchers.containsString("pathInfo=/path_info".replace("_", "?"))); + } } public static class TestServlet extends HttpServlet @@ -167,8 +177,22 @@ public class EncodedURITest response.setContentType("text/plain"); response.getWriter().println("requestURI=" + request.getRequestURI()); response.getWriter().println("contextPath=" + request.getContextPath()); - response.getWriter().println("servletPath=" + request.getServletPath()); - response.getWriter().println("pathInfo=" + request.getPathInfo()); + try + { + response.getWriter().println("servletPath=" + request.getServletPath()); + } + catch (Throwable e) + { + response.getWriter().println("servletPath=" + e); + } + try + { + response.getWriter().println("pathInfo=" + request.getPathInfo()); + } + catch (Throwable e) + { + response.getWriter().println("pathInfo=" + e); + } } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java index ad06108e55a..f121bb06a50 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java @@ -59,6 +59,7 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import static java.nio.charset.StandardCharsets.UTF_8; @@ -173,6 +174,7 @@ public class MultiPartServletTest } @Test + @Tag("Flaky") public void testManyParts() throws Exception { start(new HttpServlet() diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java index 75682f0e7c7..8cb3308e4e1 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java @@ -13,6 +13,8 @@ package org.eclipse.jetty.ee10.servlet; +import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import jakarta.servlet.http.HttpServlet; @@ -30,6 +32,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; public class RequestTest @@ -124,27 +128,70 @@ public class RequestTest } @Test - public void testSafeURI() throws Exception + public void testAmbiguousURI() throws Exception { + AtomicInteger count = new AtomicInteger(); startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse resp) + protected void service(HttpServletRequest request, HttpServletResponse resp) throws IOException { + count.incrementAndGet(); + String requestURI = request.getRequestURI(); + String servletPath; + String pathInfo; + try + { + servletPath = request.getServletPath(); + } + catch (IllegalArgumentException iae) + { + servletPath = iae.toString(); + } + try + { + pathInfo = request.getPathInfo(); + } + catch (IllegalArgumentException iae) + { + pathInfo = iae.toString(); + } + + resp.getOutputStream().println("requestURI=%s servletPath=%s pathInfo=%s".formatted(requestURI, servletPath, pathInfo)); } }); - _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 - """); + _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + String rawRequest = """ + GET /test/foo%2fbar HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """; + String rawResponse = _connector.getResponse(rawRequest); HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat(response.getStatus(), is(HttpStatus.BAD_REQUEST_400)); + assertThat(count.get(), equalTo(0)); + + _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); + rawResponse = _connector.getResponse(rawRequest); + + response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContent(), containsString("requestURI=/test/foo%2fbar")); + assertThat(response.getContent(), containsString("servletPath=org.eclipse.jetty.http.HttpException$IllegalArgumentException: 400: Ambiguous URI encoding")); + assertThat(response.getContent(), containsString("pathInfo=org.eclipse.jetty.http.HttpException$IllegalArgumentException: 400: Ambiguous URI encoding")); + assertThat(count.get(), equalTo(1)); + + _server.getContainedBeans(ServletHandler.class).iterator().next().setDecodeAmbiguousURIs(true); + rawResponse = _connector.getResponse(rawRequest); + + response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContent(), containsString("requestURI=/test/foo%2fbar")); + assertThat(response.getContent(), containsString("servletPath= ")); + assertThat(response.getContent(), containsString("pathInfo=/test/foo/bar")); + assertThat(count.get(), equalTo(2)); } @Test @@ -165,14 +212,14 @@ public class RequestTest String rawResponse = _connector.getResponse( """ - GET /test/path%20info/foo%2fbar HTTP/1.1\r + GET /test/path%20info/foo%2cbar 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")); + assertThat("request.getRequestURI", resultRequestURI.get(), is("/test/path%20info/foo%2cbar")); + assertThat("request.getPathInfo", resultPathInfo.get(), is("/test/path info/foo,bar")); } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestURITest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestURITest.java index 01aa629cca3..c56ac55ad21 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestURITest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestURITest.java @@ -116,7 +116,7 @@ public class RequestURITest ServerConnector connector = new ServerConnector(server); connector.setPort(0); server.addConnector(connector); - connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); ServletContextHandler context = new ServletContextHandler(); context.setContextPath("/"); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java index 61b34dc6045..cea87ec3430 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java @@ -2406,9 +2406,9 @@ public class ServletContextHandlerTest ServletContextHandler context = new ServletContextHandler() { @Override - protected ServletContextRequest newServletContextRequest(ServletChannel servletChannel, Request request, Response response, String pathInContext, MatchedResource matchedResource) + protected ServletContextRequest newServletContextRequest(ServletChannel servletChannel, Request request, Response response, String decodedPathInContext, MatchedResource matchedResource) { - return new ServletContextRequest(getContext().getServletContext(), servletChannel, request, response, pathInContext, matchedResource, getSessionHandler()) + return new ServletContextRequest(getContext().getServletContext(), servletChannel, request, response, decodedPathInContext, matchedResource, getSessionHandler()) { @Override protected ServletContextResponse newServletContextResponse(Response response) diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java index 32f23984e7f..8b52c4f7ea9 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java +++ b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java @@ -65,6 +65,7 @@ import org.slf4j.LoggerFactory; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -329,7 +330,7 @@ public class WebAppContextTest LocalConnector connector = new LocalConnector(server); server.addConnector(connector); - connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); server.start(); @@ -374,7 +375,8 @@ public class WebAppContextTest server.start(); - assertThat(HttpTester.parseResponse(connector.getResponse("GET " + target + " HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET " + target + " HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), + either(is(HttpStatus.NOT_FOUND_404)).or(is(HttpStatus.BAD_REQUEST_400))); } @ParameterizedTest diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppDefaultServletTest.java b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppDefaultServletTest.java index b780b2d8a74..20e395ef085 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppDefaultServletTest.java +++ b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppDefaultServletTest.java @@ -48,7 +48,7 @@ public class WebAppDefaultServletTest { server = new Server(); connector = new LocalConnector(server); - connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); server.addConnector(connector); Path directoryPath = workDir.getEmptyPathDir(); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/RequestURITest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/RequestURITest.java index becc7b0aa6f..2594ec64e84 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/RequestURITest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/RequestURITest.java @@ -116,7 +116,7 @@ public class RequestURITest ServerConnector connector = new ServerConnector(server); connector.setPort(0); server.addConnector(connector); - connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); ServletContextHandler context = new ServletContextHandler(); context.setContextPath("/"); diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppContextTest.java b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppContextTest.java index 85db78c44e7..d22faf3aeec 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppContextTest.java +++ b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppContextTest.java @@ -325,7 +325,7 @@ public class WebAppContextTest LocalConnector connector = new LocalConnector(server); server.addConnector(connector); - connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); server.start(); diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppDefaultServletTest.java b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppDefaultServletTest.java index 8a87c8ee0d5..0649a5b9227 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppDefaultServletTest.java +++ b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppDefaultServletTest.java @@ -50,7 +50,7 @@ public class WebAppDefaultServletTest assertThat(FileSystemPool.INSTANCE.mounts(), empty()); server = new Server(); connector = new LocalConnector(server); - connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE); server.addConnector(connector); Path directoryPath = workDir.getEmptyPathDir();