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 | 84 +++++++------ .../deploy/providers/WebAppProviderTest.java | 114 +++++++++++++++++- .../jetty/deploy/test/XmlConfiguredJetty.java | 96 ++++++++------- 4 files changed, 239 insertions(+), 79 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 8ecfa190b49..bb75940f795 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 @@ package org.eclipse.jetty.deploy.providers; 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 abstract class ScanningAppProvider extends AbstractLifeCycle implements A { _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 35058024137..a1b4a566df2 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 @@ package org.eclipse.jetty.deploy.providers; 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.server.handler.ContextHandler; 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 @@ import org.eclipse.jetty.xml.XmlConfiguration; @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 WebAppProvider extends ScanningAppProvider @Override public boolean accept(File dir, String name) { - if (!dir.exists()) - { - 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()) - { - return false; - } - - // ignore hidden files - if (lowername.startsWith(".")) + if (dir == null) return false; - // Ignore some directories - if (file.isDirectory()) + try { - // 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); + + File canonical = new File(dir, name).getCanonicalFile(); + Resource r = Resource.newResource(canonical); + if (getCanonicalMonitoredResources().contains(r) && r.isDirectory()) return false; - // is it an unpacked directory for an existing war file? - if (exists(name + ".war") || exists(name + ".WAR")) + // ignore hidden files + if (lowerName.startsWith(".")) return false; - // 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 a sccs dir? - return !"cvs".equals(lowername) && !"cvsroot".equals(lowername); // OK to deploy it then + // 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"); } - - // else is it a war file - if (lowername.endsWith(".war")) + 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 931b08e0267..67a296d55a0 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 class WebAppProviderTest jetty.stop(); } + @Disabled("See issue #1200") @Test public void testStartupContext() { @@ -101,7 +112,8 @@ public class WebAppProviderTest // 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 class WebAppProviderTest 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 326095f6c5a..26adfc405d8 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.URLConnection; 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 class XmlConfiguredJetty 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 class XmlConfiguredJetty { _xmlConfigurations.add(xmlConfig); } + + public List getConfigurations() + { + return Collections.unmodifiableList(_xmlConfigurations); + } public void assertNoWebAppContexts() { @@ -325,46 +376,7 @@ public class XmlConfiguredJetty 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); }