revert #8442 changes

Signed-off-by: Ludovic Orban <lorban@bitronix.be>

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Handler.java

@@ -535,10 +535,19 @@ public List<Handler> getHandlers()
         {
             Handler next = getHandler();
             if (next == null)
-                return Collections.emptyList();
+                return List.of();
             return List.of(next);
         }
 
+        @Override
+        public void setServer(Server server)
+        {
+            super.setServer(server);
+            Handler next = getHandler();
+            if (next != null)
+                next.setServer(getServer());
+        }
+
         @Override
         public Request.Processor handle(Request request) throws Exception
         {
@@ -623,8 +632,7 @@ public void setHandlers(List<Handler> handlers)
                     server.getInvocationType() != Invocable.combine(server.getInvocationType(), handler.getInvocationType()))
                     throw new IllegalArgumentException("Cannot change invocation type of started server");
 
-                if (server != null)
-                    handler.setServer(server);
+                handler.setServer(getServer());
             }
 
             updateBeans(_handlers, handlers);

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceListing.java

@@ -58,6 +58,7 @@ public static String getAsHTML(Resource resource, String base, boolean parent, S
         base = URIUtil.normalizePath(base);
         if (base == null || !resource.isDirectory())
             return null;
+
         Path path = resource.getPath();
         if (path == null) // Should never happen, as new Resource contract is that all Resources are a Path.
             return null;

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java

@@ -199,11 +199,9 @@ static void writeError(Request request, Response response, Callback callback, in
         Logger logger = LoggerFactory.getLogger(Response.class);
 
         // Let's be less verbose with BadMessageExceptions & QuietExceptions
-        if (logger.isDebugEnabled())
-            logger.debug("writeError: status={}, message={}, response={}", status, message, response, cause);
-        else if (cause instanceof BadMessageException || cause instanceof QuietException)
-            logger.debug("writeError: status={}, message={}, response={} {}", status, message, response, cause.toString());
-        else if (cause != null)
+        if (!logger.isDebugEnabled() && (cause instanceof BadMessageException || cause instanceof QuietException))
+            logger.warn("writeError: status={}, message={}, cause={}", status, message, cause.getMessage());
+        else
             logger.warn("writeError: status={}, message={}, response={}", status, message, response, cause);
 
         if (response.isCommitted())

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java

@@ -671,8 +671,6 @@ public Resource getDefaultFavicon()
      */
     private Resource newResource(String name)
     {
-        // TODO replace this.  It is needlessly complex and inefficient as it holds a mount of the server jar
-        //      just for things like favicon and default stylesheet
         URL url = getClass().getResource(name);
         if (url == null)
             throw new IllegalStateException("Missing server resource: " + name);

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java

@@ -19,7 +19,6 @@
 import java.nio.ByteBuffer;
 import java.util.Collections;
 import java.util.List;
-import java.util.Objects;
 
 import org.eclipse.jetty.http.DateGenerator;
 import org.eclipse.jetty.http.HttpField;
@@ -70,21 +69,26 @@ public DefaultHandler()
     public void setServer(Server server)
     {
         super.setServer(server);
-        if (server != null)
-            initFavIcon();
+        initFavIcon();
     }
 
     private void initFavIcon()
     {
         if (_favicon != null)
             return;
 
-        Server server = Objects.requireNonNull(getServer());
+        if (getServer() == null)
+        {
+            // TODO: investigate why DefaultHandler.setServer(server) is passing null?
+            // See bug https://github.com/eclipse/jetty.project/issues/8442
+            LOG.warn("favicon.ico not supported with null Server");
+            return;
+        }
 
         byte[] favbytes = null;
         try
         {
-            Resource faviconRes = server.getDefaultFavicon();
+            Resource faviconRes = getServer().getDefaultFavicon();
             if (faviconRes != null)
             {
                 try (InputStream is = faviconRes.newInputStream())

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/HandlerTest.java

@@ -23,7 +23,6 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.nullValue;
-import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
@@ -276,50 +275,4 @@ public Request.Processor handle(Request request)
         IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> a.insertHandler(b));
         assertThat(e.getMessage(), containsString("bad tail"));
     }
-
-    @Test
-    public void testSetServerPropagation()
-    {
-        Handler.Wrapper wrapper = new Handler.Wrapper();
-        Handler.Collection collection = new Handler.Collection();
-        Handler handler = new Handler.Abstract()
-        {
-            @Override
-            public Request.Processor handle(Request request) throws Exception
-            {
-                return null;
-            }
-        };
-
-        collection.addHandler(wrapper);
-        wrapper.setHandler(handler);
-
-        Server server = new Server();
-        collection.setServer(server);
-
-        assertThat(handler.getServer(), sameInstance(server));
-    }
-
-    @Test
-    public void testSetHandlerServerPropagation()
-    {
-        Handler.Wrapper wrapper = new Handler.Wrapper();
-        Handler.Collection collection = new Handler.Collection();
-        Handler handler = new Handler.Abstract()
-        {
-            @Override
-            public Request.Processor handle(Request request) throws Exception
-            {
-                return null;
-            }
-        };
-
-        Server server = new Server();
-        collection.setServer(server);
-
-        collection.addHandler(wrapper);
-        wrapper.setHandler(handler);
-
-        assertThat(handler.getServer(), sameInstance(server));
-    }
 }

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java

@@ -1203,20 +1203,6 @@ else if (slash)
         return canonical.toString();
     }
 
-    /**
-     * <p>Check if a path would be normalized within itself. For example,
-     * <code>/foo/../../bar</code> is normalized above its root and would
-     * thus return false, whilst <code>/foo/./bar/..</code> is normal within itself
-     * and would return true.
-     * @param path The path to check
-     * @return True if the normal form of the path is within the root of the path.
-     */
-    public static boolean isNotNormalWithinSelf(String path)
-    {
-        // TODO this can be optimized to avoid allocation.
-        return normalizePath(path) == null;
-    }
-
     /**
      * <p>Normalize a URI path by factoring out all segments of "." and "..".
      * Null is returned if the path is normalized above its root.

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java

@@ -310,8 +310,8 @@ public Resource resolve(String subUriPath)
         // Check that the path is within the root,
         // but use the original path to create the
         // resource, to preserve aliasing.
-        // TODO do a URI safe encoding?
-        if (URIUtil.isNotNormalWithinSelf(subUriPath))
+        // TODO should we canonicalize here? Or perhaps just do a URI safe encoding
+        if (URIUtil.normalizePath(subUriPath) == null)
             throw new IllegalArgumentException(subUriPath);
 
         if (URIUtil.SLASH.equals(subUriPath))

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java

@@ -112,7 +112,7 @@ public List<Resource> getResources()
     @Override
     public Resource resolve(String subUriPath)
     {
-        if (URIUtil.isNotNormalWithinSelf(subUriPath))
+        if (URIUtil.normalizePath(subUriPath) == null)
             throw new IllegalArgumentException(subUriPath);
 
         if (subUriPath.length() == 0 || URIUtil.SLASH.equals(subUriPath))

jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-jetty-webapp/src/test/java/org/eclipse/jetty/ee10/TestServer.java

@@ -96,7 +96,7 @@ public static void main(String[] args) throws Exception
         // Setup context
         HashLoginService login = new HashLoginService();
         login.setName("Test Realm");
-        Path realmPropPath = webappProjectRoot.resolve("jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-jetty-webapp/src/test/resources/test-realm.properties");
+        Path realmPropPath = webappProjectRoot.resolve("src/test/resources/test-realm.properties");
         if (!Files.exists(realmPropPath))
             throw new FileNotFoundException(realmPropPath.toString());
         Resource realmResource = ResourceFactory.of(server).newResource(realmPropPath);
@@ -112,7 +112,7 @@ public static void main(String[] args) throws Exception
         WebAppContext webapp = new WebAppContext();
         webapp.setContextPath("/test");
         webapp.setParentLoaderPriority(true);
-        Path webappBase = webappProjectRoot.resolve("jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-jetty-webapp/src/main/webapp");
+        Path webappBase = webappProjectRoot.resolve("src/main/webapp");
         if (!Files.exists(webappBase))
             throw new FileNotFoundException(webappBase.toString());
         webapp.setBaseResource(ResourceFactory.of(server).newResource(webappBase));
@@ -132,7 +132,7 @@ public static void main(String[] args) throws Exception
         contexts.addHandler(webapp);
 
         ContextHandler srcroot = new ContextHandler();
-        Path srcRootPath = webappProjectRoot.resolve("jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-jetty-webapp/src");
+        Path srcRootPath = webappProjectRoot.resolve("src");
         if (!Files.exists(srcRootPath))
             throw new FileNotFoundException(srcRootPath.toString());
         srcroot.setBaseResource(ResourceFactory.of(server).newResource(srcRootPath));

jetty-ee10/jetty-ee10-maven-plugin/src/main/java/org/eclipse/jetty/ee10/maven/plugin/SelectiveJarResource.java

@@ -162,7 +162,8 @@ public void copyTo(Path directory) throws IOException
 
                 LOG.debug("Looking at {}", entryName);
                 // make sure no access out of the root entry is present
-                if (URIUtil.isNotNormalWithinSelf(entryName))
+                String dotCheck = URIUtil.normalizePath(entryName);
+                if (dotCheck == null)
                 {
                     LOG.info("Invalid entry: {}", entryName);
                     continue;

jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java

@@ -40,7 +40,6 @@
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.handler.ContextHandlerCollection;
-import org.eclipse.jetty.server.handler.DefaultHandler;
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
@@ -68,7 +67,6 @@
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -93,8 +91,7 @@ public void tearDown()
     {
         lifeCycles.forEach(LifeCycle::stop);
         Configurations.cleanKnown();
-        if (!FileSystemPool.INSTANCE.mounts().isEmpty())
-            LOG.warn("Not empty mounts: " + FileSystemPool.INSTANCE.mounts());
+        assertThat(FileSystemPool.INSTANCE.mounts(), empty());
     }
 
     private Server newServer()
@@ -608,16 +605,4 @@ public void testExtraClasspathDir(String extraClassPathReference) throws Excepti
         extLibs = extLibs.toAbsolutePath();
         assertThat("URL[0]", urls[0].toURI(), is(extLibs.toUri()));
     }
-
-    @Test
-    void testSetServerPropagation()
-    {
-        Server server = new Server();
-        WebAppContext context = new WebAppContext();
-        context.setContextPath("/");
-        DefaultHandler handler = new DefaultHandler();
-        server.setHandler(new Handler.Collection(context, handler));
-
-        assertThat(handler.getServer(), sameInstance(server));
-    }
 }

jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServlet.java

@@ -29,8 +29,8 @@
 import org.eclipse.jetty.ee10.websocket.server.internal.JettyServerFrameHandlerFactory;
 import org.eclipse.jetty.ee10.websocket.servlet.WebSocketUpgradeFilter;
 import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.util.Blocker;
 import org.eclipse.jetty.util.Callback;
-import org.eclipse.jetty.util.FutureCallback;
 import org.eclipse.jetty.websocket.core.Configuration;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.server.FrameHandlerFactory;
@@ -184,11 +184,13 @@ protected void service(HttpServletRequest req, HttpServletResponse resp)
             throw new IllegalStateException("Base Request not available");
 
         // provide a null default customizer the customizer will be on the negotiator in the mapping
-        FutureCallback callback = new FutureCallback();
-        if (mapping.upgrade(request, request.getResponse(), callback, null))
+        try (Blocker.Callback callback = Blocker.callback())
         {
-            callback.block();
-            return;
+            if (mapping.upgrade(request, request.getResponse(), callback, null))
+            {
+                callback.block();
+                return;
+            }
         }
 
         // If we reach this point, it means we had an incoming request to upgrade

jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-servlet/src/main/java/org/eclipse/jetty/ee10/websocket/servlet/WebSocketUpgradeFilter.java

@@ -32,7 +32,7 @@
 import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
 import org.eclipse.jetty.ee10.servlet.ServletHandler;
 import org.eclipse.jetty.server.handler.ContextHandler;
-import org.eclipse.jetty.util.FutureCallback;
+import org.eclipse.jetty.util.Blocker;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.Dumpable;
 import org.eclipse.jetty.util.component.LifeCycle;
@@ -160,11 +160,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
             throw new IllegalStateException("Base Request not available");
 
         // provide a null default customizer the customizer will be on the negotiator in the mapping
-        FutureCallback callback = new FutureCallback();
-        if (mappings.upgrade(baseRequest, baseRequest.getResponse(), callback, defaultCustomizer))
+        try (Blocker.Callback callback = Blocker.callback())
         {
-            callback.block();
-            return;
+            if (mappings.upgrade(baseRequest, baseRequest.getResponse(), callback, defaultCustomizer))
+            {
+                callback.block();
+                return;
+            }
+            callback.succeeded(); // TODO this is wasteful making a blocker on every request, even if it is not used. At leasts should be shared... but better to detect if we might need to upgrade first?
         }
 
         // If we reach this point, it means we had an incoming request to upgrade

jetty-ee9/jetty-ee9-maven-plugin/src/main/java/org/eclipse/jetty/ee9/maven/plugin/SelectiveJarResource.java

@@ -162,7 +162,8 @@ public void copyTo(Path directory) throws IOException
 
                 LOG.debug("Looking at {}", entryName);
                 // make sure no access out of the root entry is present
-                if (URIUtil.isNotNormalWithinSelf(entryName))
+                String dotCheck = URIUtil.normalizePath(entryName);
+                if (dotCheck == null)
                 {
                     LOG.info("Invalid entry: {}", entryName);
                     continue;

jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppContextTest.java

@@ -40,14 +40,14 @@
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.handler.ContextHandlerCollection;
-import org.eclipse.jetty.server.handler.DefaultHandler;
 import org.eclipse.jetty.toolchain.test.FS;
 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.URIUtil;
 import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.resource.FileSystemPool;
+import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.resource.ResourceFactory;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
@@ -67,7 +67,6 @@
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -612,16 +611,4 @@ public void testExtraClasspathDir(String extraClassPathReference) throws Excepti
         extLibs = extLibs.toAbsolutePath();
         assertThat("URL[0]", urls[0].toURI(), is(extLibs.toUri()));
     }
-
-    @Test
-    void testSetServerPropagation()
-    {
-        Server server = new Server();
-        WebAppContext context = new WebAppContext();
-        context.setContextPath("/");
-        DefaultHandler handler = new DefaultHandler();
-        server.setHandler(new Handler.Collection(context.get(), handler));
-
-        assertThat(handler.getServer(), sameInstance(server));
-    }
 }