Fix #8442 server propagation

Test setServer propagation protect from inadvertent null setting
jetty · Aug 10, 2022 · a0d6f42 · a0d6f42
1 parent 842af3e
commit a0d6f42
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 22 deletions.
diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Handler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Handler.java
@@ -535,19 +535,10 @@ public List<Handler> getHandlers()
         {
             Handler next = getHandler();
             if (next == null)
-                return List.of();
+                return Collections.emptyList();
             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
         {
@@ -632,7 +623,8 @@ public void setHandlers(List<Handler> handlers)
                     server.getInvocationType() != Invocable.combine(server.getInvocationType(), handler.getInvocationType()))
                     throw new IllegalArgumentException("Cannot change invocation type of started server");
 
-                handler.setServer(getServer());
+                if (server != null)
+                    handler.setServer(server);
             }
 
             updateBeans(_handlers, handlers);

diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceListing.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceListing.java
@@ -58,7 +58,6 @@ 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;

diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java
@@ -19,6 +19,7 @@
 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;
@@ -69,26 +70,21 @@ public DefaultHandler()
     public void setServer(Server server)
     {
         super.setServer(server);
-        initFavIcon();
+        if (server != null)
+            initFavIcon();
     }
 
     private void initFavIcon()
     {
         if (_favicon != null)
             return;
 
-        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;
-        }
+        Server server = Objects.requireNonNull(getServer());
 
         byte[] favbytes = null;
         try
         {
-            Resource faviconRes = getServer().getDefaultFavicon();
+            Resource faviconRes = server.getDefaultFavicon();
             if (faviconRes != null)
             {
                 try (InputStream is = faviconRes.newInputStream())

diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/HandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/HandlerTest.java
@@ -23,6 +23,7 @@
 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;
 
 /**
@@ -275,4 +276,50 @@ 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));
+    }
 }
diff --git a/...ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java b/...ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java
@@ -40,6 +40,7 @@
 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;
@@ -67,6 +68,7 @@
 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;
@@ -605,4 +607,16 @@ 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));
+    }
 }
diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/WebAppContextTest.java b/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,6 +67,7 @@
 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;
@@ -611,4 +612,16 @@ 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));
+    }
 }