You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2018/01/13 09:07:41 UTC

[GitHub] ffang closed pull request #363: [CXF-7597] Fix suspicious class loader findResource calls

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


With regards,
Apache Git Services