diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java index 9d1cc3661bc..e9a39abd080 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java +++ b/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) - { - //not from a jar therefore not from WEB-INF so not excludable - if (LOG.isDebugEnabled()) - LOG.debug("!Excluded {} not from jar", sci); - return false; - } - + //Check if it is excluded by an ordering URI loadingJarURI = sciResource.getURI(); boolean found = false; Iterator 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; } /** @@ -834,27 +881,47 @@ public class AnnotationConfiguration extends AbstractConfiguration //No jetty-specific ordering specified, or just the wildcard value "*" specified. //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 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 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()) { @@ -874,7 +941,7 @@ public class AnnotationConfiguration extends AbstractConfiguration ListIterator it = nonExcludedInitializers.listIterator(); while (it.hasNext()) { - ServletContainerInitializer sci = it.next(); + ServletContainerInitializer sci = it.next(); if (!isFromContainerClassPath(context, sci)) { if (LOG.isDebugEnabled()) diff --git a/jetty-annotations/src/test/jar/test-sci-for-container-path.jar b/jetty-annotations/src/test/jar/test-sci-for-container-path.jar new file mode 100644 index 00000000000..d6fa63a78d6 Binary files /dev/null and b/jetty-annotations/src/test/jar/test-sci-for-container-path.jar differ diff --git a/jetty-annotations/src/test/jar/test-sci-with-ordering.jar b/jetty-annotations/src/test/jar/test-sci-with-ordering.jar new file mode 100644 index 00000000000..7e1d8fc847b Binary files /dev/null and b/jetty-annotations/src/test/jar/test-sci-with-ordering.jar differ diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java index 4a0b25f79ce..275366c1e1f 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java +++ b/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,68 +128,148 @@ public class TestAnnotationConfiguration File web25 = MavenTestingUtils.getTestResourceFile("web25.xml"); File web31false = MavenTestingUtils.getTestResourceFile("web31false.xml"); File web31true = MavenTestingUtils.getTestResourceFile("web31true.xml"); - Set 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 - AnnotationConfiguration config = new AnnotationConfiguration(); - WebAppContext context = new WebAppContext(); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web31true)); - context.getServletContext().setEffectiveMajorVersion(3); - context.getServletContext().setEffectiveMinorVersion(1); - List scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(2, scis.size()); - assertTrue (sciNames.contains(scis.get(0).getClass().getName())); - assertTrue (sciNames.contains(scis.get(1).getClass().getName())); - - //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.getServletContext().setEffectiveMajorVersion(3); - context.getServletContext().setEffectiveMinorVersion(1); - scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(2, scis.size()); - assertTrue (sciNames.contains(scis.get(0).getClass().getName())); - assertTrue (sciNames.contains(scis.get(1).getClass().getName())); - - - //test 2.5 webapp with configurationDiscovered=false loads only server scis - config = new AnnotationConfiguration(); - context = new WebAppContext(); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web25)); - context.getServletContext().setEffectiveMajorVersion(2); - context.getServletContext().setEffectiveMinorVersion(5); - scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(1, scis.size()); - assertTrue ("org.eclipse.jetty.annotations.ServerServletContainerInitializer".equals(scis.get(0).getClass().getName())); - - //test 2.5 webapp with configurationDiscovered=true loads both server and webapp scis - config = new AnnotationConfiguration(); - context = new WebAppContext(); - context.setConfigurationDiscovered(true); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web25)); - context.getServletContext().setEffectiveMajorVersion(2); - context.getServletContext().setEffectiveMinorVersion(5); - scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(2, scis.size()); - assertTrue (sciNames.contains(scis.get(0).getClass().getName())); - assertTrue (sciNames.contains(scis.get(1).getClass().getName())); + assertTrue(testSciJar.exists()); + + 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 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); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + assertEquals(3, 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.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(3, 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.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 + config = new AnnotationConfiguration(); + 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); + for (ServletContainerInitializer s:scis) + { + //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(); + context = new WebAppContext(); + 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(3, 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.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment + + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } - + @Test public void testGetFragmentFromJar() throws Exception diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/ServerServletContainerInitializer.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/WebInfClassServletContainerInitializer.java similarity index 90% rename from jetty-annotations/src/test/java/org/eclipse/jetty/annotations/ServerServletContainerInitializer.java rename to jetty-annotations/src/test/java/org/eclipse/jetty/annotations/WebInfClassServletContainerInitializer.java index 3fbfadce4c5..f770b0d19db 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/ServerServletContainerInitializer.java +++ b/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 } diff --git a/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer b/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer index 8193f60e4a7..cc22e474171 100644 --- a/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer +++ b/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer @@ -1 +1 @@ -org.eclipse.jetty.annotations.ServerServletContainerInitializer \ No newline at end of file +org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer \ No newline at end of file