From 97739f97bda49538d213078b53bdec77f05a761b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 28 Sep 2022 15:09:17 +1000 Subject: [PATCH 01/13] simplify the PathResource.resolveTargetPath code Signed-off-by: Lachlan Roberts --- .../jetty/util/resource/PathResource.java | 54 ++----------------- 1 file changed, 3 insertions(+), 51 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index d18c7422835b..618cdfd896dd 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -268,63 +268,15 @@ public URI getTargetURI() private Path resolveTargetPath() { - Path abs = path; - - // TODO: is this a valid shortcut? // If the path doesn't exist, then there's no alias to reference if (!Files.exists(path)) return null; - /* Catch situation where the Path class has already normalized - * the URI eg. input path "aa./foo.txt" - * from an #resolve(String) is normalized away during - * the creation of a Path object reference. - * If the URI is different from the Path.toUri() then - * we will just use the original URI to construct the - * alias reference Path. - * - * We use the method `toUri(Path)` here, instead of - * Path.toUri() to ensure that the path contains - * a trailing slash if it's a directory, (something - * not all FileSystems seem to support) - */ - if (!URIUtil.equalsIgnoreEncodings(uri, toUri(path))) - { - try - { - // Use normalized path to get past navigational references like "/bar/../foo/test.txt" - Path ref = Paths.get(uri.normalize()); - return ref.toRealPath(); - } - catch (IOException ioe) - { - // If the toRealPath() call fails, then let - // the alias checking routines continue on - // to other techniques. - LOG.trace("IGNORED", ioe); - } - } - - 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 (!isSameName(abs, normal)) - return normal; - try { - if (Files.isSymbolicLink(path)) - return path.getParent().resolve(Files.readSymbolicLink(path)); - if (Files.exists(path)) - { - Path real = abs.toRealPath(); - if (!isSameName(abs, real)) - return real; - } + Path real = path.toRealPath(); + if (!isSameName(path, real)) + return real; } catch (IOException e) { From 9e29c9566eefce29d5e1efbb5c8b8b59558ccd14 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 28 Sep 2022 22:19:38 +1000 Subject: [PATCH 02/13] changes to how PathResource handles aliases Signed-off-by: Lachlan Roberts --- .../jetty/util/resource/PathResource.java | 64 ++++++++++++++----- .../eclipse/jetty/util/resource/Resource.java | 4 +- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 618cdfd896dd..a602a0e22c02 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -49,7 +49,7 @@ public class PathResource extends Resource private final Path path; private final URI uri; - private boolean targetResolved = false; + private boolean alias = false; private Path targetPath; /** @@ -142,8 +142,8 @@ public static boolean isSameName(Path pathA, Path pathB) PathResource(URI uri, boolean bypassAllowedSchemeCheck) { - // normalize to referenced location, Paths.get() doesn't like "/bar/../foo/text.txt" style references - // and will return a Path that will not be found with `Files.exists()` or `Files.isDirectory()` + // Normalize to referenced location, Paths.get() doesn't like "/bar/../foo/text.txt" style references + // and will return a Path that will not be found with `Files.exists()` or `Files.isDirectory()`. this(Paths.get(uri.normalize()), uri, bypassAllowedSchemeCheck); } @@ -170,7 +170,11 @@ public static boolean isSameName(Path pathA, Path pathB) @Override public boolean exists() { - return Files.exists(targetPath != null ? targetPath : path); + if (Files.exists(path)) + return true; + if (isAlias()) + return Files.exists(targetPath); + return false; } @Override @@ -220,6 +224,13 @@ public List list() return List.of(); // empty } + @Override + public boolean isAlias() + { + checkAlias(); + return alias; + } + @Override public String getName() { @@ -256,27 +267,45 @@ public boolean isContainedIn(Resource r) @Override public URI getTargetURI() { - if (!targetResolved) + checkAlias(); + return targetPath.toUri(); + } + + private void checkAlias() + { + if (targetPath == null) { targetPath = resolveTargetPath(); - targetResolved = true; + if (Files.exists(targetPath)) + { + if (!isSameName(path, targetPath)) + alias = true; + else if (!URIUtil.equalsIgnoreEncodings(uri, toUri(path))) + { + /* Catch situation where the Path class has already normalized + * the URI e.g. input path "aa./foo.txt" + * from an #resolve(String) is normalized away during + * the creation of a Path object reference. + * If the URI is different from the Path.toUri() then + * we will just use the original URI to construct the + * alias reference Path. + * + * We use the method `toUri(Path)` here, instead of + * Path.toUri() to ensure that the path contains + * a trailing slash if it's a directory, (something + * not all FileSystems seem to support) + */ + alias = true; + } + } } - if (targetPath == null) - return null; - return targetPath.toUri(); } private Path resolveTargetPath() { - // If the path doesn't exist, then there's no alias to reference - if (!Files.exists(path)) - return null; - try { - Path real = path.toRealPath(); - if (!isSameName(path, real)) - return real; + return path.normalize().toRealPath(); } catch (IOException e) { @@ -286,7 +315,8 @@ private Path resolveTargetPath() { LOG.warn("bad alias ({} {}) for {}", e.getClass().getName(), e.getMessage(), path); } - return null; + + return path.normalize(); } @Override diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index bfd59027cc60..2358d427b46a 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -325,7 +325,7 @@ public Resource resolve(String subUriPath) */ public boolean isAlias() { - return getTargetURI() != null; + return false; } /** @@ -337,7 +337,7 @@ public boolean isAlias() */ public URI getTargetURI() { - return null; + return getURI(); } /** From fcd6e56ad7e2998e6aacdf43b9622d4917fd9b14 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 28 Sep 2022 22:24:13 +1000 Subject: [PATCH 03/13] fix usages of Resource.getTargetUri() Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java | 6 ++---- .../org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java | 4 ++-- .../org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index d44dcdb6bd12..1cef4318680e 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.util.ssl; -import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -55,9 +54,8 @@ public KeyStoreScanner(SslContextFactory sslContextFactory) throw new IllegalArgumentException("expected keystore file not directory"); // Use real location of keystore (if different), so that change monitoring can work properly - URI targetURI = keystoreResource.getTargetURI(); - if (targetURI != null) - monitoredFile = Paths.get(targetURI); + if (keystoreResource.isAlias()) + monitoredFile = Paths.get(keystoreResource.getTargetURI()); keystoreFile = monitoredFile; if (LOG.isDebugEnabled()) diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java index 5e0bfb28d083..a46ecf0c0866 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java @@ -292,9 +292,9 @@ public void unpack(WebAppContext context) throws IOException throw new IllegalStateException("No resourceBase or war set for context"); // Use real location (if different) for WAR file, so that change/modification monitoring can work. - URI targetURI = webApp.getTargetURI(); - if (targetURI != null) + if (webApp.isAlias()) { + URI targetURI = webApp.getTargetURI(); if (LOG.isDebugEnabled()) LOG.debug("{} anti-aliased to {}", webApp, targetURI); webApp = context.newResource(targetURI); diff --git a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java index 58e440abe804..b29e2014775c 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java +++ b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java @@ -290,9 +290,9 @@ public void unpack(WebAppContext context) throws IOException throw new IllegalStateException("No resourceBase or war set for context"); // Use real location (if different) for WAR file, so that change/modification monitoring can work. - URI targetURI = webApp.getTargetURI(); - if (targetURI != null) + if (webApp.isAlias()) { + URI targetURI = webApp.getTargetURI(); if (LOG.isDebugEnabled()) LOG.debug("{} anti-aliased to {}", webApp, targetURI); webApp = context.newResource(targetURI); From a32aa837e1428e8ffaef0f7462ee3b1cc37857fe Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 29 Sep 2022 05:44:28 +1000 Subject: [PATCH 04/13] fixes for FileSystemResourceTest Signed-off-by: Lachlan Roberts --- .../util/resource/FileSystemResourceTest.java | 106 +++++++++--------- 1 file changed, 50 insertions(+), 56 deletions(-) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index 223c0a99f27f..a79404eff1f1 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -57,7 +57,6 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -84,21 +83,20 @@ public void afterEach() assertThat(FileSystemPool.INSTANCE.mounts(), empty()); } - private Matcher hasNoTargetURI() + private Matcher isNotAlias() { return new BaseMatcher<>() { @Override public boolean matches(Object item) { - final Resource res = (Resource)item; - return res.getTargetURI() == null; + return !((Resource)item).isAlias(); } @Override public void describeTo(Description description) { - description.appendText("getTargetURI should return null"); + description.appendText("isAlias() should return false"); } @Override @@ -117,15 +115,7 @@ private Matcher isTargetFor(final Resource resource) public boolean matches(Object item) { final Resource ritem = (Resource)item; - final URI alias = ritem.getTargetURI(); - if (alias == null) - { - return resource.getTargetURI() == null; - } - else - { - return alias.equals(resource.getURI()); - } + return ritem.getTargetURI().equals(resource.getTargetURI()); } @Override @@ -583,9 +573,9 @@ public void testSymlink() throws Exception // Access to the same resource, but via a symlink means that they are not equivalent assertThat("foo.equals(bar)", resFoo.equals(resBar), is(false)); - assertThat("resource.targetURI", resFoo, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resFoo.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resFoo.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resFoo, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resFoo.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resFoo.getPath()), isNotAlias()); assertThat("targetURI", resBar, isTargetFor(resFoo)); assertThat("uri.targetURI", ResourceFactory.root().newResource(resBar.getURI()), isTargetFor(resFoo)); @@ -623,13 +613,14 @@ public void testNonExistantSymlink() throws Exception // Access to the same resource, but via a symlink means that they are not equivalent assertThat("foo.equals(bar)", resFoo.equals(resBar), is(false)); - assertThat("resource.targetURI", resFoo, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resFoo.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resFoo.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resFoo, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resFoo.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resFoo.getPath()), isNotAlias()); - assertThat("targetURI", resBar, isTargetFor(resFoo)); - assertThat("uri.targetURI", ResourceFactory.root().newResource(resBar.getURI()), isTargetFor(resFoo)); - assertThat("file.targetURI", ResourceFactory.root().newResource(resBar.getPath()), isTargetFor(resFoo)); + // Even though resBar contains symlink it is not an alias because the file does not exist. + assertThat("targetURI", resBar, isNotAlias()); + assertThat("uri.targetURI", ResourceFactory.root().newResource(resBar.getURI()), isNotAlias()); + assertThat("file.targetURI", ResourceFactory.root().newResource(resBar.getPath()), isNotAlias()); } @Test @@ -644,9 +635,9 @@ public void testCaseInsensitiveAlias() throws Exception // Reference to actual resource that exists Resource resource = base.resolve("file"); - assertThat("resource.targetURI", resource, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resource, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), isNotAlias()); // On some case insensitive file systems, lets see if an alternate // case for the filename results in an alias reference @@ -682,9 +673,9 @@ public void testCase8dot3Alias() throws Exception // Long filename Resource resource = base.resolve("TextFile.Long.txt"); - assertThat("resource.targetURI", resource, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resource, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), isNotAlias()); // On some versions of Windows, the long filename can be referenced // via a short 8.3 equivalent filename. @@ -718,9 +709,9 @@ public void testNTFSFileStreamAlias() throws Exception Resource base = ResourceFactory.root().newResource(dir); Resource resource = base.resolve("testfile"); - assertThat("resource.targetURI", resource, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resource, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), isNotAlias()); try { @@ -760,9 +751,9 @@ public void testNTFSFileDataStreamAlias() throws Exception Resource base = ResourceFactory.root().newResource(dir); Resource resource = base.resolve("testfile"); - assertThat("resource.targetURI", resource, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resource, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), isNotAlias()); try { @@ -804,9 +795,9 @@ public void testNTFSFileEncodedDataStreamAlias() throws Exception Resource base = ResourceFactory.root().newResource(dir); Resource resource = base.resolve("testfile"); - assertThat("resource.targetURI", resource, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), hasNoTargetURI()); + assertThat("resource.targetURI", resource, isNotAlias()); + assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resource.getURI()), isNotAlias()); + assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resource.getPath()), isNotAlias()); try { @@ -844,7 +835,7 @@ public void testSemicolon() throws Exception Resource base = ResourceFactory.root().newResource(dir); Resource res = base.resolve("foo;"); - assertThat("Target URI: " + res, res, hasNoTargetURI()); + assertThat("Target URI: " + res, res, isNotAlias()); } @Test @@ -1039,25 +1030,25 @@ public void testSingleQuoteInFileName() throws Exception Resource fileres = ResourceFactory.root().newResource(refQuoted); assertThat("Exists: " + refQuoted, fileres.exists(), is(true)); - assertThat("Target URI: " + refQuoted, fileres, hasNoTargetURI()); + assertThat("Target URI: " + refQuoted, fileres, isNotAlias()); URI refEncoded = dir.toUri().resolve("foo%27s.txt"); fileres = ResourceFactory.root().newResource(refEncoded); assertThat("Exists: " + refEncoded, fileres.exists(), is(true)); - assertThat("Target URI: " + refEncoded, fileres, hasNoTargetURI()); + assertThat("Target URI: " + refEncoded, fileres, isNotAlias()); URI refQuoteSpace = dir.toUri().resolve("f%20o's.txt"); fileres = ResourceFactory.root().newResource(refQuoteSpace); assertThat("Exists: " + refQuoteSpace, fileres.exists(), is(true)); - assertThat("Target URI: " + refQuoteSpace, fileres, hasNoTargetURI()); + assertThat("Target URI: " + refQuoteSpace, fileres, isNotAlias()); URI refEncodedSpace = dir.toUri().resolve("f%20o%27s.txt"); fileres = ResourceFactory.root().newResource(refEncodedSpace); assertThat("Exists: " + refEncodedSpace, fileres.exists(), is(true)); - assertThat("Target URI: " + refEncodedSpace, fileres, hasNoTargetURI()); + assertThat("Target URI: " + refEncodedSpace, fileres, isNotAlias()); URI refA = dir.toUri().resolve("foo's.txt"); URI refB = dir.toUri().resolve("foo%27s.txt"); @@ -1143,14 +1134,14 @@ public void testResolveWindowsSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Target URI: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa%5C/foo.txt"); - assertThat("getURI()", r.getPath().toString(), containsString("aa\\/foo.txt")); - assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); - if (org.junit.jupiter.api.condition.OS.WINDOWS.isCurrentOs()) { + assertThat("getURI()", r.getPath().toString(), containsString("aa\\foo.txt")); + assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); + assertThat("isAlias()", r.isAlias(), is(true)); assertThat("getTargetURI()", r.getTargetURI(), notNullValue()); assertThat("getTargetURI()", r.getTargetURI().toASCIIString(), containsString("aa/foo.txt")); @@ -1158,6 +1149,9 @@ public void testResolveWindowsSlash() throws Exception } else { + assertThat("getURI()", r.getPath().toString(), containsString("aa\\/foo.txt")); + assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); + assertThat("isAlias()", r.isAlias(), is(false)); assertThat("Exists: " + r, r.exists(), is(false)); } @@ -1186,7 +1180,7 @@ public void testResolveWindowsExtensionLess() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Target URI: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa./foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa./foo.txt")); @@ -1226,7 +1220,7 @@ public void testResolveInitialSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Target URI: " + basePath, base, isNotAlias()); Resource r = base.resolve("/foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("/foo.txt")); @@ -1256,13 +1250,13 @@ public void testResolveInitialDoubleSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Target URI: " + basePath, base, isNotAlias()); Resource r = base.resolve("//foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("/foo.txt")); assertThat("isAlias()", r.isAlias(), is(false)); - assertThat("getTargetURI()", r.getTargetURI(), nullValue()); + assertThat(r, isNotAlias()); } catch (IllegalArgumentException e) { @@ -1288,13 +1282,13 @@ public void testResolveDoubleSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Target URI: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa//foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa/foo.txt")); assertThat("isAlias()", r.isAlias(), is(false)); - assertThat("getTargetURI()", r.getTargetURI(), nullValue()); + assertThat(r, isNotAlias()); } catch (IllegalArgumentException e) { @@ -1342,11 +1336,11 @@ public void testUtf8Dir() throws Exception Resource base = ResourceFactory.root().newResource(utf8Dir); assertThat("Exists: " + utf8Dir, base.exists(), is(true)); - assertThat("Target URI: " + utf8Dir, base, hasNoTargetURI()); + assertThat("Target URI: " + utf8Dir, base, isNotAlias()); Resource r = base.resolve("file.txt"); assertThat("Exists: " + r, r.exists(), is(true)); - assertThat("Target URI: " + r, r, hasNoTargetURI()); + assertThat("Target URI: " + r, r, isNotAlias()); } @Test @@ -1357,7 +1351,7 @@ public void testUncPath() throws Exception Resource resource = base.resolve("WEB-INF/"); assertThat("getURI()", resource.getURI().toASCIIString(), containsString("path/WEB-INF/")); assertThat("isAlias()", resource.isAlias(), is(false)); - assertThat("getTargetURI()", resource.getTargetURI(), nullValue()); + assertThat(resource, isNotAlias()); } private String toString(Resource resource) throws IOException From db4a44ce10ae866fb247fe3940e557a1a9d1c235 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 30 Sep 2022 10:46:20 +1000 Subject: [PATCH 05/13] update javadoc for Resource.getTargetURI() Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/util/resource/Resource.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 2358d427b46a..b8eebc1aa9f6 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -329,11 +329,10 @@ public boolean isAlias() } /** - * If this Resource is an alias pointing to a different location, - * return the target location as URI. + * The target URI of the resource. If this Resource is an alias pointing to a different location, + * this will resolve the alias to return the true target URI of the resource. * - * @return The target URI location of this resource, - * or null if there is no target URI location (eg: not an alias, or a symlink) + * @return The target URI location of this resource, with any aliases resolved. */ public URI getTargetURI() { From 882cdd93373dac86af47416a306e3daa4c46a326 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 12 Oct 2022 14:04:37 +1100 Subject: [PATCH 06/13] rename getTargetURI to getCanonicalURI Signed-off-by: Lachlan Roberts --- .../jetty/server/handler/ContextHandler.java | 2 +- .../jetty/util/resource/PathResource.java | 69 +++++++++---------- .../eclipse/jetty/util/resource/Resource.java | 6 +- .../jetty/util/ssl/KeyStoreScanner.java | 2 +- .../util/resource/FileSystemResourceTest.java | 14 ++-- .../jetty/util/resource/PathResourceTest.java | 27 ++++---- .../util/resource/ResourceAliasTest.java | 22 +++--- .../jetty/util/resource/ResourceTest.java | 4 +- .../ee10/servlet/ServletContextHandler.java | 2 +- .../ee10/webapp/MetaInfConfiguration.java | 1 + .../ee10/webapp/WebInfConfiguration.java | 2 +- .../jetty/ee9/nested/ContextHandler.java | 2 +- .../jetty/ee9/webapp/WebInfConfiguration.java | 2 +- 13 files changed, 73 insertions(+), 82 deletions(-) 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 1e7a9b860d83..8f2053df3192 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 @@ -855,7 +855,7 @@ public boolean checkAlias(String pathInContext, Resource resource) if (resource.isAlias()) { if (LOG.isDebugEnabled()) - LOG.debug("Aliased resource: {} -> {}", resource, resource.getTargetURI()); + LOG.debug("Aliased resource: {} -> {}", resource, resource.getCanonicalURI()); // alias checks for (AliasCheck check : _aliasChecks) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index a602a0e22c02..6b088430c47d 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -50,7 +50,7 @@ public class PathResource extends Resource private final Path path; private final URI uri; private boolean alias = false; - private Path targetPath; + private Path canonicalPath; /** * Test if the paths are the same name. @@ -173,7 +173,7 @@ public boolean exists() if (Files.exists(path)) return true; if (isAlias()) - return Files.exists(targetPath); + return Files.exists(canonicalPath); return false; } @@ -196,14 +196,24 @@ public boolean equals(Object obj) return Objects.equals(path, other.path); } - /** - * @return the {@link Path} of the resource - */ + @Override public Path getPath() { return path; } + public Path getCanonicalPath() + { + checkAlias(); + return canonicalPath; + } + + @Override + public URI getCanonicalURI() + { + return getCanonicalPath().toUri(); + } + public List list() { if (!isDirectory()) @@ -264,48 +274,31 @@ public boolean isContainedIn(Resource r) return r.getClass() == PathResource.class && path.startsWith(r.getPath()); } - @Override - public URI getTargetURI() - { - checkAlias(); - return targetPath.toUri(); - } - private void checkAlias() { - if (targetPath == null) + if (canonicalPath == null) { - targetPath = resolveTargetPath(); - if (Files.exists(targetPath)) - { - if (!isSameName(path, targetPath)) - alias = true; - else if (!URIUtil.equalsIgnoreEncodings(uri, toUri(path))) - { - /* Catch situation where the Path class has already normalized - * the URI e.g. input path "aa./foo.txt" - * from an #resolve(String) is normalized away during - * the creation of a Path object reference. - * If the URI is different from the Path.toUri() then - * we will just use the original URI to construct the - * alias reference Path. - * - * We use the method `toUri(Path)` here, instead of - * Path.toUri() to ensure that the path contains - * a trailing slash if it's a directory, (something - * not all FileSystems seem to support) - */ - alias = true; - } - } + canonicalPath = resolveTargetPath(); + + /* If the path and canonicalPath are the same also check + * the Path class has already normalized in the constructor + * from the URI e.g. input path "aa./foo.txt" + * from an #resolve(String) is normalized away during + * the creation of a Path object reference. + * If the URI is different from the Path.toUri() then + * we will just use the original URI to construct the + * alias reference Path. + */ + alias = !isSameName(path, canonicalPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); } } private Path resolveTargetPath() { + Path normalized = path.normalize(); try { - return path.normalize().toRealPath(); + return normalized.toRealPath(); } catch (IOException e) { @@ -316,7 +309,7 @@ private Path resolveTargetPath() LOG.warn("bad alias ({} {}) for {}", e.getClass().getName(), e.getMessage(), path); } - return path.normalize(); + return normalized; } @Override diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index b8eebc1aa9f6..d847f286e9d5 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -329,12 +329,12 @@ public boolean isAlias() } /** - * The target URI of the resource. If this Resource is an alias pointing to a different location, + * The canonical URI of the resource. If this Resource is an alias pointing to a different location, * this will resolve the alias to return the true target URI of the resource. * - * @return The target URI location of this resource, with any aliases resolved. + * @return The canonical URI location of this resource, with any aliases resolved. */ - public URI getTargetURI() + public URI getCanonicalURI() { return getURI(); } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index 1cef4318680e..7c6e6692891f 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -55,7 +55,7 @@ public KeyStoreScanner(SslContextFactory sslContextFactory) // Use real location of keystore (if different), so that change monitoring can work properly if (keystoreResource.isAlias()) - monitoredFile = Paths.get(keystoreResource.getTargetURI()); + monitoredFile = Paths.get(keystoreResource.getCanonicalURI()); keystoreFile = monitoredFile; if (LOG.isDebugEnabled()) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index a79404eff1f1..e23309fdeb5c 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -102,7 +102,7 @@ public void describeTo(Description description) @Override public void describeMismatch(Object item, Description description) { - description.appendText("was ").appendValue(((Resource)item).getTargetURI()); + description.appendText("was ").appendValue(((Resource)item).getCanonicalURI()); } }; } @@ -115,7 +115,7 @@ private Matcher isTargetFor(final Resource resource) public boolean matches(Object item) { final Resource ritem = (Resource)item; - return ritem.getTargetURI().equals(resource.getTargetURI()); + return ritem.getCanonicalURI().equals(resource.getCanonicalURI()); } @Override @@ -127,7 +127,7 @@ public void describeTo(Description description) @Override public void describeMismatch(Object item, Description description) { - description.appendText("was ").appendValue(((Resource)item).getTargetURI()); + description.appendText("was ").appendValue(((Resource)item).getCanonicalURI()); } }; } @@ -1143,8 +1143,8 @@ public void testResolveWindowsSlash() throws Exception assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); assertThat("isAlias()", r.isAlias(), is(true)); - assertThat("getTargetURI()", r.getTargetURI(), notNullValue()); - assertThat("getTargetURI()", r.getTargetURI().toASCIIString(), containsString("aa/foo.txt")); + assertThat("getTargetURI()", r.getCanonicalURI(), notNullValue()); + assertThat("getTargetURI()", r.getCanonicalURI().toASCIIString(), containsString("aa/foo.txt")); assertThat("Exists: " + r, r.exists(), is(true)); } else @@ -1188,8 +1188,8 @@ public void testResolveWindowsExtensionLess() throws Exception if (OS.WINDOWS.isCurrentOs()) { assertThat("isAlias()", r.isAlias(), is(true)); - assertThat("getTargetURI()", r.getTargetURI(), notNullValue()); - assertThat("getTargetURI()", r.getTargetURI().toASCIIString(), containsString("aa/foo.txt")); + assertThat("getTargetURI()", r.getCanonicalURI(), notNullValue()); + assertThat("getTargetURI()", r.getCanonicalURI().toASCIIString(), containsString("aa/foo.txt")); assertThat("Exists: " + r, r.exists(), is(true)); } else diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 9c0a2a701d4f..bb8644f7ca33 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -45,7 +45,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -303,37 +302,37 @@ public void testNullCharEndingFilename(WorkDir workDir) throws Exception // Test not alias paths Resource resource = resourceFactory.newResource(file); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = resourceFactory.newResource(file.toAbsolutePath()); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = resourceFactory.newResource(file.toUri()); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = resourceFactory.newResource(file.toUri().toString()); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = archiveResource.resolve("test.txt"); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); // Test alias paths resource = resourceFactory.newResource(file0); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = resourceFactory.newResource(file0.toAbsolutePath()); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = resourceFactory.newResource(file0.toUri()); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = resourceFactory.newResource(file0.toUri().toString()); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = archiveResource.resolve("test.txt\0"); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); } catch (InvalidPathException e) { @@ -402,9 +401,9 @@ public void testSymlink(WorkDir workDir) throws Exception assertThat("resource.uri.alias", resourceFactory.newResource(resFoo.getURI()).isAlias(), is(false)); assertThat("resource.file.alias", resourceFactory.newResource(resFoo.getPath()).isAlias(), is(false)); - assertThat("targetURI", resBar.getTargetURI(), is(resFoo.getURI())); - assertThat("uri.targetURI", resourceFactory.newResource(resBar.getURI()).getTargetURI(), is(resFoo.getURI())); - assertThat("file.targetURI", resourceFactory.newResource(resBar.getPath()).getTargetURI(), is(resFoo.getURI())); + assertThat("targetURI", resBar.getCanonicalURI(), is(resFoo.getURI())); + assertThat("uri.targetURI", resourceFactory.newResource(resBar.getURI()).getCanonicalURI(), is(resFoo.getURI())); + assertThat("file.targetURI", resourceFactory.newResource(resBar.getPath()).getCanonicalURI(), is(resFoo.getURI())); } catch (InvalidPathException e) { diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceAliasTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceAliasTest.java index 86ce43343b98..d2425028a9dc 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceAliasTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceAliasTest.java @@ -33,8 +33,6 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @ExtendWith(WorkDirExtension.class) @@ -132,37 +130,37 @@ public void testNullCharEndingFilename() throws Exception // Test not alias paths Resource resource = resourceFactory.newResource(file); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = resourceFactory.newResource(file.toAbsolutePath()); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = resourceFactory.newResource(file.toUri()); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = resourceFactory.newResource(file.toUri().toString()); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); resource = dir.resolve("test.txt"); assertTrue(resource.exists()); - assertNull(resource.getTargetURI()); + assertFalse(resource.isAlias()); // Test alias paths resource = resourceFactory.newResource(file0); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = resourceFactory.newResource(file0.toAbsolutePath()); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = resourceFactory.newResource(file0.toUri()); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = resourceFactory.newResource(file0.toUri().toString()); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); resource = dir.resolve("test.txt\0"); assertTrue(resource.exists()); - assertNotNull(resource.getTargetURI()); + assertTrue(resource.isAlias()); } catch (InvalidPathException e) { diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java index 5f431e1a7e21..7a5c4f262b31 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java @@ -386,7 +386,7 @@ public void testDotAliasDirExists(WorkDir workDir) throws IOException assertNotNull(dot); assertTrue(dot.exists()); assertTrue(dot.isAlias(), "Reference to '.' is an alias to itself"); - assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getTargetURI()))); + assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getCanonicalURI()))); } @Test @@ -413,7 +413,7 @@ public void testDotAliasFileExists(WorkDir workDir) throws IOException assertNotNull(dot); assertTrue(dot.exists()); assertTrue(dot.isAlias(), "Reference to '.' is an alias to itself"); - assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getTargetURI()))); + assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getCanonicalURI()))); } @Test 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 da432c772e55..c68399e8b341 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 @@ -1081,7 +1081,7 @@ protected void doStart() throws Exception Resource baseResource = getBaseResource(); if (baseResource != null && baseResource.isAlias()) LOG.warn("BaseResource {} is aliased to {} in {}. May not be supported in future releases.", - baseResource, baseResource.getTargetURI(), this); + baseResource, baseResource.getCanonicalURI(), this); if (_logger == null) _logger = LoggerFactory.getLogger(ContextHandler.class.getName() + getLogNameSuffix()); diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java index ff3c6ccf768c..dacf1398b7de 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java @@ -598,6 +598,7 @@ public void postConfigure(WebAppContext context) throws Exception * @return the list of tlds found * @throws IOException if unable to scan the directory */ + // TODO: Needs to use resource. public Collection getTlds(Path dir) throws IOException { if (dir == null || !Files.isDirectory(dir)) diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java index a46ecf0c0866..125c5c25c8cb 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java @@ -294,7 +294,7 @@ public void unpack(WebAppContext context) throws IOException // Use real location (if different) for WAR file, so that change/modification monitoring can work. if (webApp.isAlias()) { - URI targetURI = webApp.getTargetURI(); + URI targetURI = webApp.getCanonicalURI(); if (LOG.isDebugEnabled()) LOG.debug("{} anti-aliased to {}", webApp, targetURI); webApp = context.newResource(targetURI); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index 7c1faca62744..79706fa3272b 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -1404,7 +1404,7 @@ public boolean checkAlias(String path, Resource resource) if (resource.isAlias()) { if (LOG.isDebugEnabled()) - LOG.debug("Alias resource {} for {}", resource, resource.getTargetURI()); + LOG.debug("Alias resource {} for {}", resource, resource.getCanonicalURI()); // alias checks for (AliasCheck check : getAliasChecks()) diff --git a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java index b29e2014775c..0524ab26d4a1 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java +++ b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java @@ -292,7 +292,7 @@ public void unpack(WebAppContext context) throws IOException // Use real location (if different) for WAR file, so that change/modification monitoring can work. if (webApp.isAlias()) { - URI targetURI = webApp.getTargetURI(); + URI targetURI = webApp.getCanonicalURI(); if (LOG.isDebugEnabled()) LOG.debug("{} anti-aliased to {}", webApp, targetURI); webApp = context.newResource(targetURI); From a3d9efb48dc9a5397942adcd9160fac5d56e8d8f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 13 Oct 2022 13:31:11 +1100 Subject: [PATCH 07/13] let resolveCanonicalPath return null if resource does not exist Signed-off-by: Lachlan Roberts --- .../jetty/util/resource/PathResource.java | 97 ++++++++++--------- .../jetty/util/ssl/KeyStoreScanner.java | 6 +- 2 files changed, 54 insertions(+), 49 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 6b088430c47d..fe82602966fe 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -50,6 +50,7 @@ public class PathResource extends Resource private final Path path; private final URI uri; private boolean alias = false; + private boolean aliasResolved = false; private Path canonicalPath; /** @@ -159,9 +160,12 @@ public static boolean isSameName(Path pathA, Path pathB) if (!bypassAllowedSchemeCheck && !ALLOWED_SCHEMES.contains(uri.getScheme())) throw new IllegalArgumentException("not an allowed scheme: " + uri); - String uriString = uri.toASCIIString(); - if (Files.isDirectory(path) && !uriString.endsWith(URIUtil.SLASH)) - uri = URIUtil.correctFileURI(URI.create(uriString + URIUtil.SLASH)); + if (Files.isDirectory(path)) + { + String uriString = uri.toASCIIString(); + if (!uriString.endsWith(URIUtil.SLASH)) + uri = URIUtil.correctFileURI(URI.create(uriString + URIUtil.SLASH)); + } this.path = path; this.uri = uri; @@ -177,25 +181,6 @@ public boolean exists() return false; } - @Override - public boolean equals(Object obj) - { - if (this == obj) - { - return true; - } - if (obj == null) - { - return false; - } - if (getClass() != obj.getClass()) - { - return false; - } - PathResource other = (PathResource)obj; - return Objects.equals(path, other.path); - } - @Override public Path getPath() { @@ -211,7 +196,8 @@ public Path getCanonicalPath() @Override public URI getCanonicalURI() { - return getCanonicalPath().toUri(); + Path canonicalPath = getCanonicalPath(); + return (canonicalPath == null) ? null : canonicalPath.toUri(); } public List list() @@ -262,12 +248,6 @@ public URI getURI() return this.uri; } - @Override - public int hashCode() - { - return Objects.hashCode(path); - } - @Override public boolean isContainedIn(Resource r) { @@ -276,29 +256,35 @@ public boolean isContainedIn(Resource r) private void checkAlias() { - if (canonicalPath == null) + if (!aliasResolved) { - canonicalPath = resolveTargetPath(); - - /* If the path and canonicalPath are the same also check - * the Path class has already normalized in the constructor - * from the URI e.g. input path "aa./foo.txt" - * from an #resolve(String) is normalized away during - * the creation of a Path object reference. - * If the URI is different from the Path.toUri() then - * we will just use the original URI to construct the - * alias reference Path. - */ - alias = !isSameName(path, canonicalPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); + aliasResolved = true; + canonicalPath = resolveCanonicalPath(); + if (canonicalPath == null) + { + alias = true; + } + else + { + /* If the path and canonicalPath are the same also check + * the Path class has already normalized in the constructor + * from the URI e.g. input path "aa./foo.txt" + * from an #resolve(String) is normalized away during + * the creation of a Path object reference. + * If the URI is different from the Path.toUri() then + * we will just use the original URI to construct the + * alias reference Path. + */ + alias = !isSameName(path, canonicalPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); + } } } - private Path resolveTargetPath() + private Path resolveCanonicalPath() { - Path normalized = path.normalize(); try { - return normalized.toRealPath(); + return path.normalize().toRealPath(); } catch (IOException e) { @@ -309,7 +295,7 @@ private Path resolveTargetPath() LOG.warn("bad alias ({} {}) for {}", e.getClass().getName(), e.getMessage(), path); } - return normalized; + return null; } @Override @@ -346,6 +332,25 @@ private static URI toUri(Path path) return pathUri; } + @Override + public boolean equals(Object obj) + { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + PathResource other = (PathResource)obj; + return Objects.equals(path, other.path) && Objects.equals(uri, other.uri); + } + + @Override + public int hashCode() + { + return Objects.hash(path, uri); + } + @Override public String toString() { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index 7c6e6692891f..4ca18a1d4d3c 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -47,13 +47,13 @@ public KeyStoreScanner(SslContextFactory sslContextFactory) { this.sslContextFactory = sslContextFactory; Resource keystoreResource = sslContextFactory.getKeyStoreResource(); - Path monitoredFile = keystoreResource.getPath(); - if (monitoredFile == null || !Files.exists(monitoredFile)) + if (!keystoreResource.exists()) throw new IllegalArgumentException("keystore file does not exist"); - if (Files.isDirectory(monitoredFile)) + if (keystoreResource.isDirectory()) throw new IllegalArgumentException("expected keystore file not directory"); // Use real location of keystore (if different), so that change monitoring can work properly + Path monitoredFile = keystoreResource.getPath(); if (keystoreResource.isAlias()) monitoredFile = Paths.get(keystoreResource.getCanonicalURI()); From 4d39c8a2329915555d31195022ed11688dfa4e74 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 13 Oct 2022 14:08:08 +1100 Subject: [PATCH 08/13] add test in PathResourceTest for broken symlinks Signed-off-by: Lachlan Roberts --- .../jetty/util/resource/PathResource.java | 2 +- .../jetty/util/resource/PathResourceTest.java | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index fe82602966fe..48aa58db41a6 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -176,7 +176,7 @@ public boolean exists() { if (Files.exists(path)) return true; - if (isAlias()) + if (isAlias() && canonicalPath != null) return Files.exists(canonicalPath); return false; } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index bb8644f7ca33..45878243a746 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -23,12 +23,14 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.InvalidPathException; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.URIUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -38,6 +40,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -45,6 +48,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -412,6 +416,44 @@ public void testSymlink(WorkDir workDir) throws Exception } } + @Test + public void testBrokenSymlink(WorkDir workDir) throws Exception + { + Path testDir = workDir.getEmptyPathDir(); + Path resourcePath = testDir.resolve("resource.txt"); + IO.copy(MavenTestingUtils.getTestResourcePathFile("resource.txt").toFile(), resourcePath.toFile()); + Path symlinkPath = Files.createSymbolicLink(testDir.resolve("symlink.txt"), resourcePath); + + PathResource fileResource = new PathResource(resourcePath); + assertTrue(fileResource.exists()); + PathResource symlinkResource = new PathResource(symlinkPath); + assertTrue(symlinkResource.exists()); + + // Their paths are not equal but not their canonical paths are. + assertThat(fileResource.getPath(), not(equalTo(symlinkResource.getPath()))); + assertThat(fileResource.getPath(), equalTo(symlinkResource.getCanonicalPath())); + assertFalse(fileResource.isAlias()); + assertTrue(symlinkResource.isAlias()); + assertTrue(fileResource.exists()); + assertTrue(symlinkResource.exists()); + + // After deleting file the Resources do not exist even though symlink file exists. + Files.delete(resourcePath); + assertFalse(fileResource.exists()); + assertFalse(symlinkResource.exists()); + assertTrue(Files.exists(symlinkPath, LinkOption.NOFOLLOW_LINKS)); + + // Re-create and test the resources now that the file has been deleted. + fileResource = new PathResource(resourcePath); + assertFalse(fileResource.exists()); + assertNull(fileResource.getCanonicalPath()); + assertTrue(symlinkResource.isAlias()); + symlinkResource = new PathResource(symlinkPath); + assertFalse(symlinkResource.exists()); + assertNull(symlinkResource.getCanonicalPath()); + assertTrue(symlinkResource.isAlias()); + } + @Test public void testResolveNavigation(WorkDir workDir) throws Exception { From 4411d10ea06c9d4db52ded7e5be3c082d2aa7781 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 13 Oct 2022 15:02:38 +1100 Subject: [PATCH 09/13] some changes from review + optimization for exists() Signed-off-by: Lachlan Roberts --- .../jetty/util/resource/PathResource.java | 26 +++++++++++++++-- .../util/resource/FileSystemResourceTest.java | 28 +++++++++---------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 48aa58db41a6..a4e950b92610 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -174,10 +174,21 @@ public static boolean isSameName(Path pathA, Path pathB) @Override public boolean exists() { - if (Files.exists(path)) - return true; - if (isAlias() && canonicalPath != null) + if (aliasResolved) + { + if (canonicalPath == null) + return false; return Files.exists(canonicalPath); + } + else + { + // Avoid resolving alias unless necessary to determine existence. + if (Files.exists(path)) + return true; + if (isAlias() && canonicalPath != null) + return Files.exists(canonicalPath); + } + return false; } @@ -274,6 +285,15 @@ private void checkAlias() * If the URI is different from the Path.toUri() then * we will just use the original URI to construct the * alias reference Path. + * + * // On Windows + * PathResource resource = PathResource("C:/temp"); + * PathResource child = resource.resolve("aa./foo.txt"); + * child.exists() == true + * child.isAlias() == true + * child.toUri() == "file:///C:/temp/aa./foo.txt" + * child.getPath().toUri() == "file:///C:/temp/aa/foo.txt" + * child.getCanonicalURI() == "file:///C:/temp/aa/foo.txt" */ alias = !isSameName(path, canonicalPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index e23309fdeb5c..b1f69cf1d220 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -1030,25 +1030,25 @@ public void testSingleQuoteInFileName() throws Exception Resource fileres = ResourceFactory.root().newResource(refQuoted); assertThat("Exists: " + refQuoted, fileres.exists(), is(true)); - assertThat("Target URI: " + refQuoted, fileres, isNotAlias()); + assertThat("Is Not Alias: " + refQuoted, fileres, isNotAlias()); URI refEncoded = dir.toUri().resolve("foo%27s.txt"); fileres = ResourceFactory.root().newResource(refEncoded); assertThat("Exists: " + refEncoded, fileres.exists(), is(true)); - assertThat("Target URI: " + refEncoded, fileres, isNotAlias()); + assertThat("Is Not Alias: " + refEncoded, fileres, isNotAlias()); URI refQuoteSpace = dir.toUri().resolve("f%20o's.txt"); fileres = ResourceFactory.root().newResource(refQuoteSpace); assertThat("Exists: " + refQuoteSpace, fileres.exists(), is(true)); - assertThat("Target URI: " + refQuoteSpace, fileres, isNotAlias()); + assertThat("Is Not Alias: " + refQuoteSpace, fileres, isNotAlias()); URI refEncodedSpace = dir.toUri().resolve("f%20o%27s.txt"); fileres = ResourceFactory.root().newResource(refEncodedSpace); assertThat("Exists: " + refEncodedSpace, fileres.exists(), is(true)); - assertThat("Target URI: " + refEncodedSpace, fileres, isNotAlias()); + assertThat("Is Not Alias: " + refEncodedSpace, fileres, isNotAlias()); URI refA = dir.toUri().resolve("foo's.txt"); URI refB = dir.toUri().resolve("foo%27s.txt"); @@ -1134,7 +1134,7 @@ public void testResolveWindowsSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, isNotAlias()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa%5C/foo.txt"); if (org.junit.jupiter.api.condition.OS.WINDOWS.isCurrentOs()) @@ -1180,7 +1180,7 @@ public void testResolveWindowsExtensionLess() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, isNotAlias()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa./foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa./foo.txt")); @@ -1220,7 +1220,7 @@ public void testResolveInitialSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, isNotAlias()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("/foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("/foo.txt")); @@ -1250,13 +1250,13 @@ public void testResolveInitialDoubleSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, isNotAlias()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("//foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("/foo.txt")); assertThat("isAlias()", r.isAlias(), is(false)); - assertThat(r, isNotAlias()); + assertThat("Is Not Alias: " + r.getPath(), r, isNotAlias()); } catch (IllegalArgumentException e) { @@ -1282,13 +1282,13 @@ public void testResolveDoubleSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, isNotAlias()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa//foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa/foo.txt")); assertThat("isAlias()", r.isAlias(), is(false)); - assertThat(r, isNotAlias()); + assertThat("Is Not Alias: " + r.getPath(), r, isNotAlias()); } catch (IllegalArgumentException e) { @@ -1336,11 +1336,11 @@ public void testUtf8Dir() throws Exception Resource base = ResourceFactory.root().newResource(utf8Dir); assertThat("Exists: " + utf8Dir, base.exists(), is(true)); - assertThat("Target URI: " + utf8Dir, base, isNotAlias()); + assertThat("Is Not Alias: " + utf8Dir, base, isNotAlias()); Resource r = base.resolve("file.txt"); assertThat("Exists: " + r, r.exists(), is(true)); - assertThat("Target URI: " + r, r, isNotAlias()); + assertThat("Is Not Alias: " + r, r, isNotAlias()); } @Test @@ -1351,7 +1351,7 @@ public void testUncPath() throws Exception Resource resource = base.resolve("WEB-INF/"); assertThat("getURI()", resource.getURI().toASCIIString(), containsString("path/WEB-INF/")); assertThat("isAlias()", resource.isAlias(), is(false)); - assertThat(resource, isNotAlias()); + assertThat("Is Not Alias: " + resource.getPath(), resource, isNotAlias()); } private String toString(Resource resource) throws IOException From 98f25cbb2094aaf54ffa4743adff6c8b73eaaeb1 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 13 Oct 2022 23:03:38 +1100 Subject: [PATCH 10/13] restore name to getTargetUri in Resource Signed-off-by: Lachlan Roberts --- .../jetty/server/handler/ContextHandler.java | 2 +- .../jetty/util/resource/PathResource.java | 32 +++++++++---------- .../eclipse/jetty/util/resource/Resource.java | 6 ++-- .../jetty/util/ssl/KeyStoreScanner.java | 2 +- .../util/resource/FileSystemResourceTest.java | 14 ++++---- .../jetty/util/resource/PathResourceTest.java | 12 +++---- .../jetty/util/resource/ResourceTest.java | 4 +-- .../ee10/servlet/ServletContextHandler.java | 3 +- .../ee10/webapp/WebInfConfiguration.java | 2 +- .../jetty/ee9/nested/ContextHandler.java | 2 +- .../jetty/ee9/webapp/WebInfConfiguration.java | 2 +- 11 files changed, 40 insertions(+), 41 deletions(-) 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 8f2053df3192..1e7a9b860d83 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 @@ -855,7 +855,7 @@ public boolean checkAlias(String pathInContext, Resource resource) if (resource.isAlias()) { if (LOG.isDebugEnabled()) - LOG.debug("Aliased resource: {} -> {}", resource, resource.getCanonicalURI()); + LOG.debug("Aliased resource: {} -> {}", resource, resource.getTargetURI()); // alias checks for (AliasCheck check : _aliasChecks) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index a4e950b92610..d195dc01ce7c 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -51,7 +51,7 @@ public class PathResource extends Resource private final URI uri; private boolean alias = false; private boolean aliasResolved = false; - private Path canonicalPath; + private Path targetPath; /** * Test if the paths are the same name. @@ -176,17 +176,17 @@ public boolean exists() { if (aliasResolved) { - if (canonicalPath == null) + if (targetPath == null) return false; - return Files.exists(canonicalPath); + return Files.exists(targetPath); } else { // Avoid resolving alias unless necessary to determine existence. if (Files.exists(path)) return true; - if (isAlias() && canonicalPath != null) - return Files.exists(canonicalPath); + if (isAlias() && targetPath != null) + return Files.exists(targetPath); } return false; @@ -198,17 +198,17 @@ public Path getPath() return path; } - public Path getCanonicalPath() + public Path getTargetPath() { checkAlias(); - return canonicalPath; + return targetPath; } @Override - public URI getCanonicalURI() + public URI getTargetURI() { - Path canonicalPath = getCanonicalPath(); - return (canonicalPath == null) ? null : canonicalPath.toUri(); + Path targetPath = getTargetPath(); + return (targetPath == null) ? null : targetPath.toUri(); } public List list() @@ -270,14 +270,14 @@ private void checkAlias() if (!aliasResolved) { aliasResolved = true; - canonicalPath = resolveCanonicalPath(); - if (canonicalPath == null) + targetPath = resolveTargetPath(); + if (targetPath == null) { alias = true; } else { - /* If the path and canonicalPath are the same also check + /* If the path and targetPath are the same also check * the Path class has already normalized in the constructor * from the URI e.g. input path "aa./foo.txt" * from an #resolve(String) is normalized away during @@ -293,14 +293,14 @@ private void checkAlias() * child.isAlias() == true * child.toUri() == "file:///C:/temp/aa./foo.txt" * child.getPath().toUri() == "file:///C:/temp/aa/foo.txt" - * child.getCanonicalURI() == "file:///C:/temp/aa/foo.txt" + * child.getTargetURI() == "file:///C:/temp/aa/foo.txt" */ - alias = !isSameName(path, canonicalPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); + alias = !isSameName(path, targetPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); } } } - private Path resolveCanonicalPath() + private Path resolveTargetPath() { try { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 264fb154697b..8a82a889cad4 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -329,12 +329,12 @@ public boolean isAlias() } /** - * The canonical URI of the resource. If this Resource is an alias pointing to a different location, + * The target URI of the resource. If this Resource is an alias pointing to a different location, * this will resolve the alias to return the true target URI of the resource. * - * @return The canonical URI location of this resource, with any aliases resolved. + * @return The target URI location of this resource, with any aliases resolved. */ - public URI getCanonicalURI() + public URI getTargetURI() { return getURI(); } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index 4ca18a1d4d3c..5a4f334df309 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -55,7 +55,7 @@ public KeyStoreScanner(SslContextFactory sslContextFactory) // Use real location of keystore (if different), so that change monitoring can work properly Path monitoredFile = keystoreResource.getPath(); if (keystoreResource.isAlias()) - monitoredFile = Paths.get(keystoreResource.getCanonicalURI()); + monitoredFile = Paths.get(keystoreResource.getTargetURI()); keystoreFile = monitoredFile; if (LOG.isDebugEnabled()) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index 0384fdd0af61..0902f37b0c86 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -101,7 +101,7 @@ public void describeTo(Description description) @Override public void describeMismatch(Object item, Description description) { - description.appendText("was ").appendValue(((Resource)item).getCanonicalURI()); + description.appendText("was ").appendValue(((Resource)item).getTargetURI()); } }; } @@ -114,7 +114,7 @@ private Matcher isTargetFor(final Resource resource) public boolean matches(Object item) { final Resource ritem = (Resource)item; - return ritem.getCanonicalURI().equals(resource.getCanonicalURI()); + return ritem.getTargetURI().equals(resource.getTargetURI()); } @Override @@ -126,7 +126,7 @@ public void describeTo(Description description) @Override public void describeMismatch(Object item, Description description) { - description.appendText("was ").appendValue(((Resource)item).getCanonicalURI()); + description.appendText("was ").appendValue(((Resource)item).getTargetURI()); } }; } @@ -1142,8 +1142,8 @@ public void testResolveWindowsSlash() throws Exception assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); assertThat("isAlias()", r.isAlias(), is(true)); - assertThat("getTargetURI()", r.getCanonicalURI(), notNullValue()); - assertThat("getTargetURI()", r.getCanonicalURI().toASCIIString(), containsString("aa/foo.txt")); + assertThat("getTargetURI()", r.getTargetURI(), notNullValue()); + assertThat("getTargetURI()", r.getTargetURI().toASCIIString(), containsString("aa/foo.txt")); assertThat("Exists: " + r, r.exists(), is(true)); } else @@ -1187,8 +1187,8 @@ public void testResolveWindowsExtensionLess() throws Exception if (WINDOWS.isCurrentOs()) { assertThat("isAlias()", r.isAlias(), is(true)); - assertThat("getTargetURI()", r.getCanonicalURI(), notNullValue()); - assertThat("getTargetURI()", r.getCanonicalURI().toASCIIString(), containsString("aa/foo.txt")); + assertThat("getTargetURI()", r.getTargetURI(), notNullValue()); + assertThat("getTargetURI()", r.getTargetURI().toASCIIString(), containsString("aa/foo.txt")); assertThat("Exists: " + r, r.exists(), is(true)); } else diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 45878243a746..2407a3bd2bd6 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -405,9 +405,9 @@ public void testSymlink(WorkDir workDir) throws Exception assertThat("resource.uri.alias", resourceFactory.newResource(resFoo.getURI()).isAlias(), is(false)); assertThat("resource.file.alias", resourceFactory.newResource(resFoo.getPath()).isAlias(), is(false)); - assertThat("targetURI", resBar.getCanonicalURI(), is(resFoo.getURI())); - assertThat("uri.targetURI", resourceFactory.newResource(resBar.getURI()).getCanonicalURI(), is(resFoo.getURI())); - assertThat("file.targetURI", resourceFactory.newResource(resBar.getPath()).getCanonicalURI(), is(resFoo.getURI())); + assertThat("targetURI", resBar.getTargetURI(), is(resFoo.getURI())); + assertThat("uri.targetURI", resourceFactory.newResource(resBar.getURI()).getTargetURI(), is(resFoo.getURI())); + assertThat("file.targetURI", resourceFactory.newResource(resBar.getPath()).getTargetURI(), is(resFoo.getURI())); } catch (InvalidPathException e) { @@ -431,7 +431,7 @@ public void testBrokenSymlink(WorkDir workDir) throws Exception // Their paths are not equal but not their canonical paths are. assertThat(fileResource.getPath(), not(equalTo(symlinkResource.getPath()))); - assertThat(fileResource.getPath(), equalTo(symlinkResource.getCanonicalPath())); + assertThat(fileResource.getPath(), equalTo(symlinkResource.getTargetPath())); assertFalse(fileResource.isAlias()); assertTrue(symlinkResource.isAlias()); assertTrue(fileResource.exists()); @@ -446,11 +446,11 @@ public void testBrokenSymlink(WorkDir workDir) throws Exception // Re-create and test the resources now that the file has been deleted. fileResource = new PathResource(resourcePath); assertFalse(fileResource.exists()); - assertNull(fileResource.getCanonicalPath()); + assertNull(fileResource.getTargetPath()); assertTrue(symlinkResource.isAlias()); symlinkResource = new PathResource(symlinkPath); assertFalse(symlinkResource.exists()); - assertNull(symlinkResource.getCanonicalPath()); + assertNull(symlinkResource.getTargetPath()); assertTrue(symlinkResource.isAlias()); } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java index 7a5c4f262b31..5f431e1a7e21 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java @@ -386,7 +386,7 @@ public void testDotAliasDirExists(WorkDir workDir) throws IOException assertNotNull(dot); assertTrue(dot.exists()); assertTrue(dot.isAlias(), "Reference to '.' is an alias to itself"); - assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getCanonicalURI()))); + assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getTargetURI()))); } @Test @@ -413,7 +413,7 @@ public void testDotAliasFileExists(WorkDir workDir) throws IOException assertNotNull(dot); assertTrue(dot.exists()); assertTrue(dot.isAlias(), "Reference to '.' is an alias to itself"); - assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getCanonicalURI()))); + assertTrue(Files.isSameFile(dot.getPath(), Paths.get(dot.getTargetURI()))); } @Test 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 bf44271912e6..6af984ae53ec 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 @@ -74,7 +74,6 @@ import org.eclipse.jetty.ee10.servlet.security.SecurityHandler; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.http.pathmap.MappedResource; import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; @@ -1081,7 +1080,7 @@ protected void doStart() throws Exception Resource baseResource = getBaseResource(); if (baseResource != null && baseResource.isAlias()) LOG.warn("BaseResource {} is aliased to {} in {}. May not be supported in future releases.", - baseResource, baseResource.getCanonicalURI(), this); + baseResource, baseResource.getTargetURI(), this); if (_logger == null) _logger = LoggerFactory.getLogger(ContextHandler.class.getName() + getLogNameSuffix()); diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java index 125c5c25c8cb..a46ecf0c0866 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java @@ -294,7 +294,7 @@ public void unpack(WebAppContext context) throws IOException // Use real location (if different) for WAR file, so that change/modification monitoring can work. if (webApp.isAlias()) { - URI targetURI = webApp.getCanonicalURI(); + URI targetURI = webApp.getTargetURI(); if (LOG.isDebugEnabled()) LOG.debug("{} anti-aliased to {}", webApp, targetURI); webApp = context.newResource(targetURI); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index 79706fa3272b..7c1faca62744 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -1404,7 +1404,7 @@ public boolean checkAlias(String path, Resource resource) if (resource.isAlias()) { if (LOG.isDebugEnabled()) - LOG.debug("Alias resource {} for {}", resource, resource.getCanonicalURI()); + LOG.debug("Alias resource {} for {}", resource, resource.getTargetURI()); // alias checks for (AliasCheck check : getAliasChecks()) diff --git a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java index 0524ab26d4a1..b29e2014775c 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java +++ b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java @@ -292,7 +292,7 @@ public void unpack(WebAppContext context) throws IOException // Use real location (if different) for WAR file, so that change/modification monitoring can work. if (webApp.isAlias()) { - URI targetURI = webApp.getCanonicalURI(); + URI targetURI = webApp.getTargetURI(); if (LOG.isDebugEnabled()) LOG.debug("{} anti-aliased to {}", webApp, targetURI); webApp = context.newResource(targetURI); From 5fb4e65b7416fff28ed6e4b7d30af3f3a4f568d2 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 14 Oct 2022 12:34:14 +1100 Subject: [PATCH 11/13] fix some tests related to PathResource changes Signed-off-by: Lachlan Roberts --- .../util/resource/FileSystemResourceTest.java | 23 ++++++++++++------- .../jetty/util/resource/PathResourceTest.java | 2 +- .../jetty/util/resource/ResourceTest.java | 4 ++-- .../ee9/nested/CachedContentFactory.java | 2 +- .../jetty/ee9/nested/ContextHandler.java | 6 +---- .../jetty/ee9/nested/ResourceService.java | 13 +++++++++++ .../jetty/ee9/servlet/DefaultServlet.java | 2 -- 7 files changed, 33 insertions(+), 19 deletions(-) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index 0902f37b0c86..d7220809c762 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -30,6 +30,7 @@ import java.nio.file.FileSystemNotFoundException; import java.nio.file.Files; import java.nio.file.InvalidPathException; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.time.Instant; import java.util.List; @@ -58,6 +59,8 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -612,14 +615,18 @@ public void testNonExistantSymlink() throws Exception // Access to the same resource, but via a symlink means that they are not equivalent assertThat("foo.equals(bar)", resFoo.equals(resBar), is(false)); - assertThat("resource.targetURI", resFoo, isNotAlias()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resFoo.getURI()), isNotAlias()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resFoo.getPath()), isNotAlias()); - - // Even though resBar contains symlink it is not an alias because the file does not exist. - assertThat("targetURI", resBar, isNotAlias()); - assertThat("uri.targetURI", ResourceFactory.root().newResource(resBar.getURI()), isNotAlias()); - assertThat("file.targetURI", ResourceFactory.root().newResource(resBar.getPath()), isNotAlias()); + // This is an alias because the file does not exist. + assertFalse(resFoo.exists()); + assertTrue(resFoo.isAlias()); + assertNotNull(resFoo.getURI()); + assertNull(resFoo.getTargetURI()); + + // This is alias because the target file does not exist even though the symlink file does exist. + assertFalse(resBar.exists()); + assertTrue(Files.exists(resBar.getPath(), LinkOption.NOFOLLOW_LINKS)); + assertTrue(resBar.isAlias()); + assertNotNull(resBar.getURI()); + assertNull(resBar.getTargetURI()); } @Test diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 2407a3bd2bd6..04b05008e90c 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -242,7 +242,7 @@ public void testJarFileIsAliasDirectory(WorkDir workDir) throws IOException // Resolve file using extension-less directory testText = archiveResource.resolve("/dir./test.txt"); assertFalse(testText.exists()); - assertFalse(testText.isAlias()); + assertTrue(testText.isAlias()); // Resolve directory to name, no slash Resource dirResource = archiveResource.resolve("/dir"); diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java index 5f431e1a7e21..788c94105768 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java @@ -398,7 +398,7 @@ public void testDotAliasDirDoesNotExist(WorkDir workDir) Resource dot = resource.resolve("."); assertNotNull(dot); assertFalse(dot.exists()); - assertFalse(dot.isAlias(), "Reference to '.' is not an alias as directory doesn't exist"); + assertTrue(dot.isAlias(), "Reference to '.' is an alias as directory doesn't exist"); } @Test @@ -428,7 +428,7 @@ public void testDotAliasFileDoesNotExists(WorkDir workDir) throws IOException Resource dot = resource.resolve("."); assertNotNull(dot); assertFalse(dot.exists()); - assertFalse(dot.isAlias(), "Reference to '.' is not an alias as file doesn't exist"); + assertTrue(dot.isAlias(), "Reference to '.' is an alias as file doesn't exist"); } @Test diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java index 8942264fc2fa..0b7c6c6a02cc 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java @@ -231,7 +231,7 @@ private HttpContent load(String pathInContext, Resource resource) throws IOExcep { compressedContent = null; Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && ResourceContentFactory.newerThanOrEqual(compressedResource, resource) && + if (compressedResource != null && compressedResource.exists() && ResourceContentFactory.newerThanOrEqual(compressedResource, resource) && compressedResource.length() < resource.length()) { compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index 7c1faca62744..1ce7e0141f3d 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -1379,11 +1379,7 @@ public Resource getResource(String pathInContext) throws MalformedURLException // 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.resolve(pathInContext); - - if (checkAlias(pathInContext, resource)) - return resource; - return null; + return baseResource.resolve(pathInContext); } catch (Exception e) { diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java index acfe3a3a781a..de224af632cc 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java @@ -253,6 +253,15 @@ public boolean doGet(HttpServletRequest request, HttpServletResponse response) return response.isCommitted(); } + ContextHandler contextHandler = ContextHandler.getContextHandler(request.getServletContext()); + if (contextHandler != null && !contextHandler.checkAlias(pathInContext, content.getResource())) + { + if (included) + throw new FileNotFoundException("!" + pathInContext); + notFound(request, response); + return response.isCommitted(); + } + // Directory? if (content.getResource().isDirectory()) { @@ -289,6 +298,10 @@ public boolean doGet(HttpServletRequest request, HttpServletResponse response) HttpContent precompressedContent = precompressedContents.get(precompressedContentEncoding); if (LOG.isDebugEnabled()) LOG.debug("precompressed={}", precompressedContent); + + if (contextHandler != null && !contextHandler.checkAlias(pathInContext, precompressedContent.getResource())) + content = null; + content = precompressedContent; response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), precompressedContentEncoding.getEncoding()); } diff --git a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java index 2b1fbda59e8f..df7ea4d266f5 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java +++ b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java @@ -457,8 +457,6 @@ protected Resource resolve(String subUriPath) if (_baseResource != null) { r = _baseResource.resolve(subUriPath); - if (!_contextHandler.checkAlias(subUriPath, r)) - r = null; } else if (_servletContext instanceof ContextHandler.APIContext) { From bbc261cad7b6a77a3a9205ebe6379277230c102b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 14 Oct 2022 13:36:30 +1100 Subject: [PATCH 12/13] revert changes to PathResource equals and hashcode Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/util/resource/PathResource.java | 4 ++-- .../eclipse/jetty/util/resource/FileSystemResourceTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index d195dc01ce7c..187bc72ee6bb 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -362,13 +362,13 @@ public boolean equals(Object obj) if (getClass() != obj.getClass()) return false; PathResource other = (PathResource)obj; - return Objects.equals(path, other.path) && Objects.equals(uri, other.uri); + return Objects.equals(path, other.path); } @Override public int hashCode() { - return Objects.hash(path, uri); + return Objects.hashCode(path); } @Override diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index d7220809c762..c588799d8f7d 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -1158,8 +1158,8 @@ public void testResolveWindowsSlash() throws Exception assertThat("getURI()", r.getPath().toString(), containsString("aa\\/foo.txt")); assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); - assertThat("isAlias()", r.isAlias(), is(false)); assertThat("Exists: " + r, r.exists(), is(false)); + assertThat("isAlias()", r.isAlias(), is(true)); } } catch (IllegalArgumentException e) @@ -1200,8 +1200,8 @@ public void testResolveWindowsExtensionLess() throws Exception } else { - assertThat("isAlias()", r.isAlias(), is(false)); assertThat("Exists: " + r, r.exists(), is(false)); + assertThat("isAlias()", r.isAlias(), is(true)); } } catch (IllegalArgumentException e) From 79cd37d161c7fc56feb64b1d06c3b60d5dffc606 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 14 Oct 2022 14:14:20 +1100 Subject: [PATCH 13/13] also compare URI in PathResource Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/util/resource/PathResource.java | 6 +++--- .../jetty/util/resource/FileSystemResourceTest.java | 13 +++++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 187bc72ee6bb..2b5d1d45846e 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -295,7 +295,7 @@ private void checkAlias() * child.getPath().toUri() == "file:///C:/temp/aa/foo.txt" * child.getTargetURI() == "file:///C:/temp/aa/foo.txt" */ - alias = !isSameName(path, targetPath) || !URIUtil.equalsIgnoreEncodings(uri, toUri(path)); + alias = !isSameName(path, targetPath) || !Objects.equals(uri, toUri(targetPath)); } } } @@ -362,13 +362,13 @@ public boolean equals(Object obj) if (getClass() != obj.getClass()) return false; PathResource other = (PathResource)obj; - return Objects.equals(path, other.path); + return Objects.equals(path, other.path) && Objects.equals(uri, other.uri); } @Override public int hashCode() { - return Objects.hashCode(path); + return Objects.hash(path, uri); } @Override diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java index c588799d8f7d..91046069544c 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java @@ -54,6 +54,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -1042,7 +1043,7 @@ public void testSingleQuoteInFileName() throws Exception fileres = ResourceFactory.root().newResource(refEncoded); assertThat("Exists: " + refEncoded, fileres.exists(), is(true)); - assertThat("Is Not Alias: " + refEncoded, fileres, isNotAlias()); + assertTrue(fileres.isAlias()); URI refQuoteSpace = dir.toUri().resolve("f%20o's.txt"); @@ -1054,7 +1055,7 @@ public void testSingleQuoteInFileName() throws Exception fileres = ResourceFactory.root().newResource(refEncodedSpace); assertThat("Exists: " + refEncodedSpace, fileres.exists(), is(true)); - assertThat("Is Not Alias: " + refEncodedSpace, fileres, isNotAlias()); + assertTrue(fileres.isAlias()); URI refA = dir.toUri().resolve("foo's.txt"); URI refB = dir.toUri().resolve("foo%27s.txt"); @@ -1065,10 +1066,14 @@ public void testSingleQuoteInFileName() throws Exception "URI[b] = " + refB; assertThat(msg, refA.equals(refB), is(false)); - // now show that Resource.equals() does work + // These resources are not equal because they have different URIs. Resource a = ResourceFactory.root().newResource(refA); Resource b = ResourceFactory.root().newResource(refB); - assertThat("A.equals(B)", a.equals(b), is(true)); + assertThat("A.equals(B)", a.equals(b), is(false)); + assertThat(a.getPath(), equalTo(b.getPath())); + assertThat(a.getTargetURI(), equalTo(b.getTargetURI())); + assertFalse(a.isAlias()); + assertTrue(b.isAlias()); } @Test