Handle ServletContainerInitializer from WEB-INF/classes #3146 (#3158)

* Handle the case where a ServletContainerInitializer comes from
WEB-INF/classes

Signed-off-by: Jan Bartel <janb@webtide.com>

jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java

@@ -667,6 +667,21 @@ public class AnnotationConfiguration extends AbstractConfiguration
         if (context == null)
             throw new IllegalArgumentException("WebAppContext null");
         
+        //if we don't know where its from it can't be excluded
+        if (sciResource == null)
+        {
+            if (LOG.isDebugEnabled()) 
+                LOG.debug("!Excluded {} null resource", sci);
+            return false; 
+        }
+        
+        //A ServletContainerInitialier that came from WEB-INF/classes or equivalent cannot be excluded by an ordering
+        if (isFromWebInfClasses(context, sciResource))
+        {
+            if (LOG.isDebugEnabled()) 
+                LOG.debug("!Excluded {} from web-inf/classes", sci);
+            return false;
+        }
                 
         //A ServletContainerInitializer that came from the container's classpath cannot be excluded by an ordering
         //of WEB-INF/lib jars
@@ -695,14 +710,7 @@ public class AnnotationConfiguration extends AbstractConfiguration
             return true;
         }
 
-        if (sciResource == null)
+        //Check if it is excluded by an ordering
-        {
-            //not from a jar therefore not from WEB-INF so not excludable
-            if (LOG.isDebugEnabled()) 
-                LOG.debug("!Excluded {} not from jar", sci);
-            return false; 
-        }
-        
         URI loadingJarURI = sciResource.getURI();
         boolean found = false;
         Iterator<Resource> itor = orderedJars.iterator();
