Improve WebAppProvider scanning (#6090)

* Cherry pick from jetty-9

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.

jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java

@@ -15,6 +15,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;
@@ -45,6 +46,7 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements
     private DeploymentManager _deploymentManager;
     private FilenameFilter _filenameFilter;
     private final List<Resource> _monitored = new CopyOnWriteArrayList<>();
+    private final List<Resource> _canonicalMonitored = new CopyOnWriteArrayList<>();
     private int _scanInterval = 10;
     private Scanner _scanner;
 
@@ -236,6 +238,24 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements
     {
         _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<Resource> getMonitoredResources()
@@ -243,6 +263,11 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements
         return Collections.unmodifiableList(_monitored);
     }
     
+    List<Resource> getCanonicalMonitoredResources()
+    {
+        return Collections.unmodifiableList(_canonicalMonitored);
+    }
+
     public void setMonitoredDirResource(Resource resource)
     {
         setMonitoredResources(Collections.singletonList(resource));

jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java

@@ -15,6 +15,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;
@@ -25,9 +26,12 @@ 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;
+import org.slf4j.LoggerFactory;
 
 /**
  * The webapps directory scanning provider.
@@ -57,6 +61,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 org.slf4j.Logger LOG = LoggerFactory.getLogger(WebAppProvider.class);
+
     private boolean _extractWars = false;
     private boolean _parentLoaderPriority = false;
     private ConfigurationManager _configurationManager;
@@ -69,28 +75,33 @@ public class WebAppProvider extends ScanningAppProvider
         @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);
+            try
-            Resource r = Resource.newResource(file);
-            if (getMonitoredResources().contains(r) && r.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())
+                    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;
-            }
 
                 // ignore hidden files
-            if (lowername.startsWith("."))
+                if (lowerName.startsWith("."))
                     return false;
 
                 // Ignore some directories
-            if (file.isDirectory())
+                if (canonical.isDirectory())
                 {
                     // is it a nominated config directory
-                if (lowername.endsWith(".d"))
+                    if (lowerName.endsWith(".d"))
                         return false;
 
                     // is it an unpacked directory for an existing war file?
@@ -102,27 +113,26 @@ public class WebAppProvider extends ScanningAppProvider
                         return false;
 
                     //is it a sccs dir?
-                if ("cvs".equals(lowername) || "cvsroot".equals(lowername))
+                    return !"cvs".equals(lowerName) && !"cvsroot".equals(lowerName); // OK to deploy it then
-                    return false;
-
-                // OK to deploy it then
-                return true;
                 }
 
                 // else is it a war file
-            if (lowername.endsWith(".war"))
+                if (lowerName.endsWith(".war"))
                 {
                     //defer deployment decision to fileChanged()
                     return true;
                 }
 
                 // else is it a context XML file 
-            if (lowername.endsWith(".xml"))
+                return lowerName.endsWith(".xml");
-                return true;
+            }
-
+            catch (IOException e)
+            {
+                LOG.warn("Cannot accept", e);
                 return false;
             }
         }
+    }
 
     public WebAppProvider()
     {

jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/WebAppProviderTest.java

@@ -14,26 +14,36 @@
 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.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.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
 {
@@ -44,7 +54,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");
@@ -81,6 +92,7 @@ public class WebAppProviderTest
         jetty.stop();
     }
 
+    @Disabled("See issue #1200")
     @Test
     public void testStartupContext()
     {
@@ -97,6 +109,7 @@ public class WebAppProviderTest
         assertTrue(hasJettyGeneratedPath(workDir, "foo.war"), "Should have generated directory in work directory: " + workDir);
     }
     
+    @Disabled("See issue #1200")
     @Test
     public void testStartupSymlinkContext()
     {
@@ -115,6 +128,102 @@ public class WebAppProviderTest
         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<String, String> 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<Resource> 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<String, String> setupJettyProperties(Path jettyHome)
+    {
+        Map<String, String> properties = new HashMap<>();
+        properties.put("jetty.home", jettyHome.toFile().getAbsolutePath());
+        return properties;
+    }
+    
     private static boolean hasJettyGeneratedPath(File basedir, String expectedWarFilename)
     {
         File[] paths = basedir.listFiles();

jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java

@@ -26,6 +26,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;
@@ -65,6 +66,51 @@ public class XmlConfiguredJetty
     private String _scheme = HttpScheme.HTTP.asString();
     private File _jettyHome;
 
+    public static Server loadConfigurations(List<Resource> configurations, Map<String, String> properties)
+        throws Exception
+    {
+        XmlConfiguration last = null;
+        Object[] obj = new Object[configurations.size()];
+
+        // Configure everything
+        for (int i = 0; i < configurations.size(); i++)
+        {
+            Resource config = configurations.get(i);
+            XmlConfiguration configuration = new XmlConfiguration(config);
+            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<>();
@@ -73,11 +119,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
@@ -149,6 +195,11 @@ public class XmlConfiguredJetty
         _xmlConfigurations.add(xmlConfig);
     }
     
+    public List<Resource> getConfigurations()
+    {
+        return Collections.unmodifiableList(_xmlConfigurations);
+    }
+
     public void assertNoWebAppContexts()
     {
         List<WebAppContext> contexts = getWebAppContexts();
@@ -168,7 +219,7 @@ public class XmlConfiguredJetty
         URL url = destUri.toURL();
 
         URLConnection conn = url.openConnection();
-
+        conn.addRequestProperty("Connection", "close");
         InputStream in = null;
         try
         {
@@ -321,46 +372,8 @@ public class XmlConfiguredJetty
 
     public void load() throws Exception
     {
-        XmlConfiguration last = null;
+        this._server = loadConfigurations(_xmlConfigurations, _properties);
-        Object[] obj = new Object[this._xmlConfigurations.size()];
+        this._server.setStopTimeout(10);
-
-        // Configure everything
-        for (int i = 0; i < this._xmlConfigurations.size(); i++)
-        {
-            Resource configResource = this._xmlConfigurations.get(i);
-            XmlConfiguration configuration = new XmlConfiguration(configResource);
-            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;
     }
 
     public void removeWebapp(String name)