From 1904d326fc58fa1775e6ce20f29c87aec77064e4 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 4 Nov 2020 08:56:17 +0100 Subject: [PATCH] Fixes #5521 ResourceCollection NPE (#5527) * Fixes #5521 ResourceCollection NPE Fix constructor and addPath so that all resources in a RC must exist when created. * Fixes #5521 ResourceCollection NPE Cleanup the vestiges of non existent directories detected by resource ending in / * Fixes #5521 ResourceCollection NPE Revert adding paths ending in / as jar:file resource needs them * feedback from review improved javadoc. --- .../jetty/util/resource/JarFileResource.java | 9 +-- .../eclipse/jetty/util/resource/Resource.java | 4 +- .../util/resource/ResourceCollection.java | 58 +++++++++++-------- .../jetty/util/resource/URLResource.java | 5 -- .../util/resource/ResourceCollectionTest.java | 3 +- .../jetty/util/resource/ResourceTest.java | 6 +- .../eclipse/jetty/webapp/WebAppContext.java | 1 - 7 files changed, 40 insertions(+), 46 deletions(-) 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..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 @@ -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(); @@ -412,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 4b7484081f1d..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 @@ -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. * @@ -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 @@ -247,27 +255,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); } @@ -384,7 +393,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/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 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/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index dafd33f0a08f..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 @@ -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;