From 37fffb1722604da1763d8a096ec5c5fb41ea0633 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 23 Mar 2021 19:18:40 -0500 Subject: [PATCH] Merge pull request from GHSA-j6qj-j888-vvgq Ensure that WebAppProvider Filter always canonicalises the file passed in from the Scanner. Thus, both the monitored directory is canonical as well as the file it is being compared against. Signed-off-by: Greg Wilkins Co-authored-by: Greg Wilkins --- .../deploy/providers/ScanningAppProvider.java | 24 ++++ .../deploy/providers/WebAppProvider.java | 82 ++++++++----- .../deploy/providers/WebAppProviderTest.java | 114 +++++++++++++++++- .../jetty/deploy/test/XmlConfiguredJetty.java | 96 ++++++++------- 4 files changed, 238 insertions(+), 78 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 8ecfa190b49f..bb75940f7950 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 @@ -20,6 +20,7 @@ import java.io.File; import java.io.FilenameFilter; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -54,6 +55,7 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A private DeploymentManager _deploymentManager; protected FilenameFilter _filenameFilter; private final List _monitored = new CopyOnWriteArrayList<>(); + private final List _canonicalMonitored = new CopyOnWriteArrayList<>(); private boolean _recursive = false; private int _scanInterval = 10; private Scanner _scanner; @@ -247,12 +249,34 @@ 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 + LOG.ignore(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 350580241373..a1b4a566df2e 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 @@ -20,6 +20,7 @@ import java.io.File; import java.io.FilenameFilter; +import java.io.IOException; import java.util.Locale; import org.eclipse.jetty.deploy.App; @@ -30,6 +31,8 @@ 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; @@ -62,6 +65,8 @@ @ManagedObject("Provider for start-up deployement of webapps based on presence in directory") public class WebAppProvider extends ScanningAppProvider { + private static final Logger LOG = Log.getLogger(WebAppProvider.class); + private boolean _extractWars = false; private boolean _parentLoaderPriority = false; private ConfigurationManager _configurationManager; @@ -74,51 +79,62 @@ public class Filter implements FilenameFilter @Override public boolean accept(File dir, String name) { - if (!dir.exists()) - { + if (dir == null) return false; - } - String lowername = name.toLowerCase(Locale.ENGLISH); - File file = new File(dir, name); - Resource r = Resource.newResource(file); - if (getMonitoredResources().contains(r) && r.isDirectory()) + try { - return false; - } - - // ignore hidden files - if (lowername.startsWith(".")) - return false; - - // Ignore some directories - if (file.isDirectory()) - { - // is it a nominated config directory - if (lowername.endsWith(".d")) + //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()) return false; + + String lowerName = name.toLowerCase(Locale.ENGLISH); - // is it an unpacked directory for an existing war file? - if (exists(name + ".war") || exists(name + ".WAR")) + File canonical = new File(dir, name).getCanonicalFile(); + Resource r = Resource.newResource(canonical); + if (getCanonicalMonitoredResources().contains(r) && r.isDirectory()) return false; - // is it a directory for an existing xml file? - if (exists(name + ".xml") || exists(name + ".XML")) + // ignore hidden files + if (lowerName.startsWith(".")) return false; - //is it a sccs dir? - return !"cvs".equals(lowername) && !"cvsroot".equals(lowername); // OK to deploy it then - } + // Ignore some directories + if (canonical.isDirectory()) + { + // is it a nominated config directory + if (lowerName.endsWith(".d")) + return false; - // else is it a war file - if (lowername.endsWith(".war")) + // 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 + } + + // else is it a war file + if (lowerName.endsWith(".war")) + { + //defer deployment decision to fileChanged() + return true; + } + + // else is it a context XML file + return lowerName.endsWith(".xml"); + } + catch (IOException e) { - //defer deployment decision to fileChanged() - return true; + LOG.warn(e); + return false; } - - // else is it a context XML file - return lowername.endsWith(".xml"); } } 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 931b08e0267a..67a296d55a07 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 @@ -19,26 +19,35 @@ 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; import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.eclipse.jetty.deploy.test.XmlConfiguredJetty; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.IO; +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.extension.ExtendWith; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; -@Disabled("See issue #1200") @ExtendWith(WorkDirExtension.class) public class WebAppProviderTest { @@ -49,7 +58,8 @@ public class WebAppProviderTest @BeforeEach public void setupEnvironment() throws Exception { - jetty = new XmlConfiguredJetty(testdir.getEmptyPathDir()); + Path p = testdir.getEmptyPathDir(); + jetty = new XmlConfiguredJetty(p); jetty.addConfiguration("jetty.xml"); jetty.addConfiguration("jetty-http.xml"); jetty.addConfiguration("jetty-deploy-wars.xml"); @@ -86,6 +96,7 @@ public void teardownEnvironment() throws Exception jetty.stop(); } + @Disabled("See issue #1200") @Test public void testStartupContext() { @@ -101,7 +112,8 @@ public void testStartupContext() // Test for correct behaviour assertTrue(hasJettyGeneratedPath(workDir, "foo.war"), "Should have generated directory in work directory: " + workDir); } - + + @Disabled("See issue #1200") @Test public void testStartupSymlinkContext() { @@ -119,7 +131,103 @@ public void testStartupSymlinkContext() File workDir = jetty.getJettyDir("workish"); assertTrue(hasJettyGeneratedPath(workDir, "bar.war"), "Should have generated directory in work directory: " + workDir); } + + @Test + public void testWebappSymlinkDir() throws Exception + { + jetty.stop(); //reconfigure jetty + + testdir.ensureEmpty(); + + jetty = new XmlConfiguredJetty(testdir.getEmptyPathDir()); + jetty.addConfiguration("jetty.xml"); + jetty.addConfiguration("jetty-http.xml"); + jetty.addConfiguration("jetty-deploy-wars.xml"); + + assumeTrue(symlinkSupported); + + //delete the existing webapps directory + File webapps = jetty.getJettyDir("webapps"); + assertTrue(IO.delete(webapps)); + + //make a different directory to contain webapps + File x = jetty.getJettyDir("x"); + Files.createDirectory(x.toPath()); + + //Put a webapp into it + File srcDir = MavenTestingUtils.getTestResourceDir("webapps"); + File fooWar = new File(x, "foo.war"); + IO.copy(new File(srcDir, "foo-webapp-1.war"), fooWar); + assertTrue(Files.exists(fooWar.toPath())); + + //make a link from x to webapps + Files.createSymbolicLink(jetty.getJettyDir("webapps").toPath(), x.toPath()); + assertTrue(Files.exists(jetty.getJettyDir("webapps").toPath())); + + jetty.load(); + jetty.start(); + + //only webapp in x should be deployed, not x itself + jetty.assertWebAppContextsExists("/foo"); + } + + @Test + public void testBaseDirSymlink() throws Exception + { + jetty.stop(); //reconfigure jetty + + testdir.ensureEmpty(); + + Path realBase = testdir.getEmptyPathDir(); + + //set jetty up on the real base + jetty = new XmlConfiguredJetty(realBase); + jetty.addConfiguration("jetty.xml"); + jetty.addConfiguration("jetty-http.xml"); + jetty.addConfiguration("jetty-deploy-wars.xml"); + + //Put a webapp into the base + jetty.copyWebapp("foo-webapp-1.war", "foo.war"); + //create the jetty structure + jetty.load(); + jetty.start(); + Path jettyHome = jetty.getJettyHome().toPath(); + + jetty.stop(); + + //Make a symbolic link to the real base + File testsDir = MavenTestingUtils.getTargetTestingDir(); + Path symlinkBase = Files.createSymbolicLink(testsDir.toPath().resolve("basedirsymlink-" + System.currentTimeMillis()), jettyHome); + Map properties = new HashMap<>(); + properties.put("jetty.home", jettyHome.toString()); + //Start jetty, but this time running from the symlinked base + System.setProperty("jetty.home", properties.get("jetty.home")); + + List configurations = jetty.getConfigurations(); + Server server = XmlConfiguredJetty.loadConfigurations(configurations, properties); + + try + { + server.start(); + HandlerCollection handlers = (HandlerCollection)server.getHandler(); + Handler[] children = server.getChildHandlersByClass(WebAppContext.class); + assertEquals(1, children.length); + assertEquals("/foo", ((WebAppContext)children[0]).getContextPath()); + } + finally + { + server.stop(); + } + } + + private Map setupJettyProperties(Path jettyHome) + { + Map properties = new HashMap<>(); + properties.put("jetty.home", jettyHome.toFile().getAbsolutePath()); + return properties; + } + private static boolean hasJettyGeneratedPath(File basedir, String expectedWarFilename) { File[] paths = basedir.listFiles(); diff --git a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java index 326095f6c5ab..26adfc405d88 100644 --- a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java +++ b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java @@ -31,6 +31,7 @@ import java.net.UnknownHostException; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -69,6 +70,51 @@ public class XmlConfiguredJetty private String _scheme = HttpScheme.HTTP.asString(); private File _jettyHome; + public static Server loadConfigurations(List configurations, Map properties) + throws Exception + { + XmlConfiguration last = null; + Object[] obj = new Object[configurations.size()]; + + // Configure everything + for (int i = 0; i < configurations.size(); i++) + { + URL configURL = configurations.get(i); + XmlConfiguration configuration = new XmlConfiguration(configURL); + if (last != null) + configuration.getIdMap().putAll(last.getIdMap()); + configuration.getProperties().putAll(properties); + obj[i] = configuration.configure(); + last = configuration; + } + + // Test for Server Instance. + Server foundServer = null; + int serverCount = 0; + for (int i = 0; i < configurations.size(); i++) + { + if (obj[i] instanceof Server) + { + if (obj[i].equals(foundServer)) + { + // Identical server instance found + break; + } + foundServer = (Server)obj[i]; + serverCount++; + } + } + + if (serverCount <= 0) + { + throw new Exception("Load failed to configure a " + Server.class.getName()); + } + + assertEquals(1, serverCount, "Server load count"); + + return foundServer; + } + public XmlConfiguredJetty(Path testdir) throws IOException { _xmlConfigurations = new ArrayList<>(); @@ -77,11 +123,11 @@ public XmlConfiguredJetty(Path testdir) throws IOException String jettyHomeBase = testdir.toString(); // Ensure we have a new (pristene) directory to work with. int idx = 0; - _jettyHome = new File(jettyHomeBase + "#" + idx); + _jettyHome = new File(jettyHomeBase + "--" + idx); while (_jettyHome.exists()) { idx++; - _jettyHome = new File(jettyHomeBase + "#" + idx); + _jettyHome = new File(jettyHomeBase + "--" + idx); } deleteContents(_jettyHome); // Prepare Jetty.Home (Test) dir @@ -152,6 +198,11 @@ public void addConfiguration(URL xmlConfig) { _xmlConfigurations.add(xmlConfig); } + + public List getConfigurations() + { + return Collections.unmodifiableList(_xmlConfigurations); + } public void assertNoWebAppContexts() { @@ -325,46 +376,7 @@ public List getWebAppContexts() public void load() throws Exception { - XmlConfiguration last = null; - Object[] obj = new Object[this._xmlConfigurations.size()]; - - // Configure everything - for (int i = 0; i < this._xmlConfigurations.size(); i++) - { - URL configURL = this._xmlConfigurations.get(i); - XmlConfiguration configuration = new XmlConfiguration(configURL); - if (last != null) - configuration.getIdMap().putAll(last.getIdMap()); - configuration.getProperties().putAll(_properties); - obj[i] = configuration.configure(); - last = configuration; - } - - // Test for Server Instance. - Server foundServer = null; - int serverCount = 0; - for (int i = 0; i < this._xmlConfigurations.size(); i++) - { - if (obj[i] instanceof Server) - { - if (obj[i].equals(foundServer)) - { - // Identical server instance found - break; - } - foundServer = (Server)obj[i]; - serverCount++; - } - } - - if (serverCount <= 0) - { - throw new Exception("Load failed to configure a " + Server.class.getName()); - } - - assertEquals(1, serverCount, "Server load count"); - - this._server = foundServer; + this._server = loadConfigurations(_xmlConfigurations, _properties); this._server.setStopTimeout(10); }