From e3faf81860073f5adabb73ed46c74715e0b1ee8e Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 4 Jun 2021 23:10:54 +1000 Subject: [PATCH] Fix #6114 Deploy symlink webapps (#6317) * Fix #6114 Deploy symlink webapps Use Path.toRealPath rather than getCanonicalPath in the Scanner Make following symlinks configurable Signed-off-by: Greg Wilkins --- .../deploy/providers/ScanningAppProvider.java | 19 +- .../deploy/providers/WebAppProvider.java | 4 +- .../deploy/providers/WebAppProviderTest.java | 16 +- .../src/test/resources/jetty-deploy-wars.xml | 1 + .../java/org/eclipse/jetty/util/Scanner.java | 188 +++++++++++++----- 5 files changed, 162 insertions(+), 66 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 eb66704f59d..a4263442c1b 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 @@ -48,6 +48,7 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements private final List _monitored = new CopyOnWriteArrayList<>(); private int _scanInterval = 10; private Scanner _scanner; + private boolean _useRealPaths; private final Scanner.DiscreteListener _scannerListener = new Scanner.DiscreteListener() { @@ -81,6 +82,22 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements addBean(_appMap); } + /** + * @return True if the real path of the scanned files should be used for deployment. + */ + public boolean isUseRealPaths() + { + return _useRealPaths; + } + + /** + * @param useRealPaths True if the real path of the scanned files should be used for deployment. + */ + public void setUseRealPaths(boolean useRealPaths) + { + _useRealPaths = useRealPaths; + } + protected void setFilenameFilter(FilenameFilter filter) { if (isRunning()) @@ -128,7 +145,7 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements LOG.warn("Does not exist: {}", resource); } - _scanner = new Scanner(); + _scanner = new Scanner(null, _useRealPaths); _scanner.setScanDirs(files); _scanner.setScanInterval(_scanInterval); _scanner.setFilenameFilter(_filenameFilter); 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 daf54df9475..24c6958c667 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 @@ -375,7 +375,7 @@ public class WebAppProvider extends ScanningAppProvider { //if a .xml file exists for it, then redeploy that instead File xml = new File(parent, xmlname); - super.fileChanged(xml.getCanonicalPath()); + super.fileChanged(xml.getPath()); return; } @@ -384,7 +384,7 @@ public class WebAppProvider extends ScanningAppProvider { //if a .XML file exists for it, then redeploy that instead File xml = new File(parent, xmlname); - super.fileChanged(xml.getCanonicalPath()); + super.fileChanged(xml.getPath()); return; } diff --git a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/WebAppProviderTest.java b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/WebAppProviderTest.java index eb6684de9f2..90a687b9663 100644 --- a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/WebAppProviderTest.java +++ b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/WebAppProviderTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.deploy.providers; import java.io.File; -import java.net.URL; import java.nio.file.FileSystemException; import java.nio.file.Files; import java.nio.file.Path; @@ -35,7 +34,6 @@ import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.webapp.WebAppContext; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.extension.ExtendWith; @@ -45,7 +43,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.junit.jupiter.api.condition.OS.LINUX; -import static org.junit.jupiter.api.condition.OS.MAC; @ExtendWith(WorkDirExtension.class) public class WebAppProviderTest @@ -68,10 +65,13 @@ public class WebAppProviderTest // Make symlink Path pathWar3 = MavenTestingUtils.getTestResourcePathFile("webapps/foo-webapp-3.war"); + Path pathFoo = jetty.getJettyDir("webapps/foo.war").toPath(); Path pathBar = jetty.getJettyDir("webapps/bar.war").toPath(); + Path pathBob = jetty.getJettyDir("webapps/bob.war").toPath(); try { Files.createSymbolicLink(pathBar, pathWar3); + Files.createSymbolicLink(pathBob, pathFoo); symlinkSupported = true; } catch (UnsupportedOperationException | FileSystemException e) @@ -95,12 +95,11 @@ public class WebAppProviderTest jetty.stop(); } - @Disabled("See issue #1200") @Test public void testStartupContext() { // Check Server for Handlers - jetty.assertWebAppContextsExists("/bar", "/foo"); + jetty.assertWebAppContextsExists("/bar", "/foo", "/bob"); File workDir = jetty.getJettyDir("workish"); @@ -109,10 +108,9 @@ public class WebAppProviderTest assertDirNotExists("root of work directory", workDir, "jsp"); // Test for correct behaviour - assertTrue(hasJettyGeneratedPath(workDir, "foo.war"), "Should have generated directory in work directory: " + workDir); + assertTrue(hasJettyGeneratedPath(workDir, "foo_war"), "Should have generated directory in work directory: " + workDir); } - @Disabled("See issue #1200") @Test public void testStartupSymlinkContext() { @@ -124,11 +122,11 @@ public class WebAppProviderTest assertTrue(barLink.isFile(), "bar.war link isFile: " + barLink.toString()); // Check Server for expected Handlers - jetty.assertWebAppContextsExists("/bar", "/foo"); + jetty.assertWebAppContextsExists("/bar", "/foo", "/bob"); // Test for expected work/temp directory behaviour File workDir = jetty.getJettyDir("workish"); - assertTrue(hasJettyGeneratedPath(workDir, "bar.war"), "Should have generated directory in work directory: " + workDir); + assertTrue(hasJettyGeneratedPath(workDir, "bar_war"), "Should have generated directory in work directory: " + workDir); } @Test diff --git a/jetty-deploy/src/test/resources/jetty-deploy-wars.xml b/jetty-deploy/src/test/resources/jetty-deploy-wars.xml index 50c192bdb07..8730443711b 100644 --- a/jetty-deploy/src/test/resources/jetty-deploy-wars.xml +++ b/jetty-deploy/src/test/resources/jetty-deploy-wars.xml @@ -18,6 +18,7 @@ /webapps 1 /workish + false diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java index 4a00309ee9b..38df54c9abe 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java @@ -20,6 +20,7 @@ import java.nio.file.FileVisitOption; import java.nio.file.FileVisitResult; import java.nio.file.FileVisitor; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.PathMatcher; import java.nio.file.attribute.BasicFileAttributes; @@ -35,6 +36,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; +import java.util.stream.Collectors; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; @@ -47,6 +49,8 @@ import org.slf4j.LoggerFactory; * * Utility for scanning a directory for added, removed and changed * files and reporting these events via registered Listeners. + * The scanner operates on the {@link Path#toRealPath(LinkOption...)} of the files scanned and + * can be configured to follow symlinks. */ public class Scanner extends ContainerLifeCycle { @@ -63,7 +67,7 @@ public class Scanner extends ContainerLifeCycle private int _scanInterval; private final AtomicInteger _scanCount = new AtomicInteger(0); private final List _listeners = new CopyOnWriteArrayList<>(); - private Map _prevScan; + private Map _prevScan; private FilenameFilter _filter; private final Map> _scannables = new ConcurrentHashMap<>(); private boolean _reportExisting = true; @@ -71,6 +75,7 @@ public class Scanner extends ContainerLifeCycle private Scheduler.Task _task; private final Scheduler _scheduler; private int _scanDepth = DEFAULT_SCAN_DEPTH; + private final LinkOption[] _linkOptions; private enum Status { @@ -150,11 +155,11 @@ public class Scanner extends ContainerLifeCycle */ private class Visitor implements FileVisitor { - Map scanInfoMap; + Map scanInfoMap; IncludeExcludeSet rootIncludesExcludes; Path root; - public Visitor(Path root, IncludeExcludeSet rootIncludesExcludes, Map scanInfoMap) + private Visitor(Path root, IncludeExcludeSet rootIncludesExcludes, Map scanInfoMap) { this.root = root; this.rootIncludesExcludes = rootIncludesExcludes; @@ -167,10 +172,11 @@ public class Scanner extends ContainerLifeCycle if (!Files.exists(dir)) return FileVisitResult.SKIP_SUBTREE; + dir = dir.toRealPath(_linkOptions); File f = dir.toFile(); //if we want to report directories and we haven't already seen it - if (_reportDirs && !scanInfoMap.containsKey(f.getCanonicalPath())) + if (_reportDirs && !scanInfoMap.containsKey(dir)) { boolean accepted = false; if (rootIncludesExcludes != null && !rootIncludesExcludes.isEmpty()) @@ -186,7 +192,7 @@ public class Scanner extends ContainerLifeCycle if (accepted) { - scanInfoMap.put(f.getCanonicalPath(), new MetaData(f.lastModified(), f.isDirectory() ? 0 : f.length())); + scanInfoMap.put(dir, new MetaData(f.lastModified(), f.isDirectory() ? 0 : f.length())); if (LOG.isDebugEnabled()) LOG.debug("scan accepted dir {} mod={}", f, f.lastModified()); } } @@ -195,20 +201,22 @@ public class Scanner extends ContainerLifeCycle } @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException + public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException { - if (!Files.exists(file)) + path = path.toRealPath(_linkOptions); + + if (!Files.exists(path)) return FileVisitResult.CONTINUE; - File f = file.toFile(); + File f = path.toFile(); boolean accepted = false; - if (f.isFile() || (f.isDirectory() && _reportDirs && !scanInfoMap.containsKey(f.getCanonicalPath()))) + if (f.isFile() || (f.isDirectory() && _reportDirs && !scanInfoMap.containsKey(path))) { if (rootIncludesExcludes != null && !rootIncludesExcludes.isEmpty()) { //accepted if not explicitly excluded and either is explicitly included or there are no explicit inclusions - accepted = rootIncludesExcludes.test(file); + accepted = rootIncludesExcludes.test(path); } else if (_filter == null || _filter.accept(f.getParentFile(), f.getName())) accepted = true; @@ -216,7 +224,7 @@ public class Scanner extends ContainerLifeCycle if (accepted) { - scanInfoMap.put(f.getCanonicalPath(), new MetaData(f.lastModified(), f.isDirectory() ? 0 : f.length())); + scanInfoMap.put(path, new MetaData(f.lastModified(), f.isDirectory() ? 0 : f.length())); if (LOG.isDebugEnabled()) LOG.debug("scan accepted {} mod={}", f, f.lastModified()); } @@ -251,10 +259,62 @@ public class Scanner extends ContainerLifeCycle */ public interface DiscreteListener extends Listener { + /** + * Called when a file is changed. + * Default implementation calls {@link #fileChanged(String)}. + * @param path the {@link Path#toRealPath(LinkOption...)} of the changed file + * @throws Exception May be thrown for handling errors + */ + default void pathChanged(Path path) throws Exception + { + path.toString(); + fileChanged(path.toString()); + } + + /** + * Called when a file is added. + * Default implementation calls {@link #fileAdded(String)}. + * @param path the {@link Path#toRealPath(LinkOption...)} of the added file + * @throws Exception May be thrown for handling errors + */ + default void pathAdded(Path path) throws Exception + { + fileAdded(path.toString()); + } + + /** + * Called when a file is removed. + * Default implementation calls {@link #fileRemoved(String)}. + * @param path the {@link Path#toRealPath(LinkOption...)} of the removed file + * @throws Exception May be thrown for handling errors + */ + default void pathRemoved(Path path) throws Exception + { + fileRemoved(path.toString()); + } + + /** + * Called when a file is changed. + * May not be called if {@link #pathChanged(Path)} is overridden. + * @param filename the {@link Path#toRealPath(LinkOption...)} as a string of the changed file + * @throws Exception May be thrown for handling errors + */ void fileChanged(String filename) throws Exception; + /** + * Called when a file is added. + * May not be called if {@link #pathAdded(Path)} is overridden. + * @param filename the {@link Path#toRealPath(LinkOption...)} as a string of the added file + * @throws Exception May be thrown for handling errors + */ void fileAdded(String filename) throws Exception; + /** + * Called when a file is removed. + * May not be called if {@link #pathRemoved(Path)} is overridden. + * @param filename the {@link Path#toRealPath(LinkOption...)} as a string of the removed file + * @throws Exception May be thrown for handling errors + */ void fileRemoved(String filename) throws Exception; } @@ -263,7 +323,12 @@ public class Scanner extends ContainerLifeCycle */ public interface BulkListener extends Listener { - public void filesChanged(Set filenames) throws Exception; + default void pathsChanged(Set paths) throws Exception + { + filesChanged(paths.stream().map(Path::toString).collect(Collectors.toSet())); + } + + void filesChanged(Set filenames) throws Exception; } /** @@ -282,14 +347,24 @@ public class Scanner extends ContainerLifeCycle public Scanner() { - this(new ScheduledExecutorScheduler("Scanner-" + SCANNER_IDS.getAndIncrement(), true, 1)); + this(null); } public Scanner(Scheduler scheduler) + { + this(scheduler, true); + } + + /** + * @param scheduler The scheduler to use for scanning. + * @param reportRealPaths If true, the {@link Listener}s are called with the real path of scanned files. + */ + public Scanner(Scheduler scheduler, boolean reportRealPaths) { //Create the scheduler and start it - _scheduler = scheduler; + _scheduler = scheduler == null ? new ScheduledExecutorScheduler("Scanner-" + SCANNER_IDS.getAndIncrement(), true, 1) : scheduler; addBean(_scheduler); + _linkOptions = reportRealPaths ? new LinkOption[0] : new LinkOption[] {LinkOption.NOFOLLOW_LINKS}; } /** @@ -335,21 +410,24 @@ public class Scanner extends ContainerLifeCycle /** * Add a file to be scanned. The file must not be null, and must exist. * - * @param p the Path of the file to scan. + * @param path the Path of the file to scan. */ - public void addFile(Path p) + public void addFile(Path path) { if (isRunning()) throw new IllegalStateException("Scanner started"); - if (p == null) + if (path == null) throw new IllegalStateException("Null path"); - if (!Files.exists(p) || Files.isDirectory(p)) - throw new IllegalStateException("Not file or doesn't exist: " + p); try { - _scannables.putIfAbsent(p.toRealPath(), new IncludeExcludeSet<>(PathMatcherSet.class)); + // Always follow links when check ultimate type of the path + Path real = path.toRealPath(); + if (!Files.exists(real) || Files.isDirectory(real)) + throw new IllegalStateException("Not file or doesn't exist: " + path); + + _scannables.putIfAbsent(real, new IncludeExcludeSet<>(PathMatcherSet.class)); } catch (IOException e) { @@ -371,13 +449,15 @@ public class Scanner extends ContainerLifeCycle if (p == null) throw new IllegalStateException("Null path"); - if (!Files.exists(p) || !Files.isDirectory(p)) - throw new IllegalStateException("Not directory or doesn't exist: " + p); - try { + // Check status of the real path + Path real = p.toRealPath(); + if (!Files.exists(real) || !Files.isDirectory(real)) + throw new IllegalStateException("Not directory or doesn't exist: " + p); + IncludeExcludeSet includesExcludes = new IncludeExcludeSet<>(PathMatcherSet.class); - IncludeExcludeSet prev = _scannables.putIfAbsent(p.toRealPath(), includesExcludes); + IncludeExcludeSet prev = _scannables.putIfAbsent(real, includesExcludes); if (prev != null) includesExcludes = prev; return includesExcludes; @@ -625,7 +705,7 @@ public class Scanner extends ContainerLifeCycle { int cycle = _scanCount.incrementAndGet(); reportScanStart(cycle); - Map currentScan = scanFiles(); + Map currentScan = scanFiles(); reportDifferences(currentScan, _prevScan == null ? Collections.emptyMap() : Collections.unmodifiableMap(_prevScan)); _prevScan = currentScan; reportScanEnd(cycle); @@ -634,9 +714,9 @@ public class Scanner extends ContainerLifeCycle /** * Scan all of the given paths. */ - private Map scanFiles() + private Map scanFiles() { - Map currentScan = new HashMap<>(); + Map currentScan = new HashMap<>(); for (Map.Entry> entry : _scannables.entrySet()) { try @@ -660,20 +740,20 @@ public class Scanner extends ContainerLifeCycle * @param currentScan the info from the most recent pass * @param oldScan info from the previous pass */ - private void reportDifferences(Map currentScan, Map oldScan) + private void reportDifferences(Map currentScan, Map oldScan) { - Map changes = new HashMap<>(); + Map changes = new HashMap<>(); //Handle deleted files - Set oldScanKeys = new HashSet<>(oldScan.keySet()); + Set oldScanKeys = new HashSet<>(oldScan.keySet()); oldScanKeys.removeAll(currentScan.keySet()); - for (String file : oldScanKeys) + for (Path path : oldScanKeys) { - changes.put(file, Notification.REMOVED); + changes.put(path, Notification.REMOVED); } // Handle new and changed files - for (Map.Entry entry : currentScan.entrySet()) + for (Map.Entry entry : currentScan.entrySet()) { MetaData current = entry.getValue(); MetaData previous = oldScan.get(entry.getKey()); @@ -714,7 +794,7 @@ public class Scanner extends ContainerLifeCycle LOG.debug("scanned {}", _scannables.keySet()); //Call the DiscreteListeners - for (Map.Entry entry : changes.entrySet()) + for (Map.Entry entry : changes.entrySet()) { switch (entry.getValue()) { @@ -736,28 +816,28 @@ public class Scanner extends ContainerLifeCycle reportBulkChanges(changes.keySet()); } - private void warn(Object listener, String filename, Throwable th) + private void warn(Object listener, Path path, Throwable th) { - LOG.warn("{} failed on '{}'", listener, filename, th); + LOG.warn("{} failed on '{}'", listener, path, th); } /** * Report a file addition to the registered FileAddedListeners * - * @param filename the filename + * @param path the path */ - private void reportAddition(String filename) + private void reportAddition(Path path) { for (Listener l : _listeners) { try { if (l instanceof DiscreteListener) - ((DiscreteListener)l).fileAdded(filename); + ((DiscreteListener)l).pathAdded(path); } catch (Throwable e) { - warn(l, filename, e); + warn(l, path, e); } } } @@ -765,20 +845,20 @@ public class Scanner extends ContainerLifeCycle /** * Report a file removal to the FileRemovedListeners * - * @param filename the filename + * @param path the path of the removed filename */ - private void reportRemoval(String filename) + private void reportRemoval(Path path) { for (Object l : _listeners) { try { if (l instanceof DiscreteListener) - ((DiscreteListener)l).fileRemoved(filename); + ((DiscreteListener)l).pathRemoved(path); } catch (Throwable e) { - warn(l, filename, e); + warn(l, path, e); } } } @@ -786,11 +866,11 @@ public class Scanner extends ContainerLifeCycle /** * Report a file change to the FileChangedListeners * - * @param filename the filename + * @param path the path of the changed file */ - private void reportChange(String filename) + private void reportChange(Path path) { - if (filename == null) + if (path == null) return; for (Listener l : _listeners) @@ -798,11 +878,11 @@ public class Scanner extends ContainerLifeCycle try { if (l instanceof DiscreteListener) - ((DiscreteListener)l).fileChanged(filename); + ((DiscreteListener)l).pathChanged(path); } catch (Throwable e) { - warn(l, filename, e); + warn(l, path, e); } } } @@ -810,11 +890,11 @@ public class Scanner extends ContainerLifeCycle /** * Report the list of filenames for which changes were detected. * - * @param filenames names of all files added/changed/removed + * @param paths The paths of all files added/changed/removed */ - private void reportBulkChanges(Set filenames) + private void reportBulkChanges(Set paths) { - if (filenames == null || filenames.isEmpty()) + if (paths == null || paths.isEmpty()) return; for (Listener l : _listeners) @@ -822,11 +902,11 @@ public class Scanner extends ContainerLifeCycle try { if (l instanceof BulkListener) - ((BulkListener)l).filesChanged(filenames); + ((BulkListener)l).pathsChanged(paths); } catch (Throwable e) { - warn(l, filenames.toString(), e); + LOG.warn("{} failed on '{}'", l, paths, e); } } }