From 9a1cadade4ef3b02405d0c7e4e2b9d6c200c550d Mon Sep 17 00:00:00 2001
From: gregw <gregw@webtide.com>
Date: Mon, 5 Oct 2020 18:12:46 +0200
Subject: [PATCH] Fixes #5378 Setting Holders during STARTING

Holders are now started/initialized if needed by a new utility method
---
 .../eclipse/jetty/servlet/ServletHandler.java | 57 +++++++++----------
 .../servlet/ServletContextHandlerTest.java    | 24 ++++++--
 2 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
index 32d80bb99084..ba41caf32858 100644
--- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
+++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
@@ -773,6 +773,29 @@ public boolean isInitialized()
         return _initialized;
     }
 
+    protected void initializeHolders(BaseHolder<?>[] holders)
+    {
+        for (BaseHolder<?> holder : holders)
+        {
+            holder.setServletHandler(this);
+            if (isInitialized())
+            {
+                try
+                {
+                    if (!holder.isStarted())
+                    {
+                        holder.start();
+                        holder.initialize();
+                    }
+                }
+                catch (Exception e)
+                {
+                    throw new RuntimeException(e);
+                }
+            }
+        }
+    }
+
     /**
      * @return whether the filter chains are cached.
      */
@@ -800,10 +823,7 @@ public ListenerHolder[] getListeners()
     public void setListeners(ListenerHolder[] listeners)
     {
         if (listeners != null)
-            for (ListenerHolder holder : listeners)
-            {
-                holder.setServletHandler(this);
-            }
+            initializeHolders(listeners);
         updateBeans(_listeners,listeners);
         _listeners = listeners;
     }
@@ -865,9 +885,6 @@ public void addServletWithMapping(ServletHolder servlet, String pathSpec)
     {
         Objects.requireNonNull(servlet);
         ServletHolder[] holders = getServlets();
-        if (holders != null)
-            holders = holders.clone();
-
         try
         {
             synchronized (this)
@@ -979,8 +996,6 @@ public void addFilterWithMapping(FilterHolder holder, String pathSpec, EnumSet<D
     {
         Objects.requireNonNull(holder);
         FilterHolder[] holders = getFilters();
-        if (holders != null)
-            holders = holders.clone();
 
         try
         {
@@ -1435,16 +1450,6 @@ else if (isAllowDuplicateMappings())
             LOG.debug("servletPathMap=" + _servletPathMap);
             LOG.debug("servletNameMap=" + _servletNameMap);
         }
-
-        try
-        {
-            if (_contextHandler != null && _contextHandler.isStarted() || _contextHandler == null && isStarted())
-                initialize();
-        }
-        catch (Exception e)
-        {
-            throw new RuntimeException(e);
-        }
     }
 
     protected void notFound(Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
@@ -1496,7 +1501,7 @@ public void setFilterMappings(FilterMapping[] filterMappings)
     {
         updateBeans(_filterMappings,filterMappings);
         _filterMappings = filterMappings;
-        if (isStarted())
+        if (isRunning())
             updateMappings();
         invalidateChainsCache();
     }
@@ -1504,10 +1509,7 @@ public void setFilterMappings(FilterMapping[] filterMappings)
     public synchronized void setFilters(FilterHolder[] holders)
     {
         if (holders != null)
-            for (FilterHolder holder : holders)
-            {
-                holder.setServletHandler(this);
-            }
+            initializeHolders(holders);
         updateBeans(_filters,holders);
         _filters = holders;
         updateNameMappings();
@@ -1521,7 +1523,7 @@ public void setServletMappings(ServletMapping[] servletMappings)
     {
         updateBeans(_servletMappings,servletMappings);
         _servletMappings = servletMappings;
-        if (isStarted())
+        if (isRunning())
             updateMappings();
         invalidateChainsCache();
     }
@@ -1534,10 +1536,7 @@ public void setServletMappings(ServletMapping[] servletMappings)
     public synchronized void setServlets(ServletHolder[] holders)
     {
         if (holders != null)
-            for (ServletHolder holder : holders)
-            {
-                holder.setServletHandler(this);
-            }
+            initializeHolders(holders);
         updateBeans(_servlets,holders);
         _servlets = holders;
         updateNameMappings();
diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java
index fe076310ae36..efab1cb97921 100644
--- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java
+++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java
@@ -1651,21 +1651,35 @@ public void testProgrammaticFilterServlet() throws Exception
         ServletHandler handler = new ServletHandler();
         _server.setHandler(context);
         context.setHandler(handler);
-        handler.addServletWithMapping(new ServletHolder(new TestServlet()), "/");
+        handler.addServletWithMapping(new ServletHolder(new TestServlet()
+        {
+            @Override
+            public void init() throws ServletException
+            {
+                handler.addFilterWithMapping(new FilterHolder(new MyFilter()), "/test/*", EnumSet.of(DispatcherType.REQUEST));
+            }
+        }), "/test");
 
         _server.start();
 
-
         String request =
-            "GET /hello HTTP/1.0\n" +
+            "GET /test HTTP/1.0\n" +
             "Host: localhost\n" +
             "\n";
         String response = _connector.getResponse(request);
         assertThat(response, containsString("200 OK"));
+        assertThat(response, containsString("filter: filter"));
         assertThat(response, containsString("Test"));
 
-        handler.addFilterWithMapping(new FilterHolder(new MyFilter()), "/*", EnumSet.of(DispatcherType.REQUEST));
-        handler.addServletWithMapping(new ServletHolder(new HelloServlet()), "/hello/*");
+
+        handler.addServletWithMapping(new ServletHolder(new HelloServlet()
+        {
+            @Override
+            public void init() throws ServletException
+            {
+                handler.addFilterWithMapping(new FilterHolder(new MyFilter()), "/hello/*", EnumSet.of(DispatcherType.REQUEST));
+            }
+        }), "/hello/*");
 
         _server.dumpStdErr();