From c1ae922ab051f0996806472d6cf8e1468e809e26 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 25 Mar 2021 07:44:27 +1100 Subject: [PATCH] Use Files.isSameFile to check Resource equality (#6093) (#6095) Use Files.isSameFile to check Resource equality Avoid using canonical and instead use Files.isSameFile Signed-off-by: Greg Wilkins Co-authored-by: Ludovic Orban --- .../deploy/providers/ScanningAppProvider.java | 26 +------ .../deploy/providers/WebAppProvider.java | 75 +++++++------------ .../jetty/util/resource/PathResource.java | 19 +++++ .../eclipse/jetty/util/resource/Resource.java | 12 +++ .../jetty/util/resource/PathResourceTest.java | 30 ++++++++ 5 files changed, 89 insertions(+), 73 deletions(-) diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java index df978f389f96..eb66704f59de 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java @@ -15,7 +15,6 @@ import java.io.File; import java.io.FilenameFilter; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -43,10 +42,10 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements private static final Logger LOG = LoggerFactory.getLogger(ScanningAppProvider.class); private final Map _appMap = new HashMap<>(); + private DeploymentManager _deploymentManager; private FilenameFilter _filenameFilter; private final List _monitored = new CopyOnWriteArrayList<>(); - private final List _canonicalMonitored = new CopyOnWriteArrayList<>(); private int _scanInterval = 10; private Scanner _scanner; @@ -238,35 +237,12 @@ public void setMonitoredResources(List resources) { _monitored.clear(); _monitored.addAll(resources); - _canonicalMonitored.clear(); - for (Resource r : resources) - { - Resource canonical = r; //assume original is canonical - try - { - File f = r.getFile(); //if it has a file, ensure it is canonical - if (f != null) - canonical = Resource.newResource(f.getCanonicalFile()); - } - catch (IOException e) - { - //use the original resource - if (LOG.isDebugEnabled()) - LOG.debug("ignored", e); - } - _canonicalMonitored.add(canonical); - } } public List getMonitoredResources() { return Collections.unmodifiableList(_monitored); } - - List getCanonicalMonitoredResources() - { - return Collections.unmodifiableList(_canonicalMonitored); - } public void setMonitoredDirResource(Resource resource) { diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java index 5d781f7280ad..daf54df94750 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java @@ -15,7 +15,6 @@ import java.io.File; import java.io.FilenameFilter; -import java.io.IOException; import java.util.Locale; import org.eclipse.jetty.deploy.App; @@ -26,8 +25,6 @@ import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.webapp.WebAppContext; import org.eclipse.jetty.xml.XmlConfiguration; @@ -75,62 +72,44 @@ public class Filter implements FilenameFilter @Override public boolean accept(File dir, String name) { - if (dir == null) + if (dir == null || !dir.canRead()) return false; - try + String lowerName = name.toLowerCase(Locale.ENGLISH); + + Resource resource = Resource.newResource(new File(dir, name)); + if (getMonitoredResources().stream().anyMatch(resource::isSame)) + return false; + + // ignore hidden files + if (lowerName.startsWith(".")) + return false; + + // Ignore some directories + if (resource.isDirectory()) { - //Always work on canonical files because we need to - //check if the file passed in is one of the - //monitored resources - if (!dir.getCanonicalFile().exists()) + // is it a nominated config directory + if (lowerName.endsWith(".d")) return false; - - String lowerName = name.toLowerCase(Locale.ENGLISH); - File canonical = new File(dir, name).getCanonicalFile(); - Resource r = Resource.newResource(canonical); - if (getCanonicalMonitoredResources().contains(r) && r.isDirectory()) + // is it an unpacked directory for an existing war file? + if (exists(name + ".war") || exists(name + ".WAR")) return false; - // ignore hidden files - if (lowerName.startsWith(".")) + // is it a directory for an existing xml file? + if (exists(name + ".xml") || exists(name + ".XML")) return false; - // Ignore some directories - if (canonical.isDirectory()) - { - // is it a nominated config directory - if (lowerName.endsWith(".d")) - return false; - - // is it an unpacked directory for an existing war file? - if (exists(name + ".war") || exists(name + ".WAR")) - return false; - - // is it a directory for an existing xml file? - if (exists(name + ".xml") || exists(name + ".XML")) - return false; - - //is it a sccs dir? - return !"cvs".equals(lowerName) && !"cvsroot".equals(lowerName); // OK to deploy it then - } + //is it a sccs dir? + return !"cvs".equals(lowerName) && !"cvsroot".equals(lowerName); // OK to deploy it then + } - // else is it a war file - if (lowerName.endsWith(".war")) - { - //defer deployment decision to fileChanged() - return true; - } + // else is it a war file + if (lowerName.endsWith(".war")) + return true; - // else is it a context XML file - return lowerName.endsWith(".xml"); - } - catch (IOException e) - { - LOG.warn("Cannot accept", e); - return false; - } + // else is it a context XML file + return lowerName.endsWith(".xml"); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 3bbc7f49ed71..244ae2ad6338 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -328,6 +328,25 @@ public PathResource(URL url) throws IOException, URISyntaxException this(url.toURI()); } + @Override + public boolean isSame(Resource resource) + { + try + { + if (resource instanceof PathResource) + { + Path path = ((PathResource)resource).getPath(); + return Files.isSameFile(getPath(), path); + } + } + catch (IOException e) + { + if (LOG.isDebugEnabled()) + LOG.debug("ignored", e); + } + return false; + } + @Override public Resource addPath(final String subpath) throws IOException { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 4d93a0aff0dd..a45421a2a2ac 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -302,6 +302,18 @@ public static boolean isContainedIn(Resource r, Resource containingResource) thr public abstract boolean isContainedIn(Resource r) throws MalformedURLException; + /** + * Return true if the passed Resource represents the same resource as the Resource. + * For many resource types, this is equivalent to {@link #equals(Object)}, however + * for resources types that support aliasing, this maybe some other check (e.g. {@link java.nio.file.Files#isSameFile(Path, Path)}). + * @param resource The resource to check + * @return true if the passed resource represents the same resource. + */ + public boolean isSame(Resource resource) + { + return equals(resource); + } + /** * Release any temporary resources held by the resource. */ diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 9d66554538cc..a4d9666b62e8 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -21,11 +21,13 @@ import java.nio.channels.ReadableByteChannel; import java.nio.file.FileSystem; import java.nio.file.FileSystems; +import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -113,4 +115,32 @@ public void testDefaultFileSystemGetFile() throws Exception File file = resource.getFile(); assertThat("File for default FileSystem", file, is(exampleJar.toFile())); } + + @Test + public void testSame() + { + Path rpath = MavenTestingUtils.getTestResourcePathFile("resource.txt"); + Path epath = MavenTestingUtils.getTestResourcePathFile("example.jar"); + PathResource rPathResource = new PathResource(rpath); + PathResource ePathResource = new PathResource(epath); + + assertThat(rPathResource.isSame(rPathResource), Matchers.is(true)); + assertThat(rPathResource.isSame(ePathResource), Matchers.is(false)); + + PathResource ePathResource2 = null; + try + { + Path epath2 = Files.createSymbolicLink(MavenTestingUtils.getTargetPath().resolve("testSame-symlink"), epath.getParent()).resolve("example.jar"); + ePathResource2 = new PathResource(epath2); + } + catch (Throwable th) + { + // Assume symbolic links are not supported + } + if (ePathResource2 != null) + { + assertThat(ePathResource.isSame(ePathResource2), Matchers.is(true)); + assertThat(ePathResource.equals(ePathResource2), Matchers.is(false)); + } + } }