[
https://issues.apache.org/jira/browse/CXF-7597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16325056#comment-16325056
]
ASF GitHub Bot commented on CXF-7597:
-------------------------------------
ffang closed pull request #363: [CXF-7597] Fix suspicious class loader
findResource calls
URL: https://github.com/apache/cxf/pull/363
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/core/src/main/java/org/apache/cxf/resource/URIResolver.java
b/core/src/main/java/org/apache/cxf/resource/URIResolver.java
index 019935865f4..5a9e43d7e55 100644
--- a/core/src/main/java/org/apache/cxf/resource/URIResolver.java
+++ b/core/src/main/java/org/apache/cxf/resource/URIResolver.java
@@ -83,7 +83,8 @@ public URIResolver(String baseUriStr, String uriStr, Class<?>
calling) throws IO
} else if (baseUriStr != null
&& (baseUriStr.startsWith("jar:")
|| baseUriStr.startsWith("zip:")
- || baseUriStr.startsWith("wsjar:"))) {
+ || baseUriStr.startsWith("wsjar:"))
+ && !isAbsolute(uriStr)) {
tryArchive(baseUriStr, uriStr);
} else if (uriStr.startsWith("jar:")
|| uriStr.startsWith("zip:")
@@ -112,7 +113,8 @@ public void resolve(String baseUriStr, String uriStr,
Class<?> callingCls) throw
} else if (baseUriStr != null
&& (baseUriStr.startsWith("jar:")
|| baseUriStr.startsWith("zip:")
- || baseUriStr.startsWith("wsjar:"))) {
+ || baseUriStr.startsWith("wsjar:"))
+ && !isAbsolute(uriStr)) {
tryArchive(baseUriStr, uriStr);
} else if (uriStr.startsWith("jar:")
|| uriStr.startsWith("zip:")
@@ -123,7 +125,13 @@ public void resolve(String baseUriStr, String uriStr,
Class<?> callingCls) throw
}
}
-
+ private boolean isAbsolute(String uriStr) {
+ try {
+ return new URI(uriStr).isAbsolute();
+ } catch (URISyntaxException e) {
+ return false;
+ }
+ }
private void tryFileSystem(String baseUriStr, String uriStr) throws
IOException, MalformedURLException {
try {
diff --git a/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java
b/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java
index 4ef6338fcb4..9efb8f00417 100644
--- a/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java
+++ b/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java
@@ -32,6 +32,7 @@
private URL resourceURL =
getClass().getResource("resources/helloworld.bpr");
+ private Throwable checkingThreadThrowable; // assumes single-thread
test execution
@Test
public void testJARProtocol() throws Exception {
@@ -57,6 +58,9 @@ public void testJARProtocol() throws Exception {
is2.read(barray2);
is2.close();
assertEquals(IOUtils.newStringFromBytes(barray),
IOUtils.newStringFromBytes(barray2));
+
+ resolveWithCheckingClassloader(uriResolver, "baseUriStr", uriStr,
null);
+ resolveWithCheckingClassloaderInConstructor("baseUriStr", uriStr,
null);
}
@Test
@@ -79,8 +83,30 @@ public void testJARResolver() throws Exception {
InputStream is3 = uriResolver.getInputStream();
assertNotNull(is3);
+
+ resolveWithCheckingClassloader(uriResolver, uriStr,
"hello_world_2.wsdl", null);
+ resolveWithCheckingClassloaderInConstructor(uriStr,
"hello_world_2.wsdl", null);
}
+ @Test
+ public void testJARResolveBaseAndAbsolute() throws Exception {
+ uriResolver = new URIResolver();
+
+ String baseUriStr = "jar:" + resourceURL.toString() +
"!/wsdl/hello_world.wsdl";
+
+ URL jarURL = new URL(baseUriStr);
+ InputStream is = jarURL.openStream();
+ assertNotNull(is);
+
+ String uriStr = "jar:" + resourceURL.toString() +
"!/wsdl/hello_world_2.wsdl";
+
+ URL jarURL2 = new URL(uriStr);
+ InputStream is2 = jarURL2.openStream();
+ assertNotNull(is2);
+
+ resolveWithCheckingClassloader(uriResolver, baseUriStr, uriStr, null);
+ resolveWithCheckingClassloaderInConstructor(baseUriStr, uriStr, null);
+ }
@Test
public void testResolveRelativeFile() throws Exception {
@@ -98,12 +124,15 @@ public void testResolveRelativeFile() throws Exception {
URIResolver xsdResolver = new URIResolver();
xsdResolver.resolve(baseUri, schemaLocation, this.getClass());
assertNotNull(xsdResolver.getInputStream());
+ resolveWithCheckingClassloader(xsdResolver, baseUri, schemaLocation,
this.getClass());
+ resolveWithCheckingClassloaderInConstructor(baseUri, schemaLocation,
this.getClass());
// resolve the schema using relative location with base uri fragment
xsdResolver = new URIResolver();
xsdResolver.resolve(baseUri + "#type2", schemaLocation,
this.getClass());
assertNotNull(xsdResolver.getInputStream());
-
+ resolveWithCheckingClassloader(xsdResolver, baseUri + "#type2",
schemaLocation, this.getClass());
+ resolveWithCheckingClassloaderInConstructor(baseUri + "#type2",
schemaLocation, this.getClass());
}
@Test
@@ -122,12 +151,15 @@ public void testResolvePathWithSpace() throws Exception {
URIResolver xsdResolver = new URIResolver();
xsdResolver.resolve(baseUri, schemaLocation, this.getClass());
assertNotNull(xsdResolver.getInputStream());
+ resolveWithCheckingClassloader(xsdResolver, baseUri, schemaLocation,
this.getClass());
+ resolveWithCheckingClassloaderInConstructor(baseUri, schemaLocation,
this.getClass());
// resolve the schema using relative location with base uri fragment
xsdResolver = new URIResolver();
xsdResolver.resolve(baseUri + "#type2", schemaLocation,
this.getClass());
assertNotNull(xsdResolver.getInputStream());
-
+ resolveWithCheckingClassloader(xsdResolver, baseUri + "#type2",
schemaLocation, this.getClass());
+ resolveWithCheckingClassloaderInConstructor(baseUri + "#type2",
schemaLocation, this.getClass());
}
@Test
@@ -136,6 +168,8 @@ public void testBasePathWithSpace() throws Exception {
// resolve the wsdl
wsdlResolver.resolve(null, "wsdl/folder with spaces/foo.wsdl",
this.getClass());
assertTrue(wsdlResolver.isResolved());
+ resolveWithCheckingClassloader(wsdlResolver, null, "wsdl/folder with
spaces/foo.wsdl", this.getClass());
+ resolveWithCheckingClassloaderInConstructor(null, "wsdl/folder with
spaces/foo.wsdl", this.getClass());
}
@Test
@@ -144,5 +178,62 @@ public void testBasePathWithEncodedSpace() throws
Exception {
// resolve the wsdl
wsdlResolver.resolve(null, "wsdl/folder%20with%20spaces/foo.wsdl",
this.getClass());
assertTrue(wsdlResolver.isResolved());
+ resolveWithCheckingClassloader(wsdlResolver, null,
"wsdl/folder%20with%20spaces/foo.wsdl", this.getClass());
+ resolveWithCheckingClassloaderInConstructor(null,
"wsdl/folder%20with%20spaces/foo.wsdl", this.getClass());
+ }
+
+ private void resolveWithCheckingClassloader(URIResolver resolver, String
baseUriStr, String uriStr,
+ Class<?> callingCls) throws InterruptedException {
+ checkingThreadThrowable = null;
+ Runnable operation = () -> {
+ try {
+ resolver.resolve(baseUriStr, uriStr, callingCls);
+ } catch (Throwable t) {
+ checkingThreadThrowable = t;
+ }
+ assertNotNull(resolver.getInputStream());
+ };
+ Thread thread = new Thread(operation);
+ thread.setContextClassLoader(new
CheckingClassLoader(this.getClass().getClassLoader()));
+ thread.start();
+ thread.join(10000);
+ assertFalse("resolve operation did not finish in time",
thread.isAlive());
+ assertNull(checkingThreadThrowable);
+ }
+
+ private void resolveWithCheckingClassloaderInConstructor(String
baseUriStr, String uriStr, Class<?> callingCls)
+ throws InterruptedException {
+ checkingThreadThrowable = null;
+ Runnable operation = () -> {
+ URIResolver resolver = null;
+ try {
+ resolver = new URIResolver(baseUriStr, uriStr, callingCls);
+ } catch (Throwable t) {
+ checkingThreadThrowable = t;
+ }
+ if (resolver != null) {
+ assertNotNull(resolver.getInputStream());
+ }
+ };
+ Thread thread = new Thread(operation);
+ thread.setContextClassLoader(new
CheckingClassLoader(this.getClass().getClassLoader()));
+ thread.start();
+ thread.join(10000);
+ assertFalse("resolve operation did not finish in time",
thread.isAlive());
+ assertNull(checkingThreadThrowable);
+ }
+
+ static class CheckingClassLoader extends ClassLoader {
+ CheckingClassLoader(ClassLoader parent) {
+ super(parent);
+ }
+
+ @Override
+ protected URL findResource(String name) {
+ if (name != null && name.startsWith("jar:file:")) {
+ throw new AssertionError("Suspicious resource name: " + name);
+ }
+ return super.findResource(name);
+ }
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Suspicious calls to ClassLoader.findResource when resolving absolute base and
> actual URIs
> -----------------------------------------------------------------------------------------
>
> Key: CXF-7597
> URL: https://issues.apache.org/jira/browse/CXF-7597
> Project: CXF
> Issue Type: Bug
> Affects Versions: 3.2.1
> Reporter: Pavol Mederly
> Assignee: Freeman Fang
>
> When {{URIResolver.resolve(..)}} is called with both {{baseUriStr}} and
> {{uriStr}} containing absolute URIs, e.g.
> {quote}
> # *baseUriStr* =
> jar:file:/C:/midpoint/tgit/cxf/core/target/test-classes/org/apache/cxf/resource/resources/helloworld.bpr!/wsdl/hello_world.wsdl
> # *uriStr* =
> jar:file:/C:/midpoint/tgit/cxf/core/target/test-classes/org/apache/cxf/resource/resources/helloworld.bpr!/wsdl/hello_world_2.wsdl
> {quote}
> then it makes suspicious calls to a class loader, trying to find resources
> with names like
> {quote}
> #
> jar:file:/C:/midpoint/tgit/cxf/core/target/test-classes/org/apache/cxf/resource/resources/helloworld.bpr!/wsdl/hello_world_2.wsdl
> #
> /jar:file:/C:/midpoint/tgit/cxf/core/target/test-classes/org/apache/cxf/resource/resources/helloworld.bpr!/wsdl/hello_world_2.wsdl
> #
> jar:file:/C:/midpoint/tgit/cxf/core/target/test-classes/org/apache/cxf/resource/resources/helloworld.bpr!/wsdl/hello_world_2.wsdl
> {quote}
> In our case (when used as part of the
> [midPoint|https://github.com/Evolveum/midpoint/] product) the situation is
> like this:
> Resolving:
> {quote}
> # *baseUriStr* =
> jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/xml/ns/public/common/common-3.xsd
> # *uriStr* =
> jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd
> {quote}
> leads first to the resolution of obviously wrong URL:
> bq.
> jar:file:/C:/tmp/mp/lib/midpoint.war!jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd
> (note the two midpoint.war segments there)
> and then to the resolution of the following resource name (via class loader):
> bq.
> jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd
> Note that as per ClassLoader javadocs, "The name of a resource is a
> '/'-separated path name that identifies the resource." Although formally the
> above URIs match this definition, some class loader implementations, namely
> the one in Spring Boot which we use in midPoint, react to such requests in an
> unexpected way, returning wrong (unrelated) content. The result is that
> {{URIResolver.resolve(..)}} returns the wrong content as well. See [Spring
> Boot issue
> #11367|https://github.com/spring-projects/spring-boot/issues/11367].
> Even when Spring Boot is eventually fixed, {{classLoader.findResource(..)}}
> calls are unnecessary overhead.
> Please see
> {{[URIResolverTest.testJARResolveBaseAndAbsolute|https://github.com/Evolveum/cxf/commit/44d99924db52f2a4b5bdacc41bd83b81bffb8cb4#diff-565a425d43d14b14b2dbb4a251b04697R92]}}
> in the upcoming commit as well as the proposed
> [fix|https://github.com/Evolveum/cxf/commit/44d99924db52f2a4b5bdacc41bd83b81bffb8cb4#diff-b7160a28d60b729a956bd1a5f5ffa351]
> in {{URIResolver}}.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