@@ -748,7 +756,46 @@ public class AnnotationConfiguration extends AbstractConfiguration
     {
         if (sci == null)
             return false;
-        return sci.getClass().getClassLoader()==context.getClassLoader().getParent();
+        
+        ClassLoader sciLoader = sci.getClass().getClassLoader();
+        
+        //if loaded by bootstrap loader, then its the container classpath
+        if ( sciLoader == null)
+            return true;
+        
+        //if there is no context classloader, then its the container classpath
+        if (context.getClassLoader() == null)
+            return true;
+        
+        ClassLoader loader = sciLoader;
+        while (loader != null)
+        {
+            if (loader == context.getClassLoader())
+                return false; //the webapp classloader is in the ancestry of the classloader for the sci
+            else
+                loader = loader.getParent();
+        }
+        
+        return true;
+    }
+    
+    /** 
+     * Test if the ServletContainerInitializer is from WEB-INF/classes
+     * 
+     * @param context the webapp to test
+     * @param sci a Resource representing the SCI
+     * @return true if the sci Resource is inside a WEB-INF/classes directory, false otherwise
+     */
+    public boolean isFromWebInfClasses (WebAppContext context, Resource sci)
+    {
+        for (Resource dir : context.getMetaData().getWebInfClassesDirs())
+        {                
+            if (dir.equals(sci))
+            { 
+                return true;
+            }
+        }
+        return false;
     }
 
     /**
@@ -835,25 +882,45 @@ public class AnnotationConfiguration extends AbstractConfiguration
             //Fallback to ordering the ServletContainerInitializers according to:
             //container classpath first, WEB-INF/classes then WEB-INF/lib (obeying any web.xml jar ordering)
             
-            //no web.xml ordering defined, add SCIs in any order
+            //First add in all SCIs that can't be excluded
+            int lastContainerSCI = -1;
+            for (Map.Entry<ServletContainerInitializer, Resource> entry:sciResourceMap.entrySet())
+            {
+                if (entry.getKey().getClass().getClassLoader()==context.getClassLoader().getParent())
+                {
+                    nonExcludedInitializers.add(++lastContainerSCI, entry.getKey()); //add all container SCIs before any webapp SCIs
+                }
+                else if (entry.getValue() == null) //can't work out provenance of SCI, so can't be ordered/excluded
+                {
+                    nonExcludedInitializers.add(entry.getKey()); //add at end of list
+                }
+                else
+                {
+                    for (Resource dir : context.getMetaData().getWebInfClassesDirs())
+                    {                
+                        if (dir.equals(entry.getValue()))//from WEB-INF/classes so can't be ordered/excluded
+                        { 
+                            nonExcludedInitializers.add(entry.getKey());
+                        }
+                    }
+                }
+            }
+            
+            //throw out the ones we've already accounted for
+            for (ServletContainerInitializer s:nonExcludedInitializers)
+                sciResourceMap.remove(s);
+            
             if (context.getMetaData().getOrdering() == null)
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("No web.xml ordering, ServletContainerInitializers in random order");
+                //add the rest of the scis
                 nonExcludedInitializers.addAll(sciResourceMap.keySet());
             }
             else
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("Ordering ServletContainerInitializers with ordering {}",context.getMetaData().getOrdering());
-                for (Map.Entry<ServletContainerInitializer, Resource> entry:sciResourceMap.entrySet())
-                {
-                    //add in SCIs from the container classpath
-                    if (entry.getKey().getClass().getClassLoader()==context.getClassLoader().getParent())
-                        nonExcludedInitializers.add(entry.getKey());
-                    else if (entry.getValue() == null) //add in SCIs not in a jar, as they must be from WEB-INF/classes and can't be ordered
-                        nonExcludedInitializers.add(entry.getKey());
-                }
                 
                 //add SCIs according to the ordering of its containing jar
                 for (Resource webInfJar:context.getMetaData().getOrderedWebInfJars())

jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java

@@ -28,6 +28,7 @@ import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -37,6 +38,7 @@ import javax.servlet.ServletContainerInitializer;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.webapp.FragmentDescriptor;
+import org.eclipse.jetty.webapp.RelativeOrdering;
 import org.eclipse.jetty.webapp.WebAppContext;
 import org.junit.jupiter.api.Test;
 
@@ -126,39 +128,100 @@ public class TestAnnotationConfiguration
         File web25 = MavenTestingUtils.getTestResourceFile("web25.xml");
         File web31false = MavenTestingUtils.getTestResourceFile("web31false.xml");
         File web31true = MavenTestingUtils.getTestResourceFile("web31true.xml");
-        Set<String> sciNames = new HashSet<>(Arrays.asList("org.eclipse.jetty.annotations.ServerServletContainerInitializer", "com.acme.initializer.FooInitializer"));
 
         //prepare an sci that will be on the webapp's classpath
         File jarDir = new File(MavenTestingUtils.getTestResourcesDir().getParentFile(), "jar");
         File testSciJar = new File(jarDir, "test-sci.jar");
         assertTrue(testSciJar.exists());
-        URLClassLoader webAppLoader = new URLClassLoader(new URL[] {testSciJar.toURI().toURL()}, Thread.currentThread().getContextClassLoader());
 
-        //test 3.1 webapp loads both server and app scis
+        File testContainerSciJar = new File(jarDir, "test-sci-for-container-path.jar");
+        URLClassLoader containerLoader = new URLClassLoader(new URL[] {testContainerSciJar.toURI().toURL()}, Thread.currentThread().getContextClassLoader());
+        URLClassLoader webAppLoader = new URLClassLoader(new URL[] {testSciJar.toURI().toURL()}, containerLoader);
+        Resource targetClasses = Resource.newResource(MavenTestingUtils.getTargetDir().toURI()).addPath("/test-classes");
+
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader(containerLoader);
+        try
+        {
+            
             AnnotationConfiguration config = new AnnotationConfiguration();
             WebAppContext context = new WebAppContext();
+            List<ServletContainerInitializer> scis;
+
+            //test 3.1 webapp loads both server and app scis
             context.setClassLoader(webAppLoader);
+            context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL()));
             context.getMetaData().setWebXml(Resource.newResource(web31true));
+            context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses));
             context.getServletContext().setEffectiveMajorVersion(3);
             context.getServletContext().setEffectiveMinorVersion(1);
-        List<ServletContainerInitializer> scis = config.getNonExcludedInitializers(context);
+            scis = config.getNonExcludedInitializers(context);
             assertNotNull(scis);
-        assertEquals(2, scis.size());
+            assertEquals(3, scis.size());
-        assertTrue (sciNames.contains(scis.get(0).getClass().getName()));
+            assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path
-        assertTrue (sciNames.contains(scis.get(1).getClass().getName()));
+            assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes
+            assertEquals("com.acme.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment
 
             //test a 3.1 webapp with metadata-complete=false loads both server and webapp scis
             config = new AnnotationConfiguration();
             context = new WebAppContext();
             context.setClassLoader(webAppLoader);
             context.getMetaData().setWebXml(Resource.newResource(web31false));
+            context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses));
+            context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL()));
             context.getServletContext().setEffectiveMajorVersion(3);
             context.getServletContext().setEffectiveMinorVersion(1);
             scis = config.getNonExcludedInitializers(context);
             assertNotNull(scis);
-        assertEquals(2, scis.size());
+            assertEquals(3, scis.size());
-        assertTrue (sciNames.contains(scis.get(0).getClass().getName()));
+            assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path
-        assertTrue (sciNames.contains(scis.get(1).getClass().getName()));
+            assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes
+            assertEquals("com.acme.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment
+
+
+            //test a 3.1 webapp with RELATIVE ORDERING loads sci from equivalent of WEB-INF/classes as well as container path
+            File orderedFragmentJar = new File(jarDir, "test-sci-with-ordering.jar");
+            assertTrue(orderedFragmentJar.exists());   
+            URLClassLoader orderedLoader = new URLClassLoader(new URL[] {orderedFragmentJar.toURI().toURL(), testSciJar.toURI().toURL()}, Thread.currentThread().getContextClassLoader());
+            config = new AnnotationConfiguration();
+            context = new WebAppContext();
+            context.setClassLoader(orderedLoader);
+            context.getMetaData().setWebXml(Resource.newResource(web31true));
+            RelativeOrdering ordering = new RelativeOrdering(context.getMetaData());
+            context.getMetaData().setOrdering(ordering);
+            context.getMetaData().addWebInfJar(Resource.newResource(orderedFragmentJar.toURI().toURL()));
+            context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL()));
+            context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses));
+            context.getMetaData().orderFragments();
+            context.getServletContext().setEffectiveMajorVersion(3);
+            context.getServletContext().setEffectiveMinorVersion(1);
+            scis = config.getNonExcludedInitializers(context);
+            assertNotNull(scis);
+            assertEquals(4, scis.size());
+            assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path
+            assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes
+            assertEquals("com.acme.AcmeServletContainerInitializer", scis.get(2).getClass().getName()); //first in ordering
+            assertEquals("com.acme.initializer.FooInitializer", scis.get(3).getClass().getName()); //other in ordering
+
+            
+            //test 3.1 webapp with a specific SCI ordering
+            config = new AnnotationConfiguration();
+            context = new WebAppContext();
+            context.setClassLoader(webAppLoader);
+            context.getMetaData().setWebXml(Resource.newResource(web31false));
+            context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses));
+            context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL()));
+            context.getServletContext().setEffectiveMajorVersion(3);
+            context.getServletContext().setEffectiveMinorVersion(1);
+            context.setAttribute("org.eclipse.jetty.containerInitializerOrder", "com.acme.initializer.FooInitializer,com.acme.ServerServletContainerInitializer, *");
+            scis = config.getNonExcludedInitializers(context);
+            assertNotNull(scis);
+            assertEquals(3, scis.size());
+            assertEquals("com.acme.initializer.FooInitializer", scis.get(0).getClass().getName()); //web-inf jar no web-fragment
+            assertEquals("com.acme.ServerServletContainerInitializer", scis.get(1).getClass().getName()); //container path
+            assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(2).getClass().getName()); //web-inf classes
+            
+            
             
             
             //test 2.5 webapp with configurationDiscovered=false loads only server scis
@@ -166,12 +229,22 @@ public class TestAnnotationConfiguration
             context = new WebAppContext();
             context.setClassLoader(webAppLoader);
             context.getMetaData().setWebXml(Resource.newResource(web25));
+            context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses));
+            context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL()));
             context.getServletContext().setEffectiveMajorVersion(2);
             context.getServletContext().setEffectiveMinorVersion(5);
             scis = config.getNonExcludedInitializers(context);
             assertNotNull(scis);
-        assertEquals(1, scis.size());
+            for (ServletContainerInitializer s:scis)
-        assertTrue ("org.eclipse.jetty.annotations.ServerServletContainerInitializer".equals(scis.get(0).getClass().getName()));
+            {
+                //should not have any of the web-inf lib scis in here
+                assertFalse(s.getClass().getName().equals("com.acme.AcmeServletContainerInitializer"));
+                assertFalse(s.getClass().getName().equals("com.acme.initializer.FooInitializer"));
+                //NOTE: should also not have the web-inf classes scis in here either, but due to the
+                //way the test is set up, the sci we're pretending is in web-inf classes will actually
+                //NOT be loaded by the webapp's classloader, but rather by the junit classloader, so
+                //it looks as if it is a container class.
+            }
 
             //test 2.5 webapp with configurationDiscovered=true loads both server and webapp scis
             config = new AnnotationConfiguration();
@@ -179,13 +252,22 @@ public class TestAnnotationConfiguration
             context.setConfigurationDiscovered(true);
             context.setClassLoader(webAppLoader);
             context.getMetaData().setWebXml(Resource.newResource(web25));
+            context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses));
+            context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL()));
             context.getServletContext().setEffectiveMajorVersion(2);
             context.getServletContext().setEffectiveMinorVersion(5);
             scis = config.getNonExcludedInitializers(context);
             assertNotNull(scis);
-        assertEquals(2, scis.size());
+            assertEquals(3, scis.size());
-        assertTrue (sciNames.contains(scis.get(0).getClass().getName()));
+            assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path
-        assertTrue (sciNames.contains(scis.get(1).getClass().getName()));
+            assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes
+            assertEquals("com.acme.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment
+
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old);
+        }
     }
 
     

jetty-annotations/src/test/java/org/eclipse/jetty/annotations/WebInfClassServletContainerInitializer.java

@@ -30,13 +30,13 @@ import javax.servlet.ServletException;
  *
  *
  */
-public class ServerServletContainerInitializer implements ServletContainerInitializer
+public class WebInfClassServletContainerInitializer implements ServletContainerInitializer
 {
 
     /**
      * 
      */
-    public ServerServletContainerInitializer()
+    public WebInfClassServletContainerInitializer()
     {
         // TODO Auto-generated constructor stub
     }

jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer

@@ -1 +1 @@
-org.eclipse.jetty.annotations.ServerServletContainerInitializer
+org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer