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..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 @@ -49,7 +49,8 @@ public class PathResource extends Resource private final Path path; private final URI uri; - private boolean targetResolved = false; + private boolean alias = false; + private boolean aliasResolved = false; private Path targetPath; /** @@ -142,8 +143,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); } @@ -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; @@ -170,36 +174,43 @@ public static boolean isSameName(Path pathA, Path pathB) @Override public boolean exists() { - return Files.exists(targetPath != null ? targetPath : path); - } - - @Override - public boolean equals(Object obj) - { - if (this == obj) + if (aliasResolved) { - return true; - } - if (obj == null) - { - return false; + if (targetPath == null) + return false; + return Files.exists(targetPath); } - if (getClass() != obj.getClass()) + else { - return false; + // Avoid resolving alias unless necessary to determine existence. + if (Files.exists(path)) + return true; + if (isAlias() && targetPath != null) + return Files.exists(targetPath); } - PathResource other = (PathResource)obj; - return Objects.equals(path, other.path); + + return false; } - /** - * @return the {@link Path} of the resource - */ + @Override public Path getPath() { return path; } + public Path getTargetPath() + { + checkAlias(); + return targetPath; + } + + @Override + public URI getTargetURI() + { + Path targetPath = getTargetPath(); + return (targetPath == null) ? null : targetPath.toUri(); + } + public List list() { if (!isDirectory()) @@ -220,6 +231,13 @@ public List list() return List.of(); // empty } + @Override + public boolean isAlias() + { + checkAlias(); + return alias; + } + @Override public String getName() { @@ -241,90 +259,52 @@ public URI getURI() return this.uri; } - @Override - public int hashCode() - { - return Objects.hashCode(path); - } - @Override public boolean isContainedIn(Resource r) { return r.getClass() == PathResource.class && path.startsWith(r.getPath()); } - @Override - public URI getTargetURI() + private void checkAlias() { - if (!targetResolved) + if (!aliasResolved) { + aliasResolved = true; targetPath = resolveTargetPath(); - targetResolved = true; - } - if (targetPath == null) - return null; - return targetPath.toUri(); - } - - 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 + if (targetPath == null) { - // Use normalized path to get past navigational references like "/bar/../foo/test.txt" - Path ref = Paths.get(uri.normalize()); - return ref.toRealPath(); + alias = true; } - catch (IOException ioe) + else { - // If the toRealPath() call fails, then let - // the alias checking routines continue on - // to other techniques. - LOG.trace("IGNORED", ioe); + /* 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 + * 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. + * + * // 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.getTargetURI() == "file:///C:/temp/aa/foo.txt" + */ + alias = !isSameName(path, targetPath) || !Objects.equals(uri, toUri(targetPath)); } } + } - 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; - + private Path resolveTargetPath() + { 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; - } + return path.normalize().toRealPath(); } catch (IOException e) { @@ -334,6 +314,7 @@ private Path resolveTargetPath() { LOG.warn("bad alias ({} {}) for {}", e.getClass().getName(), e.getMessage(), path); } + return null; } @@ -371,6 +352,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/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 6a9afb96465d..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 @@ -325,19 +325,18 @@ public Resource resolve(String subUriPath) */ public boolean isAlias() { - return getTargetURI() != null; + return false; } /** - * 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() { - return null; + 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 d44dcdb6bd12..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 @@ -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; @@ -48,16 +47,15 @@ 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 - URI targetURI = keystoreResource.getTargetURI(); - if (targetURI != null) - monitoredFile = Paths.get(targetURI); + Path monitoredFile = keystoreResource.getPath(); + if (keystoreResource.isAlias()) + 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 5818517ca536..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 @@ -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; @@ -53,12 +54,14 @@ 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; -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.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; @@ -83,21 +86,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 @@ -116,15 +118,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 @@ -582,9 +576,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)); @@ -622,13 +616,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, hasNoTargetURI()); - assertThat("resource.uri.targetURI", ResourceFactory.root().newResource(resFoo.getURI()), hasNoTargetURI()); - assertThat("resource.file.targetURI", ResourceFactory.root().newResource(resFoo.getPath()), hasNoTargetURI()); - - 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)); + // 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 @@ -643,9 +642,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 @@ -681,9 +680,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. @@ -717,9 +716,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 { @@ -759,9 +758,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 { @@ -803,9 +802,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 { @@ -843,7 +842,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 @@ -1038,25 +1037,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("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, hasNoTargetURI()); + assertTrue(fileres.isAlias()); 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("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, hasNoTargetURI()); + assertTrue(fileres.isAlias()); URI refA = dir.toUri().resolve("foo's.txt"); URI refB = dir.toUri().resolve("foo%27s.txt"); @@ -1067,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 @@ -1142,14 +1145,14 @@ public void testResolveWindowsSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa%5C/foo.txt"); - assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); - if (WINDOWS.isCurrentOs()) { - assertThat("getPath().toString()", r.getPath().toString(), containsString("aa\\foo.txt")); + 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")); @@ -1157,9 +1160,11 @@ public void testResolveWindowsSlash() throws Exception } else { - assertThat("getPath().toString()", r.getPath().toString(), containsString("aa\\/foo.txt")); - assertThat("isAlias()", r.isAlias(), is(false)); + assertThat("getURI()", r.getPath().toString(), containsString("aa\\/foo.txt")); + assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa%5C/foo.txt")); + assertThat("Exists: " + r, r.exists(), is(false)); + assertThat("isAlias()", r.isAlias(), is(true)); } } catch (IllegalArgumentException e) @@ -1186,7 +1191,7 @@ public void testResolveWindowsExtensionLess() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("aa./foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("aa./foo.txt")); @@ -1200,8 +1205,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) @@ -1226,7 +1231,7 @@ public void testResolveInitialSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + assertThat("Is Not Alias: " + basePath, base, isNotAlias()); Resource r = base.resolve("/foo.txt"); assertThat("getURI()", r.getURI().toASCIIString(), containsString("/foo.txt")); @@ -1256,13 +1261,13 @@ public void testResolveInitialDoubleSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + 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("getTargetURI()", r.getTargetURI(), nullValue()); + assertThat("Is Not Alias: " + r.getPath(), r, isNotAlias()); } catch (IllegalArgumentException e) { @@ -1288,13 +1293,13 @@ public void testResolveDoubleSlash() throws Exception try { assertThat("Exists: " + basePath, base.exists(), is(true)); - assertThat("Target URI: " + basePath, base, hasNoTargetURI()); + 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("getTargetURI()", r.getTargetURI(), nullValue()); + assertThat("Is Not Alias: " + r.getPath(), r, isNotAlias()); } catch (IllegalArgumentException e) { @@ -1342,11 +1347,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("Is Not Alias: " + utf8Dir, base, isNotAlias()); Resource r = base.resolve("file.txt"); assertThat("Exists: " + r, r.exists(), is(true)); - assertThat("Target URI: " + r, r, hasNoTargetURI()); + assertThat("Is Not Alias: " + r, r, isNotAlias()); } @Test @@ -1357,7 +1362,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("Is Not Alias: " + resource.getPath(), resource, isNotAlias()); } private String toString(Resource resource) throws IOException 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..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 @@ -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; @@ -239,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"); @@ -303,37 +306,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) { @@ -413,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.getTargetPath())); + 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.getTargetPath()); + assertTrue(symlinkResource.isAlias()); + symlinkResource = new PathResource(symlinkPath); + assertFalse(symlinkResource.exists()); + assertNull(symlinkResource.getTargetPath()); + assertTrue(symlinkResource.isAlias()); + } + @Test public void testResolveNavigation(WorkDir workDir) throws Exception { 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..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-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 aa219b0d9f0d..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; 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 c13ec516468f..02755c9c29c6 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 @@ -593,6 +593,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 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-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) { 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);