This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2a3b326b53927b6b880a539a342a95d5ac1e4ee8 Author: opencode <[email protected]> AuthorDate: Fri Oct 16 13:16:24 2026 +0200 Keep streams returned from ReferenceCountedJar open The stream-returning methods released the reference they opened before the caller could use the returned stream, so when no other reference was held the wrapped JAR was closed and the stream could no longer be read. They now transfer the opened reference to the returned stream: it is released when the stream is closed, exactly once, so the wrapped JAR stays open for the lifetime of the stream. This matches the Jar contract under which the caller is responsible for closing streams. A null stream and failed look-ups release the reference immediately. --- .../tomcat/util/scan/ReferenceCountedJar.java | 48 +++++- .../tomcat/util/scan/TestReferenceCountedJar.java | 182 +++++++++++++++++++++ 2 files changed, 226 insertions(+), 4 deletions(-) diff --git a/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java b/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java index a774104af9..83f3dc1bce 100644 --- a/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java +++ b/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java @@ -16,6 +16,7 @@ */ package org.apache.tomcat.util.scan; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.net.URL; @@ -68,6 +69,37 @@ public class ReferenceCountedJar implements Jar { } + /* + * Transfers the reference obtained by open() to the given stream obtained from the wrapped Jar. The reference is + * released when - and only when - the stream is closed, so the wrapped Jar remains open while the stream is in + * use. A null stream releases the reference immediately. + */ + private InputStream retainReference(InputStream is) { + if (is == null) { + close(); + return null; + } + + return new FilterInputStream(is) { + + private boolean released = false; + + @Override + public synchronized void close() throws IOException { + if (released) { + return; + } + released = true; + try { + in.close(); + } finally { + ReferenceCountedJar.this.close(); + } + } + }; + } + + @Override public URL getJarFileURL() { return url; @@ -76,8 +108,12 @@ public class ReferenceCountedJar implements Jar { @Override public InputStream getInputStream(String name) throws IOException { - try (ReferenceCountedJar jar = open()) { - return jar.wrappedJar.getInputStream(name); + ReferenceCountedJar jar = open(); + try { + return jar.retainReference(jar.wrappedJar.getInputStream(name)); + } catch (IOException | RuntimeException e) { + jar.close(); + throw e; } } @@ -120,8 +156,12 @@ public class ReferenceCountedJar implements Jar { @Override public InputStream getEntryInputStream() throws IOException { - try (ReferenceCountedJar jar = open()) { - return jar.wrappedJar.getEntryInputStream(); + ReferenceCountedJar jar = open(); + try { + return jar.retainReference(jar.wrappedJar.getEntryInputStream()); + } catch (IOException | RuntimeException e) { + jar.close(); + throw e; } } diff --git a/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java b/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java new file mode 100644 index 0000000000..77a6c8a117 --- /dev/null +++ b/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.scan; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import org.apache.catalina.webresources.TomcatURLStreamHandlerFactory; + +public class TestReferenceCountedJar { + + @Rule + public TemporaryFolder testDir = new TemporaryFolder(); + + private static final String ENTRY_NAME = "test/resource.txt"; + private static final String CONTENT = "hello"; + + @Before + public void register() { + TomcatURLStreamHandlerFactory.register(); + } + + + private File createJar() throws IOException { + Manifest manifest = new Manifest(); + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + + File file = new File(testDir.getRoot(), "test.jar"); + try (JarOutputStream jos = new JarOutputStream(new FileOutputStream(file), manifest)) { + jos.putNextEntry(new JarEntry(ENTRY_NAME)); + jos.write(CONTENT.getBytes(StandardCharsets.UTF_8)); + jos.closeEntry(); + } + return file; + } + + + private static String readFully(InputStream is) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int read = is.read(buffer); + while (read > 0) { + baos.write(buffer, 0, read); + read = is.read(buffer); + } + return baos.toString("UTF-8"); + } + + + /* + * The reference obtained internally when a stream is opened must be held until the stream is closed. Without it, + * reading a stream obtained after the wrapper has been closed and re-opened (the bug 62976 scenario) would fail + * because the wrapped JAR is closed as soon as getInputStream() returns. + */ + @Test + public void testInputStreamRemainsValidWhenWrapperHasNoOtherReference() throws Exception { + File file = createJar(); + URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/"); + + ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL()); + // Close the reference obtained by the constructor so the wrapper no longer holds any reference. + jar.close(); + + // This re-opens the wrapped JAR. The returned stream must stay usable even though no other reference is held. + try (InputStream is = jar.getInputStream(ENTRY_NAME)) { + Assert.assertNotNull(is); + Assert.assertEquals(CONTENT, readFully(is)); + } + + // Re-reading after the stream has been closed (releasing the reference it held) must still work. + try (InputStream is = jar.getInputStream(ENTRY_NAME)) { + Assert.assertEquals(CONTENT, readFully(is)); + } + } + + + /* + * When a caller holds a reference and iterates the entries (as jasper does when parsing tag files), the reference + * obtained for the current entry stream must be held until that stream is closed and must not, on close, release + * the caller's reference. + */ + @Test + public void testEntryInputStreamHoldsReferenceWhileIterating() throws Exception { + File file = createJar(); + URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/"); + + // Retain the constructor reference, as a real caller does across nextEntry()/getEntryInputStream(). + try (ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL())) { + boolean found = false; + jar.nextEntry(); + while (jar.getEntryName() != null) { + if (ENTRY_NAME.equals(jar.getEntryName())) { + try (InputStream is = jar.getEntryInputStream()) { + Assert.assertEquals(CONTENT, readFully(is)); + } + found = true; + } + jar.nextEntry(); + } + Assert.assertTrue(found); + + // Closing the entry stream must not have released the caller's reference. + try (InputStream is = jar.getInputStream(ENTRY_NAME)) { + Assert.assertEquals(CONTENT, readFully(is)); + } + } + } + + + /* + * Closing the returned stream more than once must release the held reference only once so the reference count is + * not decremented below the value to which the close of the stream should return it. + */ + @Test + public void testStreamDoubleCloseReleasesReferenceOnce() throws Exception { + File file = createJar(); + URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/"); + + // Here the constructor reference is retained, as a normal caller would do. + try (ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL())) { + InputStream is = jar.getInputStream(ENTRY_NAME); + is.close(); + is.close(); + + // The extra close must not have released the caller's reference. The JAR must still be readable through + // the caller's (still open) reference without being re-opened. + try (InputStream is2 = jar.getInputStream(ENTRY_NAME)) { + Assert.assertEquals(CONTENT, readFully(is2)); + } + } + } + + + /* + * A request for a stream for a name that does not exist must release the opened reference immediately and return + * null rather than holding a reference that is never released. + */ + @Test + public void testMissingEntryReleasesReference() throws Exception { + File file = createJar(); + URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/"); + + ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL()); + jar.close(); + + Assert.assertNull(jar.getInputStream("test/does-not-exist.txt")); + + // The failed request must not have left a reference held: a later successful request must still work. + try (InputStream is = jar.getInputStream(ENTRY_NAME)) { + Assert.assertEquals(CONTENT, readFully(is)); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
