From 4157876eeeddc34358641e4b5e720216aa10fa6f Mon Sep 17 00:00:00 2001 From: Ivan Bella Date: Thu, 23 Mar 2023 19:23:09 +0000 Subject: [PATCH 1/4] fixes #VFS-834: Updated to avoid prematurely closing file objects. --- .../commons/vfs2/impl/VFSClassLoader.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/VFSClassLoader.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/VFSClassLoader.java index a6c79ce7cb..e8f3932960 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/VFSClassLoader.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/VFSClassLoader.java @@ -239,10 +239,11 @@ protected Enumeration findResources(final String name) throws IOException { final List result = new ArrayList<>(2); for (final FileObject baseFile : resources) { - try (FileObject file = baseFile.resolveFile(name, NameScope.DESCENDENT_OR_SELF)) { - if (FileObjectUtils.exists(file)) { - result.add(new Resource(name, baseFile, file).getURL()); - } + final FileObject file = baseFile.resolveFile(name, NameScope.DESCENDENT_OR_SELF); + if (FileObjectUtils.exists(file)) { + result.add(new Resource(name, baseFile, file).getURL()); + } else { + file.close(); } } @@ -312,10 +313,11 @@ private boolean isSealed(final Resource res) throws FileSystemException { */ private Resource loadResource(final String name) throws FileSystemException { for (final FileObject baseFile : resources) { - try (FileObject file = baseFile.resolveFile(name, NameScope.DESCENDENT_OR_SELF)) { - if (FileObjectUtils.exists(file)) { - return new Resource(name, baseFile, file); - } + final FileObject file = baseFile.resolveFile(name, NameScope.DESCENDENT_OR_SELF); + if (FileObjectUtils.exists(file)) { + return new Resource(name, baseFile, file); + } else { + file.close(); } } return null; From e0fc2f285d68ff72347f060c351ec6cf95ff90c6 Mon Sep 17 00:00:00 2001 From: Ivan Bella Date: Thu, 23 Mar 2023 22:50:22 +0000 Subject: [PATCH 2/4] re #VFS-834: Added test cases which test for the bug. --- .../vfs2/AbstractProviderTestCase.java | 10 +++++++++- .../vfs2/impl/VfsClassLoaderTests.java | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java index cda51e1b7a..8774a974f9 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java @@ -114,6 +114,15 @@ protected void assertSameContent(final String expected, final FileObject file) t * are encoded using UTF-8. */ protected void assertSameURLContent(final String expected, final URLConnection connection) throws Exception { + assertSameURLContent(expected, connection.getInputStream(), connection); + } + + /** + * Asserts that the content of a file is the same as expected. Checks the length reported by getContentLength() is + * correct, then reads the content as a byte stream and compares the result with the expected content. Assumes files + * are encoded using UTF-8. + */ + protected void assertSameURLContent(final String expected, final InputStream instr, final URLConnection connection) throws Exception { // Get file content as a binary stream final byte[] expectedBin = expected.getBytes(StandardCharsets.UTF_8); @@ -121,7 +130,6 @@ protected void assertSameURLContent(final String expected, final URLConnection c assertEquals("same content length", expectedBin.length, connection.getContentLength()); // Read content into byte array - final InputStream instr = connection.getInputStream(); final ByteArrayOutputStream outstr; try { outstr = new ByteArrayOutputStream(); diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java index 533ba6763f..d234f4051c 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java @@ -19,6 +19,7 @@ import static org.apache.commons.vfs2.VfsTestUtils.getTestDirectoryFile; import java.io.File; +import java.io.InputStream; import java.io.PrintWriter; import java.net.URL; import java.net.URLConnection; @@ -196,11 +197,21 @@ public void testLoadClass() throws Exception { public void testLoadResource() throws Exception { final VFSClassLoader loader = createClassLoader(); - final URL resource = loader.getResource("read-tests/file1.txt"); + final URL resource1 = loader.getResource("read-tests/file1.txt"); + assertNotNull(resource1); + final URLConnection urlCon1 = resource1.openConnection(); + final InputStream instr1 = urlCon1.getInputStream(); - assertNotNull(resource); - final URLConnection urlCon = resource.openConnection(); - assertSameURLContent(FILE1_CONTENT, urlCon); + // VFS-834: testing that getting the resource again does not close out the previous input stream. + final URL resource2 = loader.getResource("read-tests/file1.txt"); + + assertSameURLContent(FILE1_CONTENT, instr1, urlCon1); + + assertNotNull(resource2); + final URLConnection urlCon2 = resource2.openConnection(); + final InputStream instr2 = urlCon1.getInputStream(); + + assertSameURLContent(FILE1_CONTENT, instr2, urlCon2); } /** From 5daf68f945629afc7ba0e6ef6a32911b7926f72a Mon Sep 17 00:00:00 2001 From: Ivan Bella Date: Fri, 24 Mar 2023 14:28:13 +0000 Subject: [PATCH 3/4] re #VFS-834: ensure resource cleaned up --- .../org/apache/commons/vfs2/AbstractProviderTestCase.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java index 8774a974f9..718f9edd81 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractProviderTestCase.java @@ -114,7 +114,9 @@ protected void assertSameContent(final String expected, final FileObject file) t * are encoded using UTF-8. */ protected void assertSameURLContent(final String expected, final URLConnection connection) throws Exception { - assertSameURLContent(expected, connection.getInputStream(), connection); + try (InputStream in = connection.getInputStream()) { + assertSameURLContent(expected, in, connection); + } } /** From 7923cac2a85006de1b2fad94725c14217f5f12f6 Mon Sep 17 00:00:00 2001 From: Ivan Bella Date: Fri, 24 Mar 2023 16:55:00 +0000 Subject: [PATCH 4/4] re #VFS-834: pushed the test case as far as I could and denoted the reason why additional step could not be taken --- .../org/apache/commons/vfs2/impl/VfsClassLoaderTests.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java index d234f4051c..64682e7793 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java @@ -204,12 +204,14 @@ public void testLoadResource() throws Exception { // VFS-834: testing that getting the resource again does not close out the previous input stream. final URL resource2 = loader.getResource("read-tests/file1.txt"); + assertNotNull(resource2); + final URLConnection urlCon2 = resource2.openConnection(); assertSameURLContent(FILE1_CONTENT, instr1, urlCon1); - assertNotNull(resource2); - final URLConnection urlCon2 = resource2.openConnection(); - final InputStream instr2 = urlCon1.getInputStream(); + // For tar files, getting the second input stream will reset the input (see TarFileSystem.resetTarFile()) + // hence we need to actually get the input stream after asserting the contents of the first one. + final InputStream instr2 = urlCon2.getInputStream(); assertSameURLContent(FILE1_CONTENT, instr2, urlCon2); }