Author: markt Date: Wed Jan 15 13:09:01 2014 New Revision: 1558371 URL: http://svn.apache.org/r1558371 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55287 ServletContainerInitializer in container classloaders may not be found
Added: tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java - copied, changed from r1507870, tomcat/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java - copied, changed from r1507870, tomcat/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1507870,1508259,1510271,1524707,1524719,1524727 Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1558371&r1=1558370&r2=1558371&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java Wed Jan 15 13:09:01 2014 @@ -16,14 +16,11 @@ */ package org.apache.catalina.startup; -import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.UnsupportedEncodingException; import java.net.JarURLConnection; import java.net.MalformedURLException; import java.net.URISyntaxException; @@ -117,9 +114,6 @@ public class ContextConfig implements Li private static final Log log = LogFactory.getLog( ContextConfig.class ); - private static final String SCI_LOCATION = - "META-INF/services/javax.servlet.ServletContainerInitializer"; - /** * The string resources for this package. @@ -1269,7 +1263,7 @@ public class ContextConfig implements Li // Step 3. Look for ServletContainerInitializer implementations if (ok) { - processServletContainerInitializers(orderedFragments); + processServletContainerInitializers(context.getServletContext()); } if (!webXml.isMetadataComplete() || typeInitializerMap.size() > 0) { @@ -1539,149 +1533,63 @@ public class ContextConfig implements Li /** * Scan JARs for ServletContainerInitializer implementations. - * Implementations will be added in web-fragment.xml priority order. */ - protected void processServletContainerInitializers( - Set<WebXml> fragments) { + protected void processServletContainerInitializers(ServletContext servletContext) { - for (WebXml fragment : fragments) { - URL url = fragment.getURL(); - Jar jar = null; - InputStream is = null; - List<ServletContainerInitializer> detectedScis = null; + List<ServletContainerInitializer> detectedScis; + try { + WebappServiceLoader<ServletContainerInitializer> loader = + new WebappServiceLoader<ServletContainerInitializer>(servletContext); + detectedScis = loader.load(ServletContainerInitializer.class); + } catch (IOException e) { + log.error(sm.getString( + "contextConfig.servletContainerInitializerFail", + context.getName()), + e); + ok = false; + return; + } + + for (ServletContainerInitializer sci : detectedScis) { + initializerClassMap.put(sci, new HashSet<Class<?>>()); + + HandlesTypes ht; try { - if ("jar".equals(url.getProtocol())) { - jar = JarFactory.newInstance(url); - is = jar.getInputStream(SCI_LOCATION); - } else if ("file".equals(url.getProtocol())) { - String path = url.getPath(); - File file = new File(path, SCI_LOCATION); - if (file.exists()) { - is = new FileInputStream(file); - } - } - if (is != null) { - detectedScis = getServletContainerInitializers(is); - } - } catch (IOException ioe) { - log.error(sm.getString( - "contextConfig.servletContainerInitializerFail", url, - context.getName())); - ok = false; - return; - } finally { - if (is != null) { - try { - is.close(); - } catch (IOException e) { - // Ignore - } - } - if (jar != null) { - jar.close(); + ht = sci.getClass().getAnnotation(HandlesTypes.class); + } catch (Exception e) { + if (log.isDebugEnabled()) { + log.info(sm.getString("contextConfig.sci.debug", + sci.getClass().getName()), + e); + } else { + log.info(sm.getString("contextConfig.sci.info", + sci.getClass().getName())); } + continue; } - - if (detectedScis == null) { + if (ht == null) { continue; } - - for (ServletContainerInitializer sci : detectedScis) { - initializerClassMap.put(sci, new HashSet<Class<?>>()); - - HandlesTypes ht = null; - try { - ht = sci.getClass().getAnnotation(HandlesTypes.class); - } catch (Exception e) { - if (log.isDebugEnabled()) { - log.info(sm.getString("contextConfig.sci.debug", url), - e); - } else { - log.info(sm.getString("contextConfig.sci.info", url)); - } - } - if (ht != null) { - Class<?>[] types = ht.value(); - if (types != null) { - for (Class<?> type : types) { - if (type.isAnnotation()) { - handlesTypesAnnotations = true; - } else { - handlesTypesNonAnnotations = true; - } - Set<ServletContainerInitializer> scis = typeInitializerMap - .get(type); - if (scis == null) { - scis = new HashSet<ServletContainerInitializer>(); - typeInitializerMap.put(type, scis); - } - scis.add(sci); - } - } - } + Class<?>[] types = ht.value(); + if (types == null) { + continue; } - } - } - - - /** - * Extract the name of the ServletContainerInitializer. - * - * @param is The resource where the name is defined - * @return The class name - * @throws IOException - */ - protected List<ServletContainerInitializer> getServletContainerInitializers( - InputStream is) throws IOException { - List<ServletContainerInitializer> initializers = new ArrayList<ServletContainerInitializer>(); - - if (is != null) { - String line = null; - try { - BufferedReader br = new BufferedReader(new InputStreamReader( - is, "UTF-8")); - while ((line = br.readLine()) != null) { - line = line.trim(); - if (line.length() > 0) { - int i = line.indexOf('#'); - if (i > -1) { - if (i == 0) { - continue; - } - line = line.substring(0, i).trim(); - } - initializers.add(getServletContainerInitializer(line)); - } + for (Class<?> type : types) { + if (type.isAnnotation()) { + handlesTypesAnnotations = true; + } else { + handlesTypesNonAnnotations = true; + } + Set<ServletContainerInitializer> scis = + typeInitializerMap.get(type); + if (scis == null) { + scis = new HashSet<ServletContainerInitializer>(); + typeInitializerMap.put(type, scis); } - } catch (UnsupportedEncodingException e) { - // Should never happen with UTF-8 - // If it does - ignore & return null + scis.add(sci); } } - - return initializers; - } - - protected ServletContainerInitializer getServletContainerInitializer( - String className) throws IOException { - ServletContainerInitializer sci = null; - try { - Class<?> clazz = Class.forName(className, true, context.getLoader() - .getClassLoader()); - sci = (ServletContainerInitializer) clazz.newInstance(); - } catch (ClassNotFoundException e) { - log.error(sm.getString("contextConfig.invalidSci", className), e); - throw new IOException(e); - } catch (InstantiationException e) { - log.error(sm.getString("contextConfig.invalidSci", className), e); - throw new IOException(e); - } catch (IllegalAccessException e) { - log.error(sm.getString("contextConfig.invalidSci", className), e); - throw new IOException(e); - } - - return sci; } Copied: tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java (from r1507870, tomcat/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java) URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java?p2=tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java&p1=tomcat/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java&r1=1507870&r2=1558371&rev=1558371&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/WebappServiceLoader.java Wed Jan 15 13:09:01 2014 @@ -24,21 +24,20 @@ import java.io.InputStreamReader; import java.net.URL; import java.nio.charset.Charset; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.Enumeration; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; import javax.servlet.ServletContext; /** - * A variation of Java's JAR ServiceLoader that respects exclusion rules for web applications. + * A variation of Java's JAR ServiceLoader that respects exclusion rules for + * web applications. * <p/> - * Primarily intended for use loading ServletContainerInitializers as defined by Servlet 8.2.4. - * This implementation does not attempt lazy loading as the container is required to - * introspect all implementations discovered. + * Primarily intended for use loading ServletContainerInitializers as defined + * by Servlet 8.2.4. This implementation does not attempt lazy loading as the + * container is required to introspect all implementations discovered. * <p/> * If the ServletContext defines ORDERED_LIBS, then only JARs in WEB-INF/lib * that are named in that set will be included in the search for @@ -74,16 +73,19 @@ public class WebappServiceLoader<T> { * @return an unmodifiable collection of service providers * @throws IOException if there was a problem loading any service */ - public Collection<T> load(Class<T> serviceType) throws IOException { + public List<T> load(Class<T> serviceType) throws IOException { String configFile = SERVICES + serviceType.getName(); - Set<String> servicesFound = new HashSet<>(); + LinkedHashSet<String> applicationServicesFound = new LinkedHashSet<String>(); + LinkedHashSet<String> containerServicesFound = new LinkedHashSet<String>(); + ClassLoader loader = context.getClassLoader(); // if the ServletContext has ORDERED_LIBS, then use that to specify the // set of JARs from WEB-INF/lib that should be used for loading services @SuppressWarnings("unchecked") - List<String> orderedLibs = (List<String>) context.getAttribute(ServletContext.ORDERED_LIBS); + List<String> orderedLibs = + (List<String>) context.getAttribute(ServletContext.ORDERED_LIBS); if (orderedLibs != null) { // handle ordered libs directly, ... for (String lib : orderedLibs) { @@ -101,7 +103,7 @@ public class WebappServiceLoader<T> { url = new URL("jar:" + base + "!/" + configFile); } try { - parseConfigFile(servicesFound, url); + parseConfigFile(applicationServicesFound, url); } catch (FileNotFoundException e) { // no provider file found, this is OK } @@ -118,19 +120,27 @@ public class WebappServiceLoader<T> { resources = loader.getResources(configFile); } while (resources.hasMoreElements()) { - parseConfigFile(servicesFound, resources.nextElement()); + parseConfigFile(containerServicesFound, resources.nextElement()); } + // Add the application services after the container services to ensure + // that the container services are loaded first + containerServicesFound.addAll(applicationServicesFound); + // load the discovered services - if (servicesFound.isEmpty()) { + if (containerServicesFound.isEmpty()) { return Collections.emptyList(); } - return loadServices(serviceType, servicesFound); + return loadServices(serviceType, containerServicesFound); } - private void parseConfigFile(Set<String> servicesFound, URL url) throws IOException { - try (InputStream is = url.openStream()) { - BufferedReader reader = new BufferedReader(new InputStreamReader(is, UTF8)); + private void parseConfigFile(LinkedHashSet<String> servicesFound, URL url) + throws IOException { + InputStream is = null; + try { + is = url.openStream(); + InputStreamReader in = new InputStreamReader(is, UTF8); + BufferedReader reader = new BufferedReader(in); String line; while ((line = reader.readLine()) != null) { int i = line.indexOf('#'); @@ -141,25 +151,33 @@ public class WebappServiceLoader<T> { if (line.length() == 0) { continue; } - if (servicesFound.contains(line)) { - continue; - } servicesFound.add(line); } + } finally { + if (is != null) { + is.close(); + } } } - private Collection<T> loadServices(Class<T> serviceType, Set<String> servicesFound) throws IOException { + private List<T> loadServices(Class<T> serviceType, LinkedHashSet<String> servicesFound) + throws IOException { ClassLoader loader = context.getClassLoader(); - List<T> services = new ArrayList<>(servicesFound.size()); + List<T> services = new ArrayList<T>(servicesFound.size()); for (String serviceClass : servicesFound) { try { Class<?> clazz = Class.forName(serviceClass, true, loader); services.add(serviceType.cast(clazz.newInstance())); - } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | ClassCastException e) { + } catch (ClassNotFoundException e) { + throw new IOException(e); + } catch (InstantiationException e) { + throw new IOException(e); + } catch (IllegalAccessException e) { + throw new IOException(e); + } catch (ClassCastException e) { throw new IOException(e); } } - return Collections.unmodifiableCollection(services); + return Collections.unmodifiableList(services); } } Copied: tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java (from r1507870, tomcat/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java) URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java?p2=tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java&p1=tomcat/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java&r1=1507870&r2=1558371&rev=1558371&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TestWebappServiceLoader.java Wed Jan 15 13:09:01 2014 @@ -30,13 +30,13 @@ public class TestWebappServiceLoader ext @Test public void testWebapp() throws Exception { Tomcat tomcat = getTomcatInstance(); - File appDir = new File("test/webapp-fragments-empty-absolute-ordering"); + File appDir = new File("test/webapp-3.0-fragments-empty-absolute-ordering"); StandardContext ctxt = (StandardContext) tomcat.addContext(null, "/test", appDir.getAbsolutePath()); ctxt.addLifecycleListener(new ContextConfig()); tomcat.start(); WebappServiceLoader<ServletContainerInitializer> loader = - new WebappServiceLoader<>(ctxt.getServletContext()); + new WebappServiceLoader<ServletContainerInitializer>(ctxt.getServletContext()); Collection<ServletContainerInitializer> initializers = loader.load(ServletContainerInitializer.class); } } Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1558371&r1=1558370&r2=1558371&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Jan 15 13:09:01 2014 @@ -59,6 +59,10 @@ <subsection name="Catalina"> <changelog> <fix> + <bug>55287</bug>: <code>ServletContainerInitializer</code> defined in + the container may not be found. (markt/jboynes) + </fix> + <fix> <bug>55937</bug>: When deploying applications, treat a context path of <code>/ROOT</code> as equivalent to <code>/</code>. (markt) </fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org