* - {@link #getScheme()} - http:
* - {@link #getAuthority()} - //name@host:port
@@ -62,11 +62,13 @@
* Thus this class avoid and/or detects such ambiguities. Furthermore, by decoding characters and
* removing parameters before relative path normalization, ambiguous paths will be resolved in such
* a way to be non-standard-but-non-ambiguous to down stream interpretation of the decoded path string.
- * The violations are recorded and available by API such as {@link #hasViolation(Violation)} so that requests
+ * The violations are recorded and available by API such as {@link #hasAmbiguousSegment()} so that requests
* containing them may be rejected in case the non-standard-but-non-ambiguous interpretations
- * are not satisfactory for a given compliance configuration. Implementations that wish to
- * process ambiguous URI paths must configure the compliance modes to accept them and then perform
- * their own decoding of {@link #getPath()}.
+ * are not satisfactory for a given compliance configuration.
+ *
+ *
+ * Implementations that wish to process ambiguous URI paths must configure the compliance modes
+ * to accept them and then perform their own decoding of {@link #getPath()}.
*
*
* If there are multiple path parameters, only the last one is returned by {@link #getParam()}.
@@ -80,32 +82,32 @@ public interface HttpURI
enum Violation
{
/**
- * Ambiguous path segments e.g. /foo/%2E%2E/bar
+ * Ambiguous path segments e.g. {@code /foo/%2E%2E/bar}
*/
SEGMENT("Ambiguous path segments"),
/**
- * Ambiguous path separator within a URI segment e.g. /foo%2Fbar
+ * Ambiguous path separator within a URI segment e.g. {@code /foo%2Fbar}
*/
SEPARATOR("Ambiguous path separator"),
/**
- * Ambiguous path parameters within a URI segment e.g. /foo/..;/bar
+ * Ambiguous path parameters within a URI segment e.g. {@code /foo/..;/bar}
*/
PARAM("Ambiguous path parameters"),
/**
- * Ambiguous double encoding within a URI segment e.g. /%2557EB-INF
+ * Ambiguous double encoding within a URI segment e.g. {@code /%2557EB-INF}
*/
ENCODING("Ambiguous double encoding"),
/**
- * Ambiguous empty segments e.g. /foo//bar
+ * Ambiguous empty segments e.g. {@code /foo//bar}
*/
EMPTY("Ambiguous empty segments"),
/**
- * Non standard UTF-16 encoding eg /foo%u2192bar.
+ * Non standard UTF-16 encoding eg {@code /foo%u2192bar}.
*/
UTF16("Non standard UTF-16 encoding");
@@ -217,12 +219,12 @@ static Immutable build(HttpURI schemeHostPort, HttpURI uri)
boolean isAbsolute();
/**
- * @return True if the URI has either an {@link #hasAmbiguousParameter()}, {@link #hasAmbiguousSegment()} or {@link #hasAmbiguousSeparator()}.
+ * @return True if the URI has any ambiguous {@link Violation}s.
*/
boolean isAmbiguous();
/**
- * @return True if the URI has any Violations.
+ * @return True if the URI has any {@link Violation}s.
*/
boolean hasViolations();
@@ -1319,6 +1321,8 @@ else if (c == '/')
switch (encodedValue)
{
case 0:
+ // Byte 0 cannot be present in a UTF-8 sequence in any position
+ // other than as the NUL ASCII byte which we do not wish to allow.
throw new IllegalArgumentException("Illegal character in path");
case '/':
_violations.add(Violation.SEPARATOR);
@@ -1405,26 +1409,11 @@ else if (c == '/')
}
case QUERY:
{
- switch (c)
+ if (c == '#')
{
- case '%':
- encodedCharacters = 2;
- break;
- case 'u':
- case 'U':
- if (encodedCharacters == 1)
- _violations.add(Violation.UTF16);
- encodedCharacters = 0;
- break;
- case '#':
- _query = uri.substring(mark, i);
- mark = i + 1;
- state = State.FRAGMENT;
- encodedCharacters = 0;
- break;
- default:
- encodedCharacters = 0;
- break;
+ _query = uri.substring(mark, i);
+ mark = i + 1;
+ state = State.FRAGMENT;
}
break;
}
@@ -1439,7 +1428,9 @@ else if (c == '/')
break;
}
default:
+ {
throw new IllegalStateException(state.toString());
+ }
}
}
@@ -1493,8 +1484,8 @@ else if (_path != null)
{
// The RFC requires this to be canonical before decoding, but this can leave dot segments and dot dot segments
// which are not canonicalized and could be used in an attempt to bypass security checks.
- String decodeNonCanonical = URIUtil.decodePath(_path);
- _decodedPath = URIUtil.canonicalPath(decodeNonCanonical);
+ String decodedNonCanonical = URIUtil.decodePath(_path);
+ _decodedPath = URIUtil.canonicalPath(decodedNonCanonical);
if (_decodedPath == null)
throw new IllegalArgumentException("Bad URI");
}
@@ -1515,7 +1506,8 @@ private void checkSegment(String uri, int segment, int end, boolean param)
// This method is called once for every segment parsed.
// A URI like "/foo/" has two segments: "foo" and an empty segment.
// Empty segments are only ambiguous if they are not the last segment
- // So if this method is called for any segment and we have previously seen an empty segment, then it was ambiguous
+ // So if this method is called for any segment and we have previously
+ // seen an empty segment, then it was ambiguous.
if (_emptySegment)
_violations.add(Violation.EMPTY);
diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
index 45e20953fa10..0d56bba97fbc 100644
--- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
+++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
@@ -36,6 +36,7 @@
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
+// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
public class HttpURITest
{
@Test
@@ -358,6 +359,7 @@ public static Stream decodePathTests()
{"/f%6f%6F/bar", "/foo/bar", EnumSet.noneOf(Violation.class)},
{"/f%u006f%u006F/bar", "/foo/bar", EnumSet.of(Violation.UTF16)},
{"/f%u0001%u0001/bar", "/f\001\001/bar", EnumSet.of(Violation.UTF16)},
+ {"/foo/%u20AC/bar", "/foo/\u20AC/bar", EnumSet.of(Violation.UTF16)},
// illegal paths
{"//host/../path/info", null, EnumSet.noneOf(Violation.class)},
@@ -369,6 +371,9 @@ public static Stream decodePathTests()
{"/path/%u000X/info", null, EnumSet.noneOf(Violation.class)},
{"/path/Fo%u0000/info", null, EnumSet.noneOf(Violation.class)},
{"/path/Fo%00/info", null, EnumSet.noneOf(Violation.class)},
+ {"/path/Foo/info%u0000", null, EnumSet.noneOf(Violation.class)},
+ {"/path/Foo/info%00", null, EnumSet.noneOf(Violation.class)},
+ {"/path/%U20AC", null, EnumSet.noneOf(Violation.class)},
{"%2e%2e/info", null, EnumSet.noneOf(Violation.class)},
{"%u002e%u002e/info", null, EnumSet.noneOf(Violation.class)},
{"%2e%2e;/info", null, EnumSet.noneOf(Violation.class)},
@@ -803,4 +808,20 @@ public void testCompareToJavaNetURI(String input, String scheme, String host, In
assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(javaUri.getFragment()));
assertThat("[" + input + "] .toString", httpUri.toString(), is(javaUri.toASCIIString()));
}
+
+ public static Stream queryData()
+ {
+ return Stream.of(
+ new String[]{"/path?p=%U20AC", "p=%U20AC"},
+ new String[]{"/path?p=%u20AC", "p=%u20AC"}
+ ).map(Arguments::of);
+ }
+
+ @ParameterizedTest
+ @MethodSource("queryData")
+ public void testEncodedQuery(String input, String expectedQuery)
+ {
+ HttpURI httpURI = HttpURI.build(input);
+ assertThat("[" + input + "] .query", httpURI.getQuery(), is(expectedQuery));
+ }
}
diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebAppContext.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebAppContext.java
index c246f9688511..74a60614ba0a 100644
--- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebAppContext.java
+++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebAppContext.java
@@ -361,6 +361,7 @@ public Resource getResource(String uriInContext) throws MalformedURLException
// /WEB-INF/classes
if ((resource == null || !resource.exists()) && uriInContext != null && _classes != null)
{
+ // Canonicalize again to look for the resource inside /WEB-INF subdirectories.
String uri = URIUtil.canonicalPath(uriInContext);
if (uri == null)
return null;
diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java
index 058dac8293da..750c4ad2ea4c 100644
--- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java
+++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java
@@ -48,12 +48,12 @@ public static String toRedirectURL(final HttpServletRequest request, String loca
String path = request.getRequestURI();
String parent = (path.endsWith("/")) ? path : URIUtil.parentPath(path);
location = URIUtil.canonicalURI(URIUtil.addEncodedPaths(parent, location));
- if (!location.startsWith("/"))
+ if (location != null && !location.startsWith("/"))
url.append('/');
}
if (location == null)
- throw new IllegalStateException("path cannot be above root");
+ throw new IllegalStateException("redirect path cannot be above root");
url.append(location);
location = url.toString();
diff --git a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java
index 363aedb3ad0c..17f370e60948 100644
--- a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java
+++ b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java
@@ -20,6 +20,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@SuppressWarnings("unused")
@@ -71,6 +72,12 @@ public void testInvalidUrlWithMessage() throws Exception
@Test
public void testInvalidJsp() throws Exception
+ {
+ assertThrows(IllegalArgumentException.class, () -> HttpURI.build(_request.getHttpURI(), "/jsp/bean1.jsp%00"));
+ }
+
+ @Test
+ public void testInvalidJspWithNullByte() throws Exception
{
_rule.setCode("405");
_rule.setMessage("foo");
@@ -83,6 +90,12 @@ public void testInvalidJsp() throws Exception
assertEquals("foo", _request.getAttribute(Dispatcher.ERROR_MESSAGE));
}
+ @Test
+ public void testInvalidShamrock() throws Exception
+ {
+ assertThrows(IllegalArgumentException.class, () -> HttpURI.build(_request.getHttpURI(), "/jsp/shamrock-%00%E2%98%98.jsp"));
+ }
+
@Test
public void testValidShamrock() throws Exception
{
@@ -106,4 +119,3 @@ public void testCharacters() throws Exception
//@checkstyle-enable-check : IllegalTokenText
}
}
-
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
index 6fe16df2bfb6..f496fa782c63 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
@@ -1926,13 +1926,11 @@ public Resource getResource(String path) throws MalformedURLException
if (_baseResource == null)
return null;
- // Does the path go above the current scope?
- path = URIUtil.canonicalPath(path);
- if (path == null)
- return null;
-
try
{
+ // addPath with accept non-canonical paths that don't go above the root,
+ // but will treat them as aliases. So unless allowed by an AliasChecker
+ // they will be rejected below.
Resource resource = _baseResource.addPath(path);
if (checkAlias(path, resource))
@@ -2274,6 +2272,10 @@ else if (path.charAt(0) != '/')
@Override
public URL getResource(String path) throws MalformedURLException
{
+ // This is an API call from the application which may have arbitrary non canonical paths passed
+ // Thus we canonicalize here, to avoid the enforcement of only canonical paths in
+ // ContextHandler.this.getResource(path).
+ path = URIUtil.canonicalPath(path);
if (path == null)
return null;
Resource resource = ContextHandler.this.getResource(path);
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java
index 5a571fada0f2..bd7ba44c5297 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java
@@ -168,8 +168,6 @@ public Resource getResource(String path) throws IOException
else if (_context != null)
{
r = _context.getResource(path);
- if (r != null)
- return r;
}
if ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css"))
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
index a7e56e725a03..b6d301285445 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
@@ -823,6 +823,12 @@ public void testBadUTF8FallsbackTo8859() throws Exception
LOG.info("badMessage: bad encoding expected ...");
String response;
+ response = connector.getResponse("GET /foo/bar%c0%00 HTTP/1.1\r\n" +
+ "Host: localhost\r\n" +
+ "Connection: close\r\n" +
+ "\r\n");
+ checkContains(response, 0, "HTTP/1.1 400");
+
response = connector.getResponse("GET /bad/utf8%c1 HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Connection: close\r\n" +
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java
index 4f021b6c55f8..d304c0bb2160 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java
@@ -230,16 +230,23 @@ public void testDoesNotExistResource() throws Exception
@Test
public void testAlias() throws Exception
{
- Resource resource = context.getResource("/./index.html");
- assertNotNull(resource);
- assertFalse(resource.isAlias());
+ String path = "/./index.html";
+ Resource resource = context.getResource(path);
+ assertNull(resource);
+ URL resourceURL = context.getServletContext().getResource(path);
+ assertFalse(resourceURL.getPath().contains("/./"));
- resource = context.getResource("/down/../index.html");
- assertNotNull(resource);
- assertFalse(resource.isAlias());
+ path = "/down/../index.html";
+ resource = context.getResource(path);
+ assertNull(resource);
+ resourceURL = context.getServletContext().getResource(path);
+ assertFalse(resourceURL.getPath().contains("/../"));
- resource = context.getResource("//index.html");
+ path = "//index.html";
+ resource = context.getResource(path);
assertNull(resource);
+ resourceURL = context.getServletContext().getResource(path);
+ assertNull(resourceURL);
}
@ParameterizedTest
diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java
index 3ee3295d5ea7..13228219a49b 100644
--- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java
+++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java
@@ -71,7 +71,8 @@ public static Stream data()
ret.add(Arguments.of("/hello%u0025world", "/hello%u0025world", null));
ret.add(Arguments.of("/hello-euro-%E2%82%AC", "/hello-euro-%E2%82%AC", null));
ret.add(Arguments.of("/hello-euro?%E2%82%AC", "/hello-euro", "%E2%82%AC"));
- // test the ascii control characters (just for completeness)
+ // Test the ascii control characters (just for completeness).
+ // Zero is not allowed in UTF-8 sequences so start from 1.
for (int i = 0x1; i < 0x1f; i++)
{
String raw = String.format("/hello%%%02Xworld", i);
@@ -196,7 +197,6 @@ public void testGetRequestURIHTTP10(String rawpath, String expectedReqUri, Strin
// Read the response.
String response = readResponse(client);
- // TODO: is HTTP/1.1 response appropriate for an HTTP/1.0 request?
assertThat(response, Matchers.containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.containsString("RequestURI: " + expectedReqUri));
assertThat(response, Matchers.containsString("QueryString: " + expectedQuery));
@@ -223,4 +223,45 @@ public void testGetRequestURIHTTP11(String rawpath, String expectedReqUri, Strin
assertThat(response, Matchers.containsString("QueryString: " + expectedQuery));
}
}
+
+ public static Stream badData()
+ {
+ List ret = new ArrayList<>();
+ ret.add(Arguments.of("/hello\000"));
+ ret.add(Arguments.of("/hello%00"));
+ ret.add(Arguments.of("/hello%u0000"));
+ ret.add(Arguments.of("/hello\000/world"));
+ ret.add(Arguments.of("/hello%00world"));
+ ret.add(Arguments.of("/hello%u0000world"));
+ ret.add(Arguments.of("/hello%GG"));
+ ret.add(Arguments.of("/hello%;/world"));
+ ret.add(Arguments.of("/hello/../../world"));
+ ret.add(Arguments.of("/hello/..;/world"));
+ ret.add(Arguments.of("/hello/..;?/world"));
+ ret.add(Arguments.of("/hello/%#x/../world"));
+ ret.add(Arguments.of("/../hello/world"));
+ ret.add(Arguments.of("/hello%u00u00/world"));
+ ret.add(Arguments.of("hello"));
+
+ return ret.stream();
+ }
+
+ @ParameterizedTest
+ @MethodSource("badData")
+ public void testGetBadRequestsURIHTTP10(String rawpath) throws Exception
+ {
+ try (Socket client = newSocket(serverURI.getHost(), serverURI.getPort()))
+ {
+ OutputStream os = client.getOutputStream();
+
+ String request = String.format("GET %s HTTP/1.0\r\n\r\n", rawpath);
+ os.write(request.getBytes(StandardCharsets.ISO_8859_1));
+ os.flush();
+
+ // Read the response.
+ String response = readResponse(client);
+
+ assertThat(response, Matchers.containsString("HTTP/1.1 400 "));
+ }
+ }
}
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 b91b71c30081..d706b230b010 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
@@ -533,7 +533,6 @@ public static String decodePath(String path, int offset, int length)
{
throw new IllegalArgumentException("cannot decode URI", e);
}
-
}
/* Decode a URI path and strip parameters of ISO-8859-1 path
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 755c4b5f3299..69c438e4adab 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
@@ -86,8 +86,11 @@ private Path checkAliasPath()
if (!abs.isAbsolute())
abs = path.toAbsolutePath();
+ // Any normalization difference means it's an alias,
+ // and we don't want to bother further to follow
+ // symlinks as it's an alias anyway.
Path normal = path.normalize();
- if (!abs.equals(normal))
+ if (!isSameName(abs, normal))
return normal;
try
@@ -97,11 +100,8 @@ private Path checkAliasPath()
if (Files.exists(path))
{
Path real = abs.toRealPath(FOLLOW_LINKS);
-
if (!isSameName(abs, real))
- {
return real;
- }
}
}
catch (IOException e)
@@ -348,20 +348,23 @@ public boolean isSame(Resource resource)
}
@Override
- public Resource addPath(final String subpath) throws IOException
+ public Resource addPath(final String subPath) throws IOException
{
- if ((subpath == null) || (subpath.length() == 0))
- throw new MalformedURLException(subpath);
+ // Check that the path is within the root,
+ // but use the original path to create the
+ // resource, to preserve aliasing.
+ if (URIUtil.canonicalPath(subPath) == null)
+ throw new MalformedURLException(subPath);
- if ("/".equals(subpath))
+ if ("/".equals(subPath))
return this;
- // subpaths are always under PathResource
- // compensate for input subpaths like "/subdir"
+ // Sub-paths are always under PathResource
+ // compensate for input sub-paths like "/subdir"
// where default resolve behavior would be
// to treat that like an absolute path
- return new PathResource(this, subpath);
+ return new PathResource(this, subPath);
}
private void assertValidPath(Path path)
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 03afc1fd2efc..585855dd0cb2 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
@@ -419,10 +419,12 @@ public abstract boolean renameTo(Resource dest)
* Returns the resource contained inside the current resource with the
* given name, which may or may not exist.
*
- * @param path The path segment to add, which is not encoded
+ * @param path The path segment to add, which is not encoded. The path may be non canonical, but if so then
+ * the resulting Resource will return true from {@link #isAlias()}.
* @return the Resource for the resolved path within this Resource, never null
* @throws IOException if unable to resolve the path
- * @throws MalformedURLException if the resolution of the path fails because the input path parameter is malformed.
+ * @throws MalformedURLException if the resolution of the path fails because the input path parameter is malformed, or
+ * a relative path attempts to access above the root resource.
*/
public abstract Resource addPath(String path)
throws IOException, MalformedURLException;
@@ -477,6 +479,8 @@ public URI getAlias()
*/
public String getListHTML(String base, boolean parent, String query) throws IOException
{
+ // This method doesn't check aliases, so it is OK to canonicalize here.
+ 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 b15d3e26f07b..6ed655080333 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,8 +280,11 @@ public String[] list()
public Resource addPath(String path)
throws IOException
{
- if (path == null)
- throw new MalformedURLException("null path");
+ // Check that the path is within the root,
+ // but use the original path to create the
+ // resource, to preserve aliasing.
+ if (URIUtil.canonicalPath(path) == null)
+ throw new MalformedURLException(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 d8bac6a9ea5f..d6e11da38ce1 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
@@ -146,7 +146,9 @@ public void testCanonicalPath(String input, String expectedResult)
// Check canonicalURI
if (expectedResult == null)
+ {
assertThat(URIUtil.canonicalURI(input), nullValue());
+ }
else
{
// mostly encodedURI will be the same
@@ -157,4 +159,22 @@ public void testCanonicalPath(String input, String expectedResult)
}
}
+ public static Stream queries()
+ {
+ String[][] data =
+ {
+ {"/ctx/../dir?/../index.html", "/dir?/../index.html"},
+ {"/get-files?file=/etc/passwd", "/get-files?file=/etc/passwd"},
+ {"/get-files?file=../../../../../passwd", "/get-files?file=../../../../../passwd"}
+ };
+ return Stream.of(data).map(Arguments::of);
+ }
+
+ @ParameterizedTest
+ @MethodSource("queries")
+ public void testQuery(String input, String expectedPath)
+ {
+ String actual = URIUtil.canonicalURI(input);
+ assertThat(actual, is(expectedPath));
+ }
}
diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java
index 1ee036f58548..b1787fe2d726 100644
--- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java
+++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java
@@ -16,6 +16,7 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.nio.file.Path;
@@ -40,6 +41,8 @@
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
public class ResourceTest
{
@@ -320,4 +323,19 @@ public void testEqualsWindowsCaseInsensitiveDrive() throws Exception
assertEquals(rb, ra);
}
+
+ @Test
+ public void testClimbAboveBase() throws Exception
+ {
+ Resource resource = Resource.newResource("/foo/bar");
+ assertThrows(MalformedURLException.class, () -> resource.addPath(".."));
+
+ Resource same = resource.addPath(".");
+ assertNotNull(same);
+ assertTrue(same.isAlias());
+
+ assertThrows(MalformedURLException.class, () -> resource.addPath("./.."));
+
+ assertThrows(MalformedURLException.class, () -> resource.addPath("./../bar"));
+ }
}