diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java index fd0dece4b80a..319b4afb87c0 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.http; +import java.util.EnumMap; import java.util.EnumSet; import java.util.HashMap; import java.util.Map; @@ -214,4 +215,46 @@ public EnumSet sections() { return _sections; } + + private static final EnumMap __uriViolations = new EnumMap<>(HttpURI.Violation.class); + static + { + // create a map from Violation to compliance in a loop, so that any new violations added are detected with ISE + for (HttpURI.Violation violation : HttpURI.Violation.values()) + { + switch (violation) + { + case SEPARATOR: + __uriViolations.put(violation, HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS); + break; + case SEGMENT: + __uriViolations.put(violation, HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS); + break; + case PARAM: + __uriViolations.put(violation, HttpComplianceSection.NO_AMBIGUOUS_PATH_PARAMETERS); + break; + case ENCODING: + __uriViolations.put(violation, HttpComplianceSection.NO_AMBIGUOUS_PATH_ENCODING); + break; + case EMPTY: + __uriViolations.put(violation, HttpComplianceSection.NO_AMBIGUOUS_EMPTY_SEGMENT); + break; + case UTF16: + __uriViolations.put(violation, HttpComplianceSection.NO_UTF16_ENCODINGS); + break; + default: + throw new IllegalStateException(); + } + } + } + + public static String checkUriCompliance(HttpCompliance compliance, HttpURI uri) + { + for (HttpURI.Violation violation : HttpURI.Violation.values()) + { + if (uri.hasViolation(violation) && (compliance == null || compliance.sections().contains(__uriViolations.get(violation)))) + return violation.getMessage(); + } + return null; + } } 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 4087291dacbb..5dc6db1302aa 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 @@ -36,22 +36,45 @@ /** * Http URI. * Parse an HTTP URI from a string or byte array. Given a URI - * http://user@host:port/path/info;param?query#fragment - * this class will split it into the following undecoded optional elements: */ +@Deprecated public class PutFilter implements Filter { public static final String __PUT = "PUT"; @@ -85,7 +86,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"); @@ -95,7 +97,7 @@ public void init(FilterConfig config) throws ServletException } else { - File base = new File(_context.getRealPath("/")); + File base = new File(realPath); _baseURI = base.toURI().toString(); } @@ -289,7 +291,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 19ed06e0db13..3c6f7a53d343 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 @@ -475,7 +475,8 @@ public static String decodePath(String path, int offset, int length) char u = path.charAt(i + 1); if (u == 'u') { - // In Jetty-10 UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. // This is wrong. This is a codepoint not a char + // In Jetty-10 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; } @@ -780,143 +781,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; @@ -925,8 +911,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) @@ -941,9 +927,6 @@ public static String canonicalEncodedPath(String path) slash = false; break; - case '?': - return path; - default: slash = false; } @@ -951,56 +934,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) @@ -1011,20 +969,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 f548ffa56e7c..ad73b863beb3 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 @@ -217,7 +217,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/FileResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java index 9ccfe9682a39..40ead6f17972 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java @@ -269,7 +269,6 @@ public Resource addPath(String path) throws IOException { assertValidPath(path); - path = org.eclipse.jetty.util.URIUtil.canonicalPath(path); if (path == null) throw new MalformedURLException(); 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 af377b19ebe4..22f936a62cfd 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 @@ -64,7 +64,7 @@ public class PathResource extends Resource private final URI uri; private final boolean belongsToDefaultFileSystem; - private final Path checkAliasPath() + private Path checkAliasPath() { Path abs = path; @@ -76,7 +76,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 @@ -93,9 +92,11 @@ private final Path checkAliasPath() } if (!abs.isAbsolute()) - { abs = path.toAbsolutePath(); - } + + Path normal = path.normalize(); + if (!abs.equals(normal)) + return normal; try { @@ -241,8 +242,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(); @@ -262,7 +262,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 += "/"; @@ -363,12 +363,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 45744360415f..dc443becffb3 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 @@ -555,7 +555,6 @@ public String getListHTML(String base, boolean parent) throws IOException */ 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/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index 089e088abd4d..6612e931307f 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -300,7 +300,8 @@ public Resource addPath(String path) throws IOException { return new ResourceCollection(resources.toArray(new Resource[0])); } - return null; + + throw new MalformedURLException(); } @Override 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 ffd9c1c9702e..054f4d0d5ab6 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 @@ -272,9 +272,7 @@ public Resource addPath(String path) throws IOException { if (path == null) - return null; - - path = URIUtil.canonicalPath(path); + 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 82f2771a42cb..c40ff6a83c7e 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 @@ -27,10 +27,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 = { @@ -88,6 +89,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}, @@ -100,10 +102,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 @@ -125,7 +125,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"}, @@ -144,9 +143,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 1fac71622a98..4cb8c79f7d3c 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 @@ -156,6 +156,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 67b9cfac21f9..8bf6fe0aad1f 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 @@ -379,7 +379,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) @@ -392,16 +392,16 @@ public Resource getResource(String uriInContext) throws MalformedURLException uriInContext = getResourceAlias(uriInContext); } - catch (IOException e) + catch (MalformedURLException e) { LOG.ignore(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; } @@ -1556,6 +1556,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 48e9b3fa3147..86c65880d697 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 @@ -54,12 +54,14 @@ import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.Resource; +import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; 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 static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; @@ -249,8 +251,50 @@ public void testIsProtected() assertFalse(context.isProtectedTarget("/something-else/web-inf")); } - @Test - public void testProtectedTarget() throws Exception + @ParameterizedTest + @ValueSource(strings = { + "/test.xml", + "/%2e/%2e/test.xml", + "/foo/%2e%2e/test.xml" + }) + public void testProtectedTargetSuccess(String path) throws Exception + { + Server server = newServer(); + server.getConnectors()[0].getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.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); + + LocalConnector connector = new LocalConnector(server); + server.addConnector(connector); + + server.start(); + assertThat(HttpTester.parseResponse(connector.getResponse("GET " + path + " HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); + } + + @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(); server.getConnectors()[0].getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.LEGACY); @@ -269,27 +313,9 @@ public void testProtectedTarget() throws Exception server.addConnector(connector); server.start(); - assertThat(HttpTester.parseResponse(connector.getResponse("GET /test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); - assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); - assertThat(HttpTester.parseResponse(connector.getResponse("GET /%u002e/%u002e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); - assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); - assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%u002e%u002e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); - - assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF 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/ 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/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/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 /%2e/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 /%u002e/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 /%2e/%2e/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 /%u002e/%u002e/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 /foo/%2e%2e/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 /foo/%u002e%u002e/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 /%2E/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 /%u002E/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/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)); - assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF%u002ftest.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 " + 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 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 2c0ac83aef8b..3ed1f6150817 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 @@ -27,8 +27,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; @@ -61,13 +65,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(); } @@ -103,6 +115,31 @@ 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(new HttpConfiguration.Customizer() + { + @Override + public void customize(Connector connector, HttpConfiguration channelConfig, Request request) + { + HttpURI uri = request.getHttpURI(); + if (uri.getPath().contains("_00_")) + { + request.setHttpURI(new HttpURI( + uri.getScheme(), + uri.getHost(), + uri.getPort(), + uri.getPath().replace("_00_", "\000"), + uri.getParam(), + uri.getQuery(), + uri.getFragment() + )); + } + } + }); + }); + server.start(); int port = ((NetworkConnector)server.getConnectors()[0]).getLocalPort();