cxf-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CXF-7597) Suspicious calls to ClassLoader.findResource when resolving absolute base and actual URIs
Date Sat, 13 Jan 2018 09:08:00 GMT

    [ 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:
users@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)

Mime
View raw message