This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new e0c76f6 Rework the fix for BZ 64021 for better custom class loader
support
e0c76f6 is described below
commit e0c76f691936fb4c9a679866adf2bbc575e55383
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Sun Apr 12 17:05:35 2020 +0100
Rework the fix for BZ 64021 for better custom class loader support
https://bz.apache.org/bugzilla/show_bug.cgi?id=64021
Better support custom class loaders that load resources from
non-standard locations and do not utilise the WebResources
implementation.
---
.../catalina/startup/WebappServiceLoader.java | 119 +++++++++++++--------
.../catalina/startup/TestWebappServiceLoader.java | 10 +-
webapps/docs/changelog.xml | 5 +
3 files changed, 86 insertions(+), 48 deletions(-)
diff --git a/java/org/apache/catalina/startup/WebappServiceLoader.java
b/java/org/apache/catalina/startup/WebappServiceLoader.java
index bd3bfe5..e222940 100644
--- a/java/org/apache/catalina/startup/WebappServiceLoader.java
+++ b/java/org/apache/catalina/startup/WebappServiceLoader.java
@@ -26,15 +26,16 @@ import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Set;
import java.util.regex.Pattern;
import jakarta.servlet.ServletContext;
import org.apache.catalina.Context;
-import org.apache.catalina.WebResource;
import org.apache.tomcat.util.scan.JarFactory;
/**
@@ -67,6 +68,7 @@ public class WebappServiceLoader<T> {
private final ServletContext servletContext;
private final Pattern containerSciFilterPattern;
+
/**
* Construct a loader to load services from a ServletContext.
*
@@ -83,8 +85,14 @@ public class WebappServiceLoader<T> {
}
}
+
/**
- * Load the providers for a service type.
+ * Load the providers for a service type. Container defined services will
be
+ * loaded before application defined services in case the application
+ * depends on a Container provided service. Note that services are always
+ * loaded via the Context (web application) class loader so it is possible
+ * for an application to provide an alternative implementation of what
would
+ * normally be a Container provided service.
*
* @param serviceType the type of service to load
* @return an unmodifiable collection of service providers
@@ -93,21 +101,64 @@ public class WebappServiceLoader<T> {
public List<T> load(Class<T> serviceType) throws IOException {
String configFile = SERVICES + serviceType.getName();
- LinkedHashSet<String> applicationServicesFound = new LinkedHashSet<>();
- LinkedHashSet<String> containerServicesFound = new LinkedHashSet<>();
+ // Obtain the Container provided service configuration files.
+ ClassLoader loader = context.getParentClassLoader();
+ Enumeration<URL> containerResources;
+ if (loader == null) {
+ containerResources = ClassLoader.getSystemResources(configFile);
+ } else {
+ containerResources = loader.getResources(configFile);
+ }
+
+ // Extract the Container provided service class names. Each
+ // configuration file may list more than one service class name. This
+ // uses a LinkedHashSet so if a service class name appears more than
+ // once in the configuration files, only the first one found is used.
+ LinkedHashSet<String> containerServiceClassNames = new
LinkedHashSet<>();
+ Set<URL> containerServiceConfigFiles = new HashSet<>();
+ while (containerResources.hasMoreElements()) {
+ URL containerServiceConfigFile = containerResources.nextElement();
+ containerServiceConfigFiles.add(containerServiceConfigFile);
+ parseConfigFile(containerServiceClassNames,
containerServiceConfigFile);
+ }
+
+ // Filter the discovered container SCIs if required
+ if (containerSciFilterPattern != null) {
+ Iterator<String> iter = containerServiceClassNames.iterator();
+ while (iter.hasNext()) {
+ if (containerSciFilterPattern.matcher(iter.next()).find()) {
+ iter.remove();
+ }
+ }
+ }
- // 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
+ // Obtaining the application provided configuration files is a little
+ // more difficult for two reasons:
+ // - The web application may employ a custom class loader. Ideally, we
+ // would use ClassLoader.findResources() but that method is
protected.
+ // We could force custom class loaders to override that method and
+ // make it public but that would be a new requirement and break
+ // backwards compatibility for what is an often customised component.
+ // - If the application web.xml file has defined an order for fragments
+ // then only those JAR files represented by fragments in that order
+ // (and arguably WEB-INF/classes) should be scanned for services.
+ LinkedHashSet<String> applicationServiceClassNames = new
LinkedHashSet<>();
+
+ // Check to see if the ServletContext has ORDERED_LIBS defined
@SuppressWarnings("unchecked")
List<String> orderedLibs = (List<String>)
servletContext.getAttribute(ServletContext.ORDERED_LIBS);
- // Handle application SCIs directly...
+ // Obtain the application provided service configuration files
if (orderedLibs == null) {
- // No ordered libs, so use every service definition we can find
- WebResource[] resources =
context.getResources().getClassLoaderResources("/" + configFile);
- for (WebResource resource : resources) {
- if (resource.isFile()) {
- parseConfigFile(applicationServicesFound,
resource.getURL());
+ // Because a custom class loader may be being used, we have to use
+ // getResources() which will return application and Container
files.
+ Enumeration<URL> allResources =
servletContext.getClassLoader().getResources(configFile);
+ while (allResources.hasMoreElements()) {
+ URL serviceConfigFile = allResources.nextElement();
+ // Only process the service configuration file if it is not a
+ // Container level file that has already been processed
+ if (!containerServiceConfigFiles.contains(serviceConfigFile)) {
+ parseConfigFile(applicationServiceClassNames,
serviceConfigFile);
}
}
} else {
@@ -115,7 +166,7 @@ public class WebappServiceLoader<T> {
// in WEB-INF/classes
URL unpacked = servletContext.getResource(CLASSES + configFile);
if (unpacked != null) {
- parseConfigFile(applicationServicesFound, unpacked);
+ parseConfigFile(applicationServiceClassNames, unpacked);
}
for (String lib : orderedLibs) {
@@ -133,49 +184,27 @@ public class WebappServiceLoader<T> {
url = JarFactory.getJarEntryURL(jarUrl, configFile);
}
try {
- parseConfigFile(applicationServicesFound, url);
+ parseConfigFile(applicationServiceClassNames, url);
} catch (FileNotFoundException e) {
// no provider file found, this is OK
}
}
}
- // and use the parent ClassLoader for all other SCIs
- ClassLoader loader = context.getParentClassLoader();
-
- Enumeration<URL> resources;
- if (loader == null) {
- resources = ClassLoader.getSystemResources(configFile);
- } else {
- resources = loader.getResources(configFile);
- }
- while (resources.hasMoreElements()) {
- parseConfigFile(containerServicesFound, resources.nextElement());
- }
-
- // Filter the discovered container SCIs if required
- if (containerSciFilterPattern != null) {
- Iterator<String> iter = containerServicesFound.iterator();
- while (iter.hasNext()) {
- if (containerSciFilterPattern.matcher(iter.next()).find()) {
- iter.remove();
- }
- }
- }
-
// Add the application services after the container services to ensure
// that the container services are loaded first
- containerServicesFound.addAll(applicationServicesFound);
+ containerServiceClassNames.addAll(applicationServiceClassNames);
- // load the discovered services
- if (containerServicesFound.isEmpty()) {
+ // Short-cut if no services have been found
+ if (containerServiceClassNames.isEmpty()) {
return Collections.emptyList();
}
- return loadServices(serviceType, containerServicesFound);
+ // Load the discovered services
+ return loadServices(serviceType, containerServiceClassNames);
}
- void parseConfigFile(LinkedHashSet<String> servicesFound, URL url)
- throws IOException {
+
+ void parseConfigFile(LinkedHashSet<String> servicesFound, URL url) throws
IOException {
try (InputStream is = url.openStream();
InputStreamReader in = new InputStreamReader(is,
StandardCharsets.UTF_8);
BufferedReader reader = new BufferedReader(in)) {
@@ -194,8 +223,8 @@ public class WebappServiceLoader<T> {
}
}
- List<T> loadServices(Class<T> serviceType, LinkedHashSet<String>
servicesFound)
- throws IOException {
+
+ List<T> loadServices(Class<T> serviceType, LinkedHashSet<String>
servicesFound) throws IOException {
ClassLoader loader = servletContext.getClassLoader();
List<T> services = new ArrayList<>(servicesFound.size());
for (String serviceClass : servicesFound) {
diff --git a/test/org/apache/catalina/startup/TestWebappServiceLoader.java
b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
index e3f3dda..3e4d1e2 100644
--- a/test/org/apache/catalina/startup/TestWebappServiceLoader.java
+++ b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
@@ -65,6 +65,8 @@ public class TestWebappServiceLoader {
@Test
public void testNoInitializersFound() throws IOException {
loader = new WebappServiceLoader<>(context);
+ EasyMock.expect(cl.getResources(CONFIG_FILE))
+ .andReturn(Collections.<URL>emptyEnumeration());
EasyMock.expect(servletContext.getAttribute(ServletContext.ORDERED_LIBS))
.andReturn(null);
EasyMock.expect(cl.getResources(CONFIG_FILE))
@@ -81,11 +83,13 @@ public class TestWebappServiceLoader {
loader = EasyMock.createMockBuilder(WebappServiceLoader.class)
.addMockedMethod("parseConfigFile", LinkedHashSet.class,
URL.class)
.withConstructor(context).createMock(control);
+ EasyMock.expect(cl.getResources(CONFIG_FILE))
+
.andReturn(Collections.enumeration(Collections.singleton(url)));
+ loader.parseConfigFile(EasyMock.isA(LinkedHashSet.class),
EasyMock.same(url));
EasyMock.expect(servletContext.getAttribute(ServletContext.ORDERED_LIBS))
.andReturn(null);
EasyMock.expect(cl.getResources(CONFIG_FILE))
.andReturn(Collections.enumeration(Collections.singleton(url)));
- loader.parseConfigFile(EasyMock.isA(LinkedHashSet.class),
EasyMock.same(url));
control.replay();
Assert.assertTrue(loader.load(ServletContainerInitializer.class).isEmpty());
control.verify();
@@ -102,6 +106,8 @@ public class TestWebappServiceLoader {
.addMockedMethod("parseConfigFile", LinkedHashSet.class,
URL.class)
.withConstructor(context).createMock(control);
List<String> jars = Arrays.asList("jar1.jar", "dir/");
+ EasyMock.expect(parent.getResources(CONFIG_FILE))
+ .andReturn(Collections.<URL>emptyEnumeration());
EasyMock.expect(servletContext.getAttribute(ServletContext.ORDERED_LIBS))
.andReturn(jars);
EasyMock.expect(servletContext.getResource("/WEB-INF/classes/" +
CONFIG_FILE))
@@ -112,8 +118,6 @@ public class TestWebappServiceLoader {
EasyMock.expect(servletContext.getResource("/WEB-INF/lib/dir/"))
.andReturn(url2);
loader.parseConfigFile(EasyMock.isA(LinkedHashSet.class),
EasyMock.eq(sci2));
- EasyMock.expect(parent.getResources(CONFIG_FILE))
- .andReturn(Collections.<URL>emptyEnumeration());
control.replay();
Assert.assertTrue(loader.load(ServletContainerInitializer.class).isEmpty());
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ab27bb6..69ad10e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -66,6 +66,11 @@
Reduce reflection use and remove AJP specific code in the Connector.
(remm/markt/fhanik)
</fix>
+ <fix>
+ Rework the fix for <bug>64021</bug> to better support web applications
+ that use a custom class loader that loads resources from non-standard
+ locations. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
