From 92a74d6734881cf067f54e71bf5188dc7fdbbd52 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 28 Jun 2021 15:52:14 +1000 Subject: [PATCH] Issue #6473 - canonicalPath refactor & fix alias check in PathResource Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/http/HttpURI.java | 154 +++++++++-- .../org/eclipse/jetty/http/UriCompliance.java | 58 +++- .../org/eclipse/jetty/http/HttpURITest.java | 132 +++++----- .../jetty/rewrite/handler/RedirectUtil.java | 4 +- .../rewrite/handler/ValidUrlRuleTest.java | 17 +- .../org/eclipse/jetty/server/Dispatcher.java | 34 ++- .../org/eclipse/jetty/server/Request.java | 33 +-- .../org/eclipse/jetty/server/Response.java | 4 +- .../jetty/server/handler/ContextHandler.java | 21 +- .../jetty/server/handler/ResourceHandler.java | 1 - .../jetty/server/HttpConnectionTest.java | 12 +- .../org/eclipse/jetty/server/RequestTest.java | 9 - .../ContextHandlerGetResourceTest.java | 34 ++- .../eclipse/jetty/servlet/DefaultServlet.java | 4 +- .../eclipse/jetty/servlet/RequestURITest.java | 4 +- .../servlets/DataRateLimitedServlet.java | 7 +- .../org/eclipse/jetty/servlets/PutFilter.java | 8 +- .../java/org/eclipse/jetty/util/URIUtil.java | 248 +++++++++--------- .../eclipse/jetty/util/Utf8Appendable.java | 1 - .../jetty/util/resource/PathResource.java | 20 +- .../eclipse/jetty/util/resource/Resource.java | 1 - .../jetty/util/resource/URLResource.java | 5 +- .../jetty/util/URIUtilCanonicalPathTest.java | 27 +- .../jetty/util/Utf8AppendableTest.java | 29 ++ .../eclipse/jetty/webapp/WebAppContext.java | 15 +- .../jetty/webapp/WebAppContextTest.java | 40 +++ .../jsp/JspAndDefaultWithoutAliasesTest.java | 37 ++- 27 files changed, 608 insertions(+), 351 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index 42f829068e9c..596f1acc1424 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -15,7 +15,10 @@ import java.net.URI; import java.net.URISyntaxException; +import java.util.Collection; +import java.util.Collections; import java.util.EnumSet; +import java.util.Set; import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.Index; @@ -31,54 +34,92 @@ * via the static methods such as {@link #build()} and {@link #from(String)}. * * A URI such as - * http://user@host:port/path;ignored/info;param?query#ignored - * is split into the following undecoded elements: */ +@Deprecated public class PutFilter implements Filter { public static final String __PUT = "PUT"; @@ -80,7 +81,8 @@ public void init(FilterConfig config) throws ServletException _tmpdir = (File)_context.getAttribute("javax.servlet.context.tempdir"); - if (_context.getRealPath("/") == null) + String realPath = _context.getRealPath("/"); + if (realPath == null) throw new UnavailableException("Packed war"); String b = config.getInitParameter("baseURI"); @@ -90,7 +92,7 @@ public void init(FilterConfig config) throws ServletException } else { - File base = new File(_context.getRealPath("/")); + File base = new File(realPath); _baseURI = base.toURI().toString(); } @@ -284,7 +286,7 @@ public void handleDelete(HttpServletRequest request, HttpServletResponse respons public void handleMove(HttpServletRequest request, HttpServletResponse response, String pathInContext, File file) throws ServletException, IOException, URISyntaxException { - String newPath = URIUtil.canonicalEncodedPath(request.getHeader("new-uri")); + String newPath = URIUtil.canonicalURI(request.getHeader("new-uri")); if (newPath == null) { response.sendError(HttpServletResponse.SC_BAD_REQUEST); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index f18e685f2e8b..b91b71c30081 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -471,6 +471,7 @@ public static String decodePath(String path, int offset, int length) if (u == 'u') { // UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. + // This is wrong. This is a codepoint not a char builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16))); i += 5; } @@ -776,143 +777,128 @@ public static String parentPath(String p) } /** - * Convert an encoded path to a canonical form. + * Convert a partial URI to a canonical form. *

