Issue #7059 - prevent an internal NPE in AllowedResourceAliasChecker doStart (9.4)

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

@@ -22,14 +22,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.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.util.resource.PathResource;
@@ -49,48 +49,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.addLifeCycleListener(_listener);
+    }
+
     @Override
     protected void doStop() throws Exception
     {
+        _contextHandler.removeLifeCycleListener(_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.
@@ -189,7 +212,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)
         {
@@ -198,13 +221,43 @@ protected Path getPath(Resource resource)
         }
     }
 
+    private class AllowedResourceAliasCheckListener implements LifeCycle.Listener
+    {
+        @Override
+        public void lifeCycleStarting(LifeCycle event)
+        {
+        }
+
+        @Override
+        public void lifeCycleStarted(LifeCycle event)
+        {
+            initialize();
+        }
+
+        @Override
+        public void lifeCycleFailure(LifeCycle event, Throwable cause)
+        {
+        }
+
+        @Override
+        public void lifeCycleStopping(LifeCycle event)
+        {
+        }
+
+        @Override
+        public void lifeCycleStopped(LifeCycle event)
+        {
+        }
+    }
+
     @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

@@ -45,6 +45,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

@@ -229,7 +229,7 @@ public enum ContextStatus
     private final List<ContextScopeListener> _contextListeners = new CopyOnWriteArrayList<>();
     private final List<EventListener> _durableListeners = new CopyOnWriteArrayList<>();
     private String[] _protectedTargets;
-    private final CopyOnWriteArrayList<AliasCheck> _aliasChecks = new CopyOnWriteArrayList<>();
+    private final List<AliasCheck> _aliasChecks = new CopyOnWriteArrayList<>();
 
     public enum Availability
     {
@@ -1724,6 +1724,8 @@ public String getResourceBase()
      */
     public void setBaseResource(Resource base)
     {
+        if (isStarted())
+            throw new IllegalStateException("Cannot call setBaseResource after starting");
         _baseResource = base;
     }
 
@@ -2084,19 +2086,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);
     }
 
     /**
@@ -2105,17 +2107,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

@@ -66,7 +66,6 @@
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.log.StacklessLogging;
 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;
@@ -974,8 +973,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");
@@ -1008,8 +1005,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");
@@ -1232,8 +1227,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>");