From a1e837530eecaee9e3abe1f780eca72721df6d14 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 15:42:36 +0100 Subject: [PATCH 1/4] Fixes #5521 ResourceCollection NPE Fix constructor and addPath so that all resources in a RC must exist when created. --- .../util/resource/ResourceCollection.java | 47 ++++++++++--------- .../util/resource/ResourceCollectionTest.java | 3 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index 4b7484081f1d..f67232b1ab0f 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -62,8 +62,19 @@ public ResourceCollection() * @param resources the resources to be added to collection */ public ResourceCollection(Resource... resources) + { + this(Arrays.asList(resources)); + } + + /** + * Instantiates a new resource collection. + * + * @param resources the resources to be added to collection + */ + public ResourceCollection(Collection resources) { _resources = new ArrayList<>(); + for (Resource r : resources) { if (r == null) @@ -82,17 +93,6 @@ public ResourceCollection(Resource... resources) } } - /** - * Instantiates a new resource collection. - * - * @param resources the resources to be added to collection - */ - public ResourceCollection(Collection resources) - { - _resources = new ArrayList<>(); - _resources.addAll(resources); - } - /** * Instantiates a new resource collection. * @@ -247,27 +247,28 @@ public Resource addPath(String path) throws IOException ArrayList resources = null; // Attempt a simple (single) Resource lookup that exists + Resource addedResource = null; for (Resource res : _resources) { - Resource r = res.addPath(path); - if (!r.isDirectory() && r.exists()) - { - // Return simple (non-directory) Resource - return r; - } - + addedResource = res.addPath(path); + if (!addedResource.exists()) + continue; + if (!addedResource.isDirectory()) + return addedResource; // Return simple (non-directory) Resource if (resources == null) - { resources = new ArrayList<>(); - } + resources.add(addedResource); + } - resources.add(r); + if (resources == null) + { + if (addedResource != null) + return addedResource; // This will not exist + return EmptyResource.INSTANCE; } if (resources.size() == 1) - { return resources.get(0); - } return new ResourceCollection(resources); } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java index fc39c573f476..cb5313fe8d0e 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java @@ -35,7 +35,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -189,7 +188,7 @@ public void testList() throws Exception assertThat(Arrays.asList(rc1.list()), contains("1.txt", "2.txt", "3.txt", "dir/")); assertThat(Arrays.asList(rc1.addPath("dir").list()), contains("1.txt", "2.txt", "3.txt")); - assertThat(rc1.addPath("unknown").list(), emptyArray()); + assertThat(rc1.addPath("unknown").list(), nullValue()); } @Test From 60665745ebd8f586c9418ade001937ac5e5c4859 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 16:12:21 +0100 Subject: [PATCH 2/4] Fixes #5521 ResourceCollection NPE Cleanup the vestiges of non existent directories detected by resource ending in / --- .../docs/programming/server/http/HTTPServerDocs.java | 4 ++-- .../eclipse/jetty/util/resource/JarFileResource.java | 9 ++------- .../java/org/eclipse/jetty/util/resource/Resource.java | 2 -- .../jetty/util/resource/ResourceCollection.java | 1 - .../org/eclipse/jetty/util/resource/URLResource.java | 5 ----- .../org/eclipse/jetty/util/resource/ResourceTest.java | 6 +++--- .../org/eclipse/jetty/webapp/MetaInfConfiguration.java | 4 ++-- .../java/org/eclipse/jetty/webapp/WebAppContext.java | 3 +-- .../org/eclipse/jetty/webapp/WebInfConfiguration.java | 10 +++++----- 9 files changed, 15 insertions(+), 29 deletions(-) diff --git a/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java b/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java index 89e21d7e027e..26a4b255a0dd 100644 --- a/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java +++ b/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java @@ -658,8 +658,8 @@ public void multipleResourcesHandler() throws Exception // For multiple directories, use ResourceCollection. ResourceCollection directories = new ResourceCollection(); - directories.addPath("/path/to/static/resources/"); - directories.addPath("/another/path/to/static/resources/"); + directories.addPath("/path/to/static/resources"); + directories.addPath("/another/path/to/static/resources"); handler.setBaseResource(directories); // end::multipleResourcesHandler[] diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java index 736f605df70f..43576a25c0f9 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java @@ -144,7 +144,7 @@ public boolean exists() String fileUrl = _urlString.substring(4, _urlString.length() - 2); try { - return newResource(fileUrl).exists(); + return _directory = newResource(fileUrl).exists(); } catch (Exception e) { @@ -236,15 +236,10 @@ else if (entry.isDirectory()) return _exists; } - /** - * Returns true if the represented resource is a container/directory. - * If the resource is not a file, resources ending with "/" are - * considered directories. - */ @Override public boolean isDirectory() { - return _urlString.endsWith("/") || exists() && _directory; + return exists() && _directory; } /** diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 4aa957715393..dd179265bea1 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -320,8 +320,6 @@ public static boolean isContainedIn(Resource r, Resource containingResource) thr /** * @return true if the represented resource is a container/directory. - * if the resource is not a file, resources ending with "/" are - * considered directories. */ public abstract boolean isDirectory(); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index f67232b1ab0f..8987a01abc50 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -385,7 +385,6 @@ public URI getURI() public boolean isDirectory() { assertResourcesSet(); - return true; } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java index 3dbd48ffa025..557bf2fbfbbd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java @@ -128,11 +128,6 @@ public boolean exists() return _in != null; } - /** - * Returns true if the represented resource is a container/directory. - * If the resource is not a file, resources ending with "/" are - * considered directories. - */ @Override public boolean isDirectory() { diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java index ddb5153ff6b1..d2a639ab411e 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java @@ -215,15 +215,15 @@ public static Stream scenarios() throws Exception cases.addCase(new Scenario(tdata1, "alphabet.txt", EXISTS, !DIR, "ABCDEFGHIJKLMNOPQRSTUVWXYZ")); cases.addCase(new Scenario(tdata2, "alphabet.txt", EXISTS, !DIR, "ABCDEFGHIJKLMNOPQRSTUVWXYZ")); - cases.addCase(new Scenario("jar:file:/somejar.jar!/content/", !EXISTS, DIR)); - cases.addCase(new Scenario("jar:file:/somejar.jar!/", !EXISTS, DIR)); + cases.addCase(new Scenario("jar:file:/somejar.jar!/content/", !EXISTS, !DIR)); + cases.addCase(new Scenario("jar:file:/somejar.jar!/", !EXISTS, !DIR)); String urlRef = cases.uriRef.toASCIIString(); Scenario zdata = new Scenario("jar:" + urlRef + "TestData/test.zip!/", EXISTS, DIR); cases.addCase(zdata); cases.addCase(new Scenario(zdata, "Unknown", !EXISTS, !DIR)); - cases.addCase(new Scenario(zdata, "/Unknown/", !EXISTS, DIR)); + cases.addCase(new Scenario(zdata, "/Unknown/", !EXISTS, !DIR)); cases.addCase(new Scenario(zdata, "subdir", EXISTS, DIR)); cases.addCase(new Scenario(zdata, "/subdir/", EXISTS, DIR)); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java index 422df3c47a01..41519cf50ecf 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java @@ -766,7 +766,7 @@ protected List findWebInfLibJars(WebAppContext context) return null; List jarResources = new ArrayList(); - Resource webInfLib = webInf.addPath("/lib"); + Resource webInfLib = webInf.addPath("lib"); if (webInfLib.exists() && webInfLib.isDirectory()) { String[] files = webInfLib.list(); @@ -834,7 +834,7 @@ protected Resource findWebInfClassesDir(WebAppContext context) if (webInf != null && webInf.isDirectory()) { // Look for classes directory - Resource classes = webInf.addPath("classes/"); + Resource classes = webInf.addPath("classes"); if (classes.exists()) return classes; } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index dafd33f0a08f..da9ab6e3ccf0 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -31,7 +31,6 @@ import java.util.EventListener; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -835,7 +834,7 @@ public Resource getWebInf() throws IOException return null; // Iw there a WEB-INF directory? - Resource webInf = super.getBaseResource().addPath("WEB-INF/"); + Resource webInf = super.getBaseResource().addPath("WEB-INF"); if (!webInf.exists() || !webInf.isDirectory()) return null; diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java index 09999ab56d9e..5ee2406420c5 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -70,12 +70,12 @@ public void configure(WebAppContext context) throws Exception if (webInf != null && webInf.isDirectory() && context.getClassLoader() instanceof WebAppClassLoader) { // Look for classes directory - Resource classes = webInf.addPath("classes/"); + Resource classes = webInf.addPath("classes"); if (classes.exists()) ((WebAppClassLoader)context.getClassLoader()).addClassPath(classes); // Look for jars - Resource lib = webInf.addPath("lib/"); + Resource lib = webInf.addPath("lib"); if (lib.exists() || lib.isDirectory()) ((WebAppClassLoader)context.getClassLoader()).addJars(lib); } @@ -413,13 +413,13 @@ public void unpack(WebAppContext context) throws IOException // Do we need to extract WEB-INF/lib? if (context.isCopyWebInf() && !context.isCopyWebDir()) { - Resource webInf = webApp.addPath("WEB-INF/"); + Resource webInf = webApp.addPath("WEB-INF"); File extractedWebInfDir = new File(context.getTempDirectory(), "webinf"); if (extractedWebInfDir.exists()) IO.delete(extractedWebInfDir); extractedWebInfDir.mkdir(); - Resource webInfLib = webInf.addPath("lib/"); + Resource webInfLib = webInf.addPath("lib"); File webInfDir = new File(extractedWebInfDir, "WEB-INF"); webInfDir.mkdir(); @@ -435,7 +435,7 @@ public void unpack(WebAppContext context) throws IOException webInfLib.copyTo(webInfLibDir); } - Resource webInfClasses = webInf.addPath("classes/"); + Resource webInfClasses = webInf.addPath("classes"); if (webInfClasses.exists()) { File webInfClassesDir = new File(webInfDir, "classes"); From ae4f9f6fa841d1b56a8b3714864ff41558c59752 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 23:23:11 +0100 Subject: [PATCH 3/4] Fixes #5521 ResourceCollection NPE Revert adding paths ending in / as jar:file resource needs them --- .../docs/programming/server/http/HTTPServerDocs.java | 4 ++-- .../org/eclipse/jetty/webapp/MetaInfConfiguration.java | 4 ++-- .../java/org/eclipse/jetty/webapp/WebAppContext.java | 2 +- .../org/eclipse/jetty/webapp/WebInfConfiguration.java | 10 +++++----- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java b/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java index 26a4b255a0dd..89e21d7e027e 100644 --- a/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java +++ b/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java @@ -658,8 +658,8 @@ public void multipleResourcesHandler() throws Exception // For multiple directories, use ResourceCollection. ResourceCollection directories = new ResourceCollection(); - directories.addPath("/path/to/static/resources"); - directories.addPath("/another/path/to/static/resources"); + directories.addPath("/path/to/static/resources/"); + directories.addPath("/another/path/to/static/resources/"); handler.setBaseResource(directories); // end::multipleResourcesHandler[] diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java index 41519cf50ecf..422df3c47a01 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java @@ -766,7 +766,7 @@ protected List findWebInfLibJars(WebAppContext context) return null; List jarResources = new ArrayList(); - Resource webInfLib = webInf.addPath("lib"); + Resource webInfLib = webInf.addPath("/lib"); if (webInfLib.exists() && webInfLib.isDirectory()) { String[] files = webInfLib.list(); @@ -834,7 +834,7 @@ protected Resource findWebInfClassesDir(WebAppContext context) if (webInf != null && webInf.isDirectory()) { // Look for classes directory - Resource classes = webInf.addPath("classes"); + Resource classes = webInf.addPath("classes/"); if (classes.exists()) return classes; } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index da9ab6e3ccf0..b2a6e553a4f8 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -834,7 +834,7 @@ public Resource getWebInf() throws IOException return null; // Iw there a WEB-INF directory? - Resource webInf = super.getBaseResource().addPath("WEB-INF"); + Resource webInf = super.getBaseResource().addPath("WEB-INF/"); if (!webInf.exists() || !webInf.isDirectory()) return null; diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java index 5ee2406420c5..09999ab56d9e 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -70,12 +70,12 @@ public void configure(WebAppContext context) throws Exception if (webInf != null && webInf.isDirectory() && context.getClassLoader() instanceof WebAppClassLoader) { // Look for classes directory - Resource classes = webInf.addPath("classes"); + Resource classes = webInf.addPath("classes/"); if (classes.exists()) ((WebAppClassLoader)context.getClassLoader()).addClassPath(classes); // Look for jars - Resource lib = webInf.addPath("lib"); + Resource lib = webInf.addPath("lib/"); if (lib.exists() || lib.isDirectory()) ((WebAppClassLoader)context.getClassLoader()).addJars(lib); } @@ -413,13 +413,13 @@ public void unpack(WebAppContext context) throws IOException // Do we need to extract WEB-INF/lib? if (context.isCopyWebInf() && !context.isCopyWebDir()) { - Resource webInf = webApp.addPath("WEB-INF"); + Resource webInf = webApp.addPath("WEB-INF/"); File extractedWebInfDir = new File(context.getTempDirectory(), "webinf"); if (extractedWebInfDir.exists()) IO.delete(extractedWebInfDir); extractedWebInfDir.mkdir(); - Resource webInfLib = webInf.addPath("lib"); + Resource webInfLib = webInf.addPath("lib/"); File webInfDir = new File(extractedWebInfDir, "WEB-INF"); webInfDir.mkdir(); @@ -435,7 +435,7 @@ public void unpack(WebAppContext context) throws IOException webInfLib.copyTo(webInfLibDir); } - Resource webInfClasses = webInf.addPath("classes"); + Resource webInfClasses = webInf.addPath("classes/"); if (webInfClasses.exists()) { File webInfClassesDir = new File(webInfDir, "classes"); From 1acef42b5d8f3def272a6c3bdd183bbf834d9878 Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 3 Nov 2020 15:06:51 +0100 Subject: [PATCH 4/4] feedback from review improved javadoc. --- .../java/org/eclipse/jetty/util/resource/Resource.java | 2 +- .../jetty/util/resource/ResourceCollection.java | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index dd179265bea1..657c7998eaa0 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -410,7 +410,7 @@ public abstract boolean renameTo(Resource dest) /** * Returns the resource contained inside the current resource with the - * given name. + * given name, which may or may not exist. * * @param path The path segment to add, which is not encoded * @return the Resource for the resolved path within this Resource, never null diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index 8987a01abc50..4ef8d68c7a8f 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -226,8 +226,16 @@ public void setResources(String resources) throws IOException } /** + * Add a path to the resource collection. * @param path The path segment to add - * @return The contained resource (found first) in the collection of resources + * @return The resulting resource(s) : + *
    + *
  • is a file that exists in at least one of the collection, then the first one found is returned
    • + *
  • is a directory that exists in at exactly one of the collection, then that directory resource is returned
    • + *
  • is a directory that exists in several of the collection, then a ResourceCollection of those directories is returned
    • + *
  • do not exist in any of the collection, then a new non existent resource relative to the first in the collection is returned.
    • + *
+ * @throws MalformedURLException if the resolution of the path fails because the input path parameter is malformed against any of the collection */ @Override public Resource addPath(String path) throws IOException