- * All instances of "." and ".." are factored out. + * All segments of "." and ".." are factored out. * Null is returned if the path tries to .. above its root. *

* - * @param path the path to convert, decoded, with path separators '/' and no queries. + * @param uri the encoded URI from the path onwards, which may contain query strings and/or fragments * @return the canonical path, or null if path traversal above root. + * @see #canonicalPath(String) + * @see #canonicalURI(String) */ - public static String canonicalPath(String path) + public static String canonicalURI(String uri) { - // See https://tools.ietf.org/html/rfc3986#section-5.2.4 - - if (path == null || path.isEmpty()) - return path; + if (uri == null || uri.isEmpty()) + return uri; - int end = path.length(); + boolean slash = true; + int end = uri.length(); int i = 0; - int dots = 0; + // Initially just loop looking if we may need to normalize loop: while (i < end) { - char c = path.charAt(i); + char c = uri.charAt(i); switch (c) { case '/': - dots = 0; + slash = true; break; case '.': - if (dots == 0) - { - dots = 1; + if (slash) break loop; - } - dots = -1; + slash = false; break; + case '?': + case '#': + // Nothing to normalize so return original path + return uri; + default: - dots = -1; + slash = false; } i++; } + // Nothing to normalize so return original path if (i == end) - return path; + return uri; - StringBuilder canonical = new StringBuilder(path.length()); - canonical.append(path, 0, i); + // We probably need to normalize, so copy to path so far into builder + StringBuilder canonical = new StringBuilder(uri.length()); + canonical.append(uri, 0, i); + // Loop looking for single and double dot segments + int dots = 1; i++; - while (i <= end) + loop : while (i < end) { - char c = i < end ? path.charAt(i) : '\0'; + char c = uri.charAt(i); switch (c) { - case '\0': - if (dots == 2) - { - if (canonical.length() < 2) - return null; - canonical.setLength(canonical.length() - 1); - canonical.setLength(canonical.lastIndexOf("/") + 1); - } - break; - case '/': - switch (dots) - { - case 1: - break; - - case 2: - if (canonical.length() < 2) - return null; - canonical.setLength(canonical.length() - 1); - canonical.setLength(canonical.lastIndexOf("/") + 1); - break; - - default: - canonical.append(c); - } + if (doDotsSlash(canonical, dots)) + return null; + slash = true; dots = 0; break; + case '?': + case '#': + // finish normalization at a query + break loop; + case '.': - switch (dots) - { - case 0: - dots = 1; - break; - case 1: - dots = 2; - break; - case 2: - canonical.append("..."); - dots = -1; - break; - default: - canonical.append('.'); - } + // Count dots only if they are leading in the segment + if (dots > 0) + dots++; + else if (slash) + dots = 1; + else + canonical.append('.'); + slash = false; break; default: - switch (dots) - { - case 1: - canonical.append('.'); - break; - case 2: - canonical.append(".."); - break; - default: - } + // Add leading dots to the path + while (dots-- > 0) + canonical.append('.'); canonical.append(c); - dots = -1; + dots = 0; + slash = false; } - i++; } + + // process any remaining dots + if (doDots(canonical, dots)) + return null; + + // append any query + if (i < end) + canonical.append(uri, i, end); + return canonical.toString(); } /** - * Convert a path to a cananonical form. - *

- * All instances of "." and ".." are factored out. - *

+ * Convert a decoded URI path to a canonical form. *

+ * All segments of "." and ".." are factored out. * Null is returned if the path tries to .. above its root. *

