From 0dc9f6c157392905be80df7560d625d565bd2caa Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 12:08:11 +0100 Subject: [PATCH 1/4] Fixes #5521 ResourceCollection list NPE NPE check. Note that I cannot work out how the none directory resource was added in the first place, so I was unable to reproduce the exact exception. --- .../jetty/util/resource/PathResource.java | 6 +----- .../jetty/util/resource/ResourceCollection.java | 4 +++- .../util/resource/ResourceCollectionTest.java | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index e9931f548e43..e29e285c4b11 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -571,11 +571,7 @@ public String[] list() int size = entries.size(); return entries.toArray(new String[size]); } - catch (DirectoryIteratorException e) - { - LOG.debug(e); - } - catch (IOException e) + catch (DirectoryIteratorException | IOException e) { LOG.debug(e); } 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 594ba49c3b3c..4b8b8818d179 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 @@ -434,10 +434,12 @@ public long length() public String[] list() { assertResourcesSet(); - HashSet set = new HashSet<>(); for (Resource r : _resources) { + String[] list = r.list(); + if (list != null) + Collections.addAll(set, list); Collections.addAll(set, r.list()); } String[] result = set.toArray(new String[0]); 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 51200037cce0..f307e57ca2c3 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 @@ -22,6 +22,7 @@ import java.io.File; import java.io.InputStreamReader; import java.nio.file.Path; +import java.util.Arrays; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; @@ -32,7 +33,9 @@ import org.junit.jupiter.api.extension.ExtendWith; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -174,6 +177,19 @@ private void assertThrowIllegalStateException(ResourceCollection coll) }); } + @Test + public void testList() throws Exception + { + ResourceCollection rc1 = new ResourceCollection( + Resource.newResource("src/test/resources/org/eclipse/jetty/util/resource/one/"), + Resource.newResource("src/test/resources/org/eclipse/jetty/util/resource/two/"), + Resource.newResource("src/test/resources/org/eclipse/jetty/util/resource/three/")); + + 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(), nullValue()); + } + @Test public void testMultipleSources1() throws Exception { From 156943701a02394d18cb5083be4e0706b547f1c5 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 12:51:21 +0100 Subject: [PATCH 2/4] Fixes #5521 ResourceCollection list NPE Refactor addPath in a simpler way, plus with TODOs to fix issues in 10 when merged. --- .../util/resource/ResourceCollection.java | 61 ++++++++----------- .../util/resource/ResourceCollectionTest.java | 3 +- 2 files changed, 27 insertions(+), 37 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 4b8b8818d179..e11c77136df9 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 @@ -256,51 +256,40 @@ public Resource addPath(String path) throws IOException return this; } - Resource resource = null; ArrayList resources = null; - int i = 0; - for (; i < _resources.length; i++) - { - resource = _resources[i].addPath(path); - if (resource.exists()) - { - if (resource.isDirectory()) - { - break; - } - return resource; - } - } - for (i++; i < _resources.length; i++) + // Attempt a simple (single) Resource lookup that exists + for (Resource res : _resources) { - Resource r = _resources[i].addPath(path); - if (r.exists() && r.isDirectory()) - { - if (resources == null) - { - resources = new ArrayList<>(); - } + Resource r = res.addPath(path); + if (!r.exists()) + continue; - if (resource != null) - { - resources.add(resource); - resource = null; - } + if (!r.isDirectory()) + return r; - resources.add(r); - } + if (resources == null) + resources = new ArrayList<>(); + resources.add(r); } - if (resource != null) - { - return resource; - } - if (resources != null) + if (resources == null) { - return new ResourceCollection(resources.toArray(new Resource[0])); + return null; /* TODO this is not allowed in 10. Instead do: + for (Resource res : _r1esources) + { + Resource r = res.addPath(path); + if (r.exists()) + return r; + } + return EmptyResource.INSTANCE; + */ } - return null; + + if (resources.size() == 1) + return resources.get(0); + + return new ResourceCollection(resources.toArray(new Resource[0])); } @Override 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 f307e57ca2c3..ef28688ef483 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 @@ -187,7 +187,8 @@ 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(), nullValue()); + assertThat(rc1.addPath("unknown"), nullValue()); + // TODO for jetty-10 assertThat(rc1.addPath("unknown").list(), nullValue()); } @Test From 6c7a63bbb5629cc857ef4930f81a8a70f2db6155 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 15:12:59 +0100 Subject: [PATCH 3/4] Fixes #5521 ResourceCollection list NPE Minimized changes in 9. Will do bulk of changes in 10. --- .../util/resource/ResourceCollection.java | 61 +++++++++++-------- .../util/resource/ResourceCollectionTest.java | 2 +- 2 files changed, 37 insertions(+), 26 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 e11c77136df9..4b8b8818d179 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 @@ -256,40 +256,51 @@ public Resource addPath(String path) throws IOException return this; } + Resource resource = null; ArrayList resources = null; - - // Attempt a simple (single) Resource lookup that exists - for (Resource res : _resources) + int i = 0; + for (; i < _resources.length; i++) { - Resource r = res.addPath(path); - if (!r.exists()) - continue; - - if (!r.isDirectory()) - return r; - - if (resources == null) - resources = new ArrayList<>(); - resources.add(r); + resource = _resources[i].addPath(path); + if (resource.exists()) + { + if (resource.isDirectory()) + { + break; + } + return resource; + } } - if (resources == null) + for (i++; i < _resources.length; i++) { - return null; /* TODO this is not allowed in 10. Instead do: - for (Resource res : _r1esources) + Resource r = _resources[i].addPath(path); + if (r.exists() && r.isDirectory()) { - Resource r = res.addPath(path); - if (r.exists()) - return r; + if (resources == null) + { + resources = new ArrayList<>(); + } + + if (resource != null) + { + resources.add(resource); + resource = null; + } + + resources.add(r); } - return EmptyResource.INSTANCE; - */ } - if (resources.size() == 1) - return resources.get(0); - - return new ResourceCollection(resources.toArray(new Resource[0])); + if (resource != null) + { + return resource; + } + if (resources != null) + { + return new ResourceCollection(resources.toArray(new Resource[0])); + } + return null; } @Override 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 ef28688ef483..d3e212517c6f 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 @@ -187,7 +187,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"), nullValue()); + assertThat(rc1.addPath("unknown").list(), nullValue()); // TODO for jetty-10 assertThat(rc1.addPath("unknown").list(), nullValue()); } From 2446ca3b5d668143e01e8cb6a3b287854bb9e7d1 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 28 Oct 2020 15:21:57 +0100 Subject: [PATCH 4/4] Fixes #5521 ResourceCollection list NPE Fix from review. remove duplicate line. --- .../java/org/eclipse/jetty/util/resource/ResourceCollection.java | 1 - 1 file changed, 1 deletion(-) 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 4b8b8818d179..3f080072465a 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 @@ -440,7 +440,6 @@ public String[] list() String[] list = r.list(); if (list != null) Collections.addAll(set, list); - Collections.addAll(set, r.list()); } String[] result = set.toArray(new String[0]); Arrays.sort(result);