You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2020/04/12 18:48:09 UTC
[tomcat] branch 7.0.x updated (2def975 -> b732c45)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 2def975 Fix typo. Better test coverage.
new 57bc709 Make JAR URL generation consistent with how JRE constructs JAR URLs
new b732c45 Rework the fix for BZ 64021 for better custom class loader support
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../catalina/startup/WebappServiceLoader.java | 116 +++++++-----
java/org/apache/tomcat/util/buf/UriUtil.java | 2 +-
.../catalina/startup/TestWebappServiceLoader.java | 197 +++++++++++++++++++--
.../org/apache/catalina/startup/service-config.txt | 8 +-
webapps/docs/changelog.xml | 5 +
5 files changed, 265 insertions(+), 63 deletions(-)
copy java/org/apache/catalina/ha/deploy/LocalStrings_ru.properties => test/org/apache/catalina/startup/service-config.txt (75%)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/02: Rework the fix for BZ 64021 for better custom class
loader support
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b732c45fb3550b9d83984b11bf7009b1511bffac
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Sun Apr 12 19:47:22 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 | 116 +++++++-----
.../catalina/startup/TestWebappServiceLoader.java | 197 +++++++++++++++++++--
.../org/apache/catalina/startup/service-config.txt | 20 +++
webapps/docs/changelog.xml | 5 +
4 files changed, 279 insertions(+), 59 deletions(-)
diff --git a/java/org/apache/catalina/startup/WebappServiceLoader.java b/java/org/apache/catalina/startup/WebappServiceLoader.java
index efdf002..eb5c11e 100644
--- a/java/org/apache/catalina/startup/WebappServiceLoader.java
+++ b/java/org/apache/catalina/startup/WebappServiceLoader.java
@@ -22,14 +22,15 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URL;
-import java.net.URLClassLoader;
import java.nio.charset.Charset;
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 javax.servlet.ServletContext;
@@ -68,6 +69,7 @@ public class WebappServiceLoader<T> {
private final ServletContext servletContext;
private final Pattern containerSciFilterPattern;
+
/**
* Construct a loader to load services from a ServletContext.
*
@@ -84,8 +86,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
@@ -94,24 +102,64 @@ public class WebappServiceLoader<T> {
public List<T> load(Class<T> serviceType) throws IOException {
String configFile = SERVICES + serviceType.getName();
- LinkedHashSet<String> applicationServicesFound = new LinkedHashSet<String>();
- LinkedHashSet<String> containerServicesFound = new LinkedHashSet<String>();
+ // 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<String>();
+ Set<URL> containerServiceConfigFiles = new HashSet<URL>();
+ while (containerResources.hasMoreElements()) {
+ URL containerServiceConfigFile = containerResources.nextElement();
+ containerServiceConfigFiles.add(containerServiceConfigFile);
+ parseConfigFile(containerServiceClassNames, containerServiceConfigFile);
+ }
- ClassLoader loader = servletContext.getClassLoader();
+ // 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<String>();
+
+ // 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
- if (loader instanceof URLClassLoader) {
- Enumeration<URL> resources = ((URLClassLoader) loader).findResources(configFile);
- while (resources.hasMoreElements()) {
- URL resource = resources.nextElement();
- parseConfigFile(applicationServicesFound, resource);
+ // 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 {
@@ -119,7 +167,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) {
@@ -137,48 +185,26 @@ public class WebappServiceLoader<T> {
url = UriUtil.buildJarUrl(base, 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
- 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);
}
- private void parseConfigFile(LinkedHashSet<String> servicesFound, URL url)
+ void parseConfigFile(LinkedHashSet<String> servicesFound, URL url)
throws IOException {
InputStream is = null;
BufferedReader reader = null;
@@ -208,7 +234,7 @@ public class WebappServiceLoader<T> {
}
}
- private List<T> loadServices(Class<T> serviceType, LinkedHashSet<String> servicesFound)
+ List<T> loadServices(Class<T> serviceType, LinkedHashSet<String> servicesFound)
throws IOException {
ClassLoader loader = servletContext.getClassLoader();
List<T> services = new ArrayList<T>(servicesFound.size());
diff --git a/test/org/apache/catalina/startup/TestWebappServiceLoader.java b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
index 5e30245..dddf9dc 100644
--- a/test/org/apache/catalina/startup/TestWebappServiceLoader.java
+++ b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
@@ -16,27 +16,196 @@
*/
package org.apache.catalina.startup;
-import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
import javax.servlet.ServletContainerInitializer;
+import javax.servlet.ServletContext;
+import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
-import org.apache.catalina.core.StandardContext;
+import org.apache.catalina.Context;
+import org.apache.tomcat.unittest.TesterContext;
+import org.easymock.EasyMock;
+import org.easymock.IMocksControl;
+
+public class TestWebappServiceLoader {
+ private static final String CONFIG_FILE =
+ "META-INF/services/javax.servlet.ServletContainerInitializer";
+ private IMocksControl control;
+ private ClassLoader cl;
+ private ClassLoader parent;
+ private Context context;
+ private ServletContext servletContext;
+ private WebappServiceLoader<ServletContainerInitializer> loader;
+
+ @Before
+ public void init() {
+ control = EasyMock.createStrictControl();
+ parent = control.createMock(ClassLoader.class);
+ cl = EasyMock.createMockBuilder(ClassLoader.class)
+ .withConstructor(parent)
+ .addMockedMethod("loadClass", String.class)
+ .createMock(control);
+ servletContext = control.createMock(ServletContext.class);
+ EasyMock.expect(servletContext.getClassLoader()).andStubReturn(cl);
+ context = new ExtendedTesterContext(servletContext, parent);
+ }
+
+ @Test
+ public void testNoInitializersFound() throws IOException {
+ loader = new WebappServiceLoader<ServletContainerInitializer>(context);
+ EasyMock.expect(cl.getResources(CONFIG_FILE))
+ .andReturn(Collections.<URL>enumeration(Collections.<URL>emptyList()));
+ EasyMock.expect(servletContext.getAttribute(ServletContext.ORDERED_LIBS))
+ .andReturn(null);
+ EasyMock.expect(cl.getResources(CONFIG_FILE))
+ .andReturn(Collections.<URL>enumeration(Collections.<URL>emptyList()));
+ control.replay();
+ Assert.assertTrue(loader.load(ServletContainerInitializer.class).isEmpty());
+ control.verify();
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testInitializerFromClasspath() throws IOException {
+ URL url = new URL("file://test");
+ 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)));
+ control.replay();
+ Assert.assertTrue(loader.load(ServletContainerInitializer.class).isEmpty());
+ control.verify();
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testWithOrdering() throws IOException {
+ URL url1 = new URL("file://jar1.jar");
+ URL sci1 = new URL("jar:file://jar1.jar!/" + CONFIG_FILE);
+ URL url2 = new URL("file://dir/");
+ URL sci2 = new URL("file://dir/" + CONFIG_FILE);
+ loader = EasyMock.createMockBuilder(WebappServiceLoader.class)
+ .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>enumeration(Collections.<URL>emptyList()));
+ EasyMock.expect(servletContext.getAttribute(ServletContext.ORDERED_LIBS))
+ .andReturn(jars);
+ EasyMock.expect(servletContext.getResource("/WEB-INF/classes/" + CONFIG_FILE))
+ .andReturn(null);
+ EasyMock.expect(servletContext.getResource("/WEB-INF/lib/jar1.jar"))
+ .andReturn(url1);
+ loader.parseConfigFile(EasyMock.isA(LinkedHashSet.class), EasyMock.eq(sci1));
+ EasyMock.expect(servletContext.getResource("/WEB-INF/lib/dir/"))
+ .andReturn(url2);
+ loader.parseConfigFile(EasyMock.isA(LinkedHashSet.class), EasyMock.eq(sci2));
+
+ control.replay();
+ Assert.assertTrue(loader.load(ServletContainerInitializer.class).isEmpty());
+ control.verify();
+ }
+
+ @Test
+ public void testParseConfigFile() throws IOException {
+ LinkedHashSet<String> found = new LinkedHashSet<String>();
+ loader = new WebappServiceLoader<ServletContainerInitializer>(context);
+ loader.parseConfigFile(found, getClass().getResource("service-config.txt"));
+ Assert.assertEquals(Collections.singleton("provider1"), found);
+ }
+
+ @Test
+ public void testLoadServices() throws Exception {
+ Class<?> sci = TesterServletContainerInitializer1.class;
+ loader = new WebappServiceLoader<ServletContainerInitializer>(context);
+ cl.loadClass(sci.getName());
+ EasyMock.expectLastCall()
+ .andReturn(sci);
+ LinkedHashSet<String> names = new LinkedHashSet<String>();
+ names.add(sci.getName());
+ control.replay();
+ Collection<ServletContainerInitializer> initializers =
+ loader.loadServices(ServletContainerInitializer.class, names);
+ Assert.assertEquals(1, initializers.size());
+ Assert.assertTrue(sci.isInstance(initializers.iterator().next()));
+ control.verify();
+ }
+
+ @Test
+ public void testServiceIsNotExpectedType() throws Exception {
+ Class<?> sci = Object.class;
+ loader = new WebappServiceLoader<ServletContainerInitializer>(context);
+ cl.loadClass(sci.getName());
+ EasyMock.expectLastCall()
+ .andReturn(sci);
+ LinkedHashSet<String> names = new LinkedHashSet<String>();
+ names.add(sci.getName());
+ control.replay();
+ try {
+ loader.loadServices(ServletContainerInitializer.class, names);
+ } catch (IOException e) {
+ Assert.assertTrue(e.getCause() instanceof ClassCastException);
+ } finally {
+ control.verify();
+ }
+ }
-public class TestWebappServiceLoader extends TomcatBaseTest {
@Test
- public void testWebapp() throws Exception {
- Tomcat tomcat = getTomcatInstance();
- File appDir = new File("test/webapp-fragments-empty-absolute-ordering");
- StandardContext ctxt = (StandardContext) tomcat.addContext(null, "/test", appDir.getAbsolutePath());
- ctxt.addLifecycleListener(new ContextConfig());
- tomcat.start();
-
- WebappServiceLoader<ServletContainerInitializer> loader =
- new WebappServiceLoader<ServletContainerInitializer>(ctxt);
- @SuppressWarnings("unused")
- Collection<ServletContainerInitializer> initializers = loader.load(ServletContainerInitializer.class);
+ public void testServiceCannotBeConstructed() throws Exception {
+ Class<?> sci = Integer.class;
+ loader = new WebappServiceLoader<ServletContainerInitializer>(context);
+ cl.loadClass(sci.getName());
+ EasyMock.expectLastCall()
+ .andReturn(sci);
+ LinkedHashSet<String> names = new LinkedHashSet<String>();
+ names.add(sci.getName());
+ control.replay();
+ try {
+ loader.loadServices(ServletContainerInitializer.class, names);
+ } catch (IOException e) {
+ Assert.assertTrue(e.getCause() instanceof InstantiationException);
+ } finally {
+ control.verify();
+ }
+ }
+
+ private static class ExtendedTesterContext extends TesterContext {
+ private final ServletContext servletContext;
+ private final ClassLoader parent;
+
+ public ExtendedTesterContext(ServletContext servletContext, ClassLoader parent) {
+ this.servletContext = servletContext;
+ this.parent = parent;
+ }
+
+ @Override
+ public ServletContext getServletContext() {
+ return servletContext;
+ }
+
+ @Override
+ public String getContainerSciFilter() {
+ return "";
+ }
+
+ @Override
+ public ClassLoader getParentClassLoader() {
+ return parent;
+ }
}
}
diff --git a/test/org/apache/catalina/startup/service-config.txt b/test/org/apache/catalina/startup/service-config.txt
new file mode 100644
index 0000000..a2081b0
--- /dev/null
+++ b/test/org/apache/catalina/startup/service-config.txt
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This is a test file for the WebappServiceLoader
+# It contains comment lines and blank lines
+
+provider1 # This comment should be ignored
+provider1 # provider 1 should only be returned once
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 79e9f9f..65cb37c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -80,6 +80,11 @@
replacement in configuration files. Based on a pull request provided by
Bernd Bohmann. (markt)
</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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/02: Make JAR URL generation consistent with how JRE
constructs JAR URLs
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 57bc7099fda29af35c2d8e1af1d5e92618a829f5
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Sun Apr 12 19:42:15 2020 +0100
Make JAR URL generation consistent with how JRE constructs JAR URLs
---
java/org/apache/tomcat/util/buf/UriUtil.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/buf/UriUtil.java b/java/org/apache/tomcat/util/buf/UriUtil.java
index bb32d3b..c034a59 100644
--- a/java/org/apache/tomcat/util/buf/UriUtil.java
+++ b/java/org/apache/tomcat/util/buf/UriUtil.java
@@ -93,7 +93,7 @@ public final class UriUtil {
if (entryPath != null) {
sb.append(makeSafeForJarUrl(entryPath));
}
- return new URL("jar", null, -1, sb.toString());
+ return new URL("jar", "", -1, sb.toString());
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org