* - * @param path the path to convert (expects URI/URL form, encoded, and with path separators '/') + * @param path the decoded URI path to convert. Any special characters (e.g. '?', "#") are assumed to be part of + * the path segments. * @return the canonical path, or null if path traversal above root. + * @see #canonicalURI(String) */ - public static String canonicalEncodedPath(String path) + public static String canonicalPath(String path) { if (path == null || path.isEmpty()) return path; @@ -921,8 +907,8 @@ public static String canonicalEncodedPath(String path) int end = path.length(); int i = 0; - loop: - while (i < end) + // Initially just loop looking if we may need to normalize + loop: while (i < end) { char c = path.charAt(i); switch (c) @@ -937,9 +923,6 @@ public static String canonicalEncodedPath(String path) slash = false; break; - case '?': - return path; - default: slash = false; } @@ -947,56 +930,31 @@ public static String canonicalEncodedPath(String path) i++; } + // Nothing to normalize so return original path if (i == end) return path; + // We probably need to normalize, so copy to path so far into builder StringBuilder canonical = new StringBuilder(path.length()); canonical.append(path, 0, i); + // Loop looking for single and double dot segments int dots = 1; i++; - while (i <= end) + while (i < end) { - char c = i < end ? path.charAt(i) : '\0'; + char c = path.charAt(i); switch (c) { - case '\0': case '/': - case '?': - switch (dots) - { - case 0: - if (c != '\0') - canonical.append(c); - break; - - case 1: - if (c == '?') - canonical.append(c); - break; - - case 2: - if (canonical.length() < 2) - return null; - canonical.setLength(canonical.length() - 1); - canonical.setLength(canonical.lastIndexOf("/") + 1); - if (c == '?') - canonical.append(c); - break; - default: - while (dots-- > 0) - { - canonical.append('.'); - } - if (c != '\0') - canonical.append(c); - } - + if (doDotsSlash(canonical, dots)) + return null; slash = true; dots = 0; break; case '.': + // Count dots only if they are leading in the segment if (dots > 0) dots++; else if (slash) @@ -1007,20 +965,66 @@ else if (slash) break; default: + // Add leading dots to the path while (dots-- > 0) - { canonical.append('.'); - } canonical.append(c); dots = 0; slash = false; } - i++; } + + // process any remaining dots + if (doDots(canonical, dots)) + return null; + return canonical.toString(); } + private static boolean doDots(StringBuilder canonical, int dots) + { + switch (dots) + { + case 0: + case 1: + break; + case 2: + if (canonical.length() < 2) + return true; + canonical.setLength(canonical.length() - 1); + canonical.setLength(canonical.lastIndexOf("/") + 1); + break; + default: + while (dots-- > 0) + canonical.append('.'); + } + return false; + } + + private static boolean doDotsSlash(StringBuilder canonical, int dots) + { + switch (dots) + { + case 0: + canonical.append('/'); + break; + case 1: + break; + case 2: + if (canonical.length() < 2) + return true; + canonical.setLength(canonical.length() - 1); + canonical.setLength(canonical.lastIndexOf("/") + 1); + break; + default: + while (dots-- > 0) + canonical.append('.'); + canonical.append('/'); + } + return false; + } + /** * Convert a path to a compact form. * All instances of "//" and "///" etc. are factored out to single "/" diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8Appendable.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8Appendable.java index f5e70fc7f352..ea7305ff86e2 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8Appendable.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8Appendable.java @@ -213,7 +213,6 @@ public boolean append(byte[] b, int offset, int length, int maxChars) protected void appendByte(byte b) throws IOException { - if (b > 0 && _state == UTF8_ACCEPT) { _appendable.append((char)(b & 0xFF)); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 244ae2ad6338..755c4b5f3299 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -56,7 +56,7 @@ public class PathResource extends Resource private final URI uri; private final boolean belongsToDefaultFileSystem; - private final Path checkAliasPath() + private Path checkAliasPath() { Path abs = path; @@ -68,7 +68,6 @@ private final Path checkAliasPath() * we will just use the original URI to construct the * alias reference Path. */ - if (!URIUtil.equalsIgnoreEncodings(uri, path.toUri())) { try @@ -85,9 +84,11 @@ private final Path checkAliasPath() } if (!abs.isAbsolute()) - { abs = path.toAbsolutePath(); - } + + Path normal = path.normalize(); + if (!abs.equals(normal)) + return normal; try { @@ -233,8 +234,7 @@ public PathResource(Path path) LOG.debug("Unable to get real/canonical path for {}", path, e); } - // cleanup any lingering relative path nonsense (like "/./" and "/../") - this.path = absPath.normalize(); + this.path = absPath; assertValidPath(path); this.uri = this.path.toUri(); @@ -254,7 +254,7 @@ private PathResource(PathResource parent, String childPath) // Calculate the URI and the path separately, so that any aliasing done by // FileSystem.getPath(path,childPath) is visible as a difference to the URI // obtained via URIUtil.addDecodedPath(uri,childPath) - + // The checkAliasPath normalization checks will only work correctly if the getPath implementation here does not normalize. this.path = parent.path.getFileSystem().getPath(parent.path.toString(), childPath); if (isDirectory() && !childPath.endsWith("/")) childPath += "/"; @@ -350,12 +350,10 @@ public boolean isSame(Resource resource) @Override public Resource addPath(final String subpath) throws IOException { - String cpath = URIUtil.canonicalPath(subpath); - - if ((cpath == null) || (cpath.length() == 0)) + if ((subpath == null) || (subpath.length() == 0)) throw new MalformedURLException(subpath); - if ("/".equals(cpath)) + if ("/".equals(subpath)) return this; // subpaths are always under PathResource diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index a45421a2a2ac..03afc1fd2efc 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -477,7 +477,6 @@ public URI getAlias() */ public String getListHTML(String base, boolean parent, String query) throws IOException { - base = URIUtil.canonicalPath(base); if (base == null || !isDirectory()) return null; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java index 74a260c2ecda..b15d3e26f07b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java @@ -280,9 +280,8 @@ public String[] list() public Resource addPath(String path) throws IOException { - Objects.requireNonNull(path, "Path may not be null"); - - path = URIUtil.canonicalPath(path); + if (path == null) + throw new MalformedURLException("null path"); return newResource(URIUtil.addEncodedPaths(_url.toExternalForm(), URIUtil.encodePath(path)), _useCaches); } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java index 6a9c88d494ea..d8bac6a9ea5f 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java @@ -22,10 +22,11 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; public class URIUtilCanonicalPathTest { - public static Stream data() + public static Stream paths() { String[][] canonical = { @@ -83,6 +84,7 @@ public static Stream data() {"/foo/../bar//", "/bar//"}, {"/ctx/../bar/../ctx/all/index.txt", "/ctx/all/index.txt"}, {"/down/.././index.html", "/index.html"}, + {"/aaa/bbb/ccc/..", "/aaa/bbb/"}, // Path traversal up past root {"..", null}, @@ -95,10 +97,8 @@ public static Stream data() {"a/../..", null}, {"/foo/../../bar", null}, - // Query parameter specifics - {"/ctx/dir?/../index.html", "/ctx/index.html"}, - {"/get-files?file=/etc/passwd", "/get-files?file=/etc/passwd"}, - {"/get-files?file=../../../../../passwd", null}, + // Encoded ? + {"/ctx/dir%3f/../index.html", "/ctx/index.html"}, // Known windows shell quirks {"file.txt ", "file.txt "}, // with spaces @@ -120,7 +120,6 @@ public static Stream data() {"/%2e%2e/", "/%2e%2e/"}, // paths with parameters are not elided - // canonicalPath() is not responsible for decoding characters {"/foo/.;/bar", "/foo/.;/bar"}, {"/foo/..;/bar", "/foo/..;/bar"}, {"/foo/..;/..;/bar", "/foo/..;/..;/bar"}, @@ -139,9 +138,23 @@ public static Stream data() } @ParameterizedTest - @MethodSource("data") + @MethodSource("paths") public void testCanonicalPath(String input, String expectedResult) { + // Check canonicalPath assertThat(URIUtil.canonicalPath(input), is(expectedResult)); + + // Check canonicalURI + if (expectedResult == null) + assertThat(URIUtil.canonicalURI(input), nullValue()); + else + { + // mostly encodedURI will be the same + assertThat(URIUtil.canonicalURI(input), is(expectedResult)); + // but will terminate on fragments and queries + assertThat(URIUtil.canonicalURI(input + "?/foo/../bar/."), is(expectedResult + "?/foo/../bar/.")); + assertThat(URIUtil.canonicalURI(input + "#/foo/../bar/."), is(expectedResult + "#/foo/../bar/.")); + } } + } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/Utf8AppendableTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/Utf8AppendableTest.java index 384a2aaf6458..4786da17eb01 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/Utf8AppendableTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/Utf8AppendableTest.java @@ -151,6 +151,35 @@ public void testInvalidUTF8(Class impl) throws UnsupportedEncodi }); } + @ParameterizedTest + @MethodSource("implementations") + public void testInvalidZeroUTF8(Class impl) throws UnsupportedEncodingException + { + // From https://datatracker.ietf.org/doc/html/rfc3629#section-10 + assertThrows(Utf8Appendable.NotUtf8Exception.class, () -> + { + Utf8Appendable buffer = impl.getDeclaredConstructor().newInstance(); + buffer.append((byte)0xC0); + buffer.append((byte)0x80); + }); + } + + @ParameterizedTest + @MethodSource("implementations") + public void testInvalidAlternateDotEncodingUTF8(Class impl) throws UnsupportedEncodingException + { + // From https://datatracker.ietf.org/doc/html/rfc3629#section-10 + assertThrows(Utf8Appendable.NotUtf8Exception.class, () -> + { + Utf8Appendable buffer = impl.getDeclaredConstructor().newInstance(); + buffer.append((byte)0x2f); + buffer.append((byte)0xc0); + buffer.append((byte)0xae); + buffer.append((byte)0x2e); + buffer.append((byte)0x2f); + }); + } + @ParameterizedTest @MethodSource("implementations") public void testFastFail1(Class impl) throws Exception diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index ecd65c66fcc6..1a2af27da4c7 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -400,7 +400,7 @@ public Resource getResource(String uriInContext) throws MalformedURLException if (uriInContext == null || !uriInContext.startsWith(URIUtil.SLASH)) throw new MalformedURLException(uriInContext); - IOException ioe = null; + MalformedURLException mue = null; Resource resource = null; int loop = 0; while (uriInContext != null && loop++ < 100) @@ -413,16 +413,16 @@ public Resource getResource(String uriInContext) throws MalformedURLException uriInContext = getResourceAlias(uriInContext); } - catch (IOException e) + catch (MalformedURLException e) { LOG.trace("IGNORED", e); - if (ioe == null) - ioe = e; + if (mue == null) + mue = e; } } - if (ioe instanceof MalformedURLException) - throw (MalformedURLException)ioe; + if (mue != null) + throw mue; return resource; } @@ -1438,6 +1438,9 @@ public void checkListener(Class listener) throws Illega @Override public URL getResource(String path) throws MalformedURLException { + if (path == null) + return null; + Resource resource = WebAppContext.this.getResource(path); if (resource == null || !resource.exists()) return null; diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index 83f67fdc2d2d..765a21867813 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -53,6 +53,7 @@ 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; @@ -328,6 +329,45 @@ public void testProtectedTarget() throws Exception assertThat(HttpTester.parseResponse(connector.getResponse("GET //WEB-INF/test.xml 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 /WEB-INF%2ftest.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); } + + @ParameterizedTest + @ValueSource(strings = { + "/WEB-INF", + "/WEB-INF/", + "/WEB-INF/test.xml", + "/web-inf/test.xml", + "/%2e/WEB-INF/test.xml", + "/%2e/%2e/WEB-INF/test.xml", + "/foo/%2e%2e/WEB-INF/test.xml", + "/%2E/WEB-INF/test.xml", + "//WEB-INF/test.xml", + "/WEB-INF%2ftest.xml", + "/.%00/WEB-INF/test.xml", + "/WEB-INF%00/test.xml" + }) + public void testProtectedTargetFailure(String path) throws Exception + { + Server server = newServer(); + + LocalConnector connector = new LocalConnector(server); + server.addConnector(connector); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY); + + HandlerList handlers = new HandlerList(); + ContextHandlerCollection contexts = new ContextHandlerCollection(); + WebAppContext context = new WebAppContext(); + Path testWebapp = MavenTestingUtils.getProjectDirPath("src/test/webapp"); + context.setBaseResource(new PathResource(testWebapp)); + context.setContextPath("/"); + server.setHandler(handlers); + handlers.addHandler(contexts); + contexts.addHandler(context); + + server.start(); + + assertThat(HttpTester.parseResponse(connector.getResponse("GET " + path + " HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), + Matchers.anyOf(is(HttpStatus.NOT_FOUND_404), is(HttpStatus.BAD_REQUEST_400))); + } @Test public void testNullPath() throws Exception diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/jsp/JspAndDefaultWithoutAliasesTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/jsp/JspAndDefaultWithoutAliasesTest.java index 3cbf66f8ac37..f841c023ecc4 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/jsp/JspAndDefaultWithoutAliasesTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/jsp/JspAndDefaultWithoutAliasesTest.java @@ -22,8 +22,12 @@ import java.util.List; import java.util.stream.Stream; +import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.security.HashLoginService; +import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.NetworkConnector; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.DefaultServlet; import org.eclipse.jetty.servlet.ServletContextHandler; @@ -56,13 +60,21 @@ public static Stream aliases() data.add(Arguments.of("/dump.jsp")); data.add(Arguments.of("/dump.jsp/")); - data.add(Arguments.of("/dump.jsp%00")); - data.add(Arguments.of("/dump.jsp%00x")); - data.add(Arguments.of("/dump.jsp%00x/dump.jsp")); - data.add(Arguments.of("/dump.jsp%00/dump.jsp")); - data.add(Arguments.of("/dump.jsp%00/index.html")); - data.add(Arguments.of("/dump.jsp%00/")); - data.add(Arguments.of("/dump.jsp%00x/")); + data.add(Arguments.of("/dump.jsp%1e")); + data.add(Arguments.of("/dump.jsp%1ex")); + data.add(Arguments.of("/dump.jsp%1ex/dump.jsp")); + data.add(Arguments.of("/dump.jsp%1e/dump.jsp")); + data.add(Arguments.of("/dump.jsp%1e/index.html")); + data.add(Arguments.of("/dump.jsp%1e/")); + data.add(Arguments.of("/dump.jsp%1ex/")); + // The _00_ is later replaced with a real null character in a customizer + data.add(Arguments.of("/dump.jsp_00_")); + data.add(Arguments.of("/dump.jsp_00_")); + data.add(Arguments.of("/dump.jsp_00_/dump.jsp")); + data.add(Arguments.of("/dump.jsp_00_/dump.jsp")); + data.add(Arguments.of("/dump.jsp_00_/index.html")); + data.add(Arguments.of("/dump.jsp_00_/")); + data.add(Arguments.of("/dump.jsp_00_/")); return data.stream(); } @@ -98,6 +110,17 @@ public static void startServer() throws Exception // add context server.setHandler(context); + // Add customizer to convert "_00_" to a real null + server.getContainedBeans(HttpConfiguration.class).forEach(config -> + { + config.addCustomizer((connector, channelConfig, request) -> + { + HttpURI uri = request.getHttpURI(); + if (uri.getPath().contains("_00_")) + request.setHttpURI(HttpURI.build(uri, uri.getPath().replace("_00_", "\000"), uri.getParam(), uri.getQuery())); + }); + }); + server.start(); int port = ((NetworkConnector)server.getConnectors()[0]).getLocalPort();