Issue #7059 - prevent an internal NPE in AllowedResourceAliasChecker …

…doStart (#7076)

- prevent an internal NPE in AllowedResourceAliasChecker doStart
- Fix LifeCycle issues with AllowedResourceAliasChecker
- add null check for protected targets in toString.
- improve warning message for AllowedResourceAliasChecker
- add AllowedResourceAliasCheckerTest

jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java

@@ -17,14 +17,14 @@
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 
 import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
+import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.resource.PathResource;
 import org.eclipse.jetty.util.resource.Resource;
 import org.slf4j.Logger;
@@ -44,48 +44,71 @@ public class AllowedResourceAliasChecker extends AbstractLifeCycle implements Co
 
     private final ContextHandler _contextHandler;
     private final List<Path> _protected = new ArrayList<>();
+    private final AllowedResourceAliasCheckListener _listener = new AllowedResourceAliasCheckListener();
     protected Path _base;
 
     /**
      * @param contextHandler the context handler to use.
      */
     public AllowedResourceAliasChecker(ContextHandler contextHandler)
     {
-        _contextHandler = contextHandler;
+        _contextHandler = Objects.requireNonNull(contextHandler);
     }
 
     protected ContextHandler getContextHandler()
     {
         return _contextHandler;
     }
 
-    @Override
-    protected void doStart() throws Exception
+    protected void initialize()
     {
         _base = getPath(_contextHandler.getBaseResource());
         if (_base == null)
-            _base = Paths.get("/").toAbsolutePath();
-        if (Files.exists(_base, NO_FOLLOW_LINKS))
-            _base = _base.toRealPath(FOLLOW_LINKS);
+            return;
 
-        String[] protectedTargets = _contextHandler.getProtectedTargets();
-        if (protectedTargets != null)
+        try
+        {
+            if (Files.exists(_base, NO_FOLLOW_LINKS))
+                _base = _base.toRealPath(FOLLOW_LINKS);
+            String[] protectedTargets = _contextHandler.getProtectedTargets();
+            if (protectedTargets != null)
+            {
+                for (String s : protectedTargets)
+                    _protected.add(_base.getFileSystem().getPath(_base.toString(), s));
+            }
+        }
+        catch (IOException e)
         {
-            for (String s : protectedTargets)
-                _protected.add(_base.getFileSystem().getPath(_base.toString(), s));
+            LOG.warn("Base resource failure ({} is disabled): {}", this.getClass().getName(), _base, e);
+            _base = null;
         }
     }
 
+    @Override
+    protected void doStart() throws Exception
+    {
+        // We can only initialize if ContextHandler in started state, the baseResource can be changed even in starting state.
+        // If the ContextHandler is not started add a listener to delay initialization until fully started.
+        if (_contextHandler.isStarted())
+            initialize();
+        else
+            _contextHandler.addEventListener(_listener);
+    }
+
     @Override
     protected void doStop() throws Exception
     {
+        _contextHandler.removeEventListener(_listener);
         _base = null;
         _protected.clear();
     }
 
     @Override
     public boolean check(String pathInContext, Resource resource)
     {
+        if (_base == null)
+            return false;
+
         try
         {
             // The existence check resolves the symlinks.
@@ -184,7 +207,7 @@ protected Path getPath(Resource resource)
         {
             if (resource instanceof PathResource)
                 return ((PathResource)resource).getPath();
-            return resource.getFile().toPath();
+            return (resource == null) ? null : resource.getFile().toPath();
         }
         catch (Throwable t)
         {
@@ -193,13 +216,23 @@ protected Path getPath(Resource resource)
         }
     }
 
+    private class AllowedResourceAliasCheckListener implements LifeCycle.Listener
+    {
+        @Override
+        public void lifeCycleStarted(LifeCycle event)
+        {
+            initialize();
+        }
+    }
+
     @Override
     public String toString()
     {
+        String[] protectedTargets = _contextHandler.getProtectedTargets();
         return String.format("%s@%x{base=%s,protected=%s}",
             this.getClass().getSimpleName(),
             hashCode(),
             _base,
-            Arrays.asList(_contextHandler.getProtectedTargets()));
+            (protectedTargets == null) ? null : Arrays.asList(protectedTargets));
     }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java

@@ -40,6 +40,9 @@ public SymlinkAllowedResourceAliasChecker(ContextHandler contextHandler)
     @Override
     protected boolean check(String pathInContext, Path path)
     {
+        if (_base == null)
+            return false;
+
         // do not allow any file separation characters in the URI, as we need to know exactly what are the segments
         if (File.separatorChar != '/' && pathInContext.indexOf(File.separatorChar) >= 0)
             return false;

jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java

@@ -228,7 +228,7 @@ private enum ProtectedTargetType
     private final List<ContextScopeListener> _contextListeners = new CopyOnWriteArrayList<>();
     private final Set<EventListener> _durableListeners = new HashSet<>();
     private Index<ProtectedTargetType> _protectedTargets = Index.empty(false);
-    private final CopyOnWriteArrayList<AliasCheck> _aliasChecks = new CopyOnWriteArrayList<>();
+    private final List<AliasCheck> _aliasChecks = new CopyOnWriteArrayList<>();
 
     public enum Availability
     {
@@ -1700,6 +1700,8 @@ public String getResourceBase()
      */
     public void setBaseResource(Resource base)
     {
+        if (isStarted())
+            throw new IllegalStateException("Cannot call setBaseResource after starting");
         _baseResource = base;
     }
 
@@ -2072,19 +2074,19 @@ private String normalizeHostname(String host)
      */
     public void addAliasCheck(AliasCheck check)
     {
-        getAliasChecks().add(check);
+        _aliasChecks.add(check);
         if (check instanceof LifeCycle)
             addManaged((LifeCycle)check);
         else
             addBean(check);
     }
 
     /**
-     * @return Mutable list of Alias checks
+     * @return Immutable list of Alias checks
      */
     public List<AliasCheck> getAliasChecks()
     {
-        return _aliasChecks;
+        return Collections.unmodifiableList(_aliasChecks);
     }
 
     /**
@@ -2093,17 +2095,16 @@ public List<AliasCheck> getAliasChecks()
     public void setAliasChecks(List<AliasCheck> checks)
     {
         clearAliasChecks();
-        getAliasChecks().addAll(checks);
+        checks.forEach(this::addAliasCheck);
     }
 
     /**
      * clear the list of AliasChecks
      */
     public void clearAliasChecks()
     {
-        List<AliasCheck> aliasChecks = getAliasChecks();
-        aliasChecks.forEach(this::removeBean);
-        aliasChecks.clear();
+        _aliasChecks.forEach(this::removeBean);
+        _aliasChecks.clear();
     }
 
     /**

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java

@@ -60,7 +60,6 @@
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.resource.PathResource;
-import org.eclipse.jetty.util.resource.Resource;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -958,8 +957,6 @@ public void testRelativeRedirect() throws Exception
     public void testWelcomeRedirectDirWithQuestion() throws Exception
     {
         FS.ensureDirExists(docRoot);
-        context.setBaseResource(new PathResource(docRoot));
-
         Path dir = assumeMkDirSupported(docRoot, "dir?");
 
         Path index = dir.resolve("index.html");
@@ -992,8 +989,6 @@ public void testWelcomeRedirectDirWithQuestion() throws Exception
     public void testWelcomeRedirectDirWithSemicolon() throws Exception
     {
         FS.ensureDirExists(docRoot);
-        context.setBaseResource(new PathResource(docRoot));
-
         Path dir = assumeMkDirSupported(docRoot, "dir;");
 
         Path index = dir.resolve("index.html");
@@ -1215,8 +1210,6 @@ public void testWelcomeExactServlet(Scenario scenario) throws Exception
     public void testDirectFromResourceHttpContent() throws Exception
     {
         FS.ensureDirExists(docRoot);
-        context.setBaseResource(Resource.newResource(docRoot));
-
         Path index = docRoot.resolve("index.html");
         createFile(index, "<h1>Hello World</h1>");
 

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java

@@ -130,8 +130,7 @@ public void testLifeCycle() throws Exception
         server.start();
         context.addEventListener(new EventListener() {});
         listeners = context.getEventListeners();
-        listeners = context.getEventListeners();
-        assertThat(listeners.size(), is(3));
+        assertThat(listeners.size(), is(4));
 
         server.stop();
         listeners = context.getEventListeners();