This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 01968c2d8b Review "Close the descriptor stream in the digester modeler
source"
01968c2d8b is described below
commit 01968c2d8bbce3b9b351a0de9e01aa5e412e58f9
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Nov 3 18:37:23 2026 +0000
Review "Close the descriptor stream in the digester modeler source"
Apply a broader fix that catches more error cases
Co-authored by: Claude / Sonnet 5.5
---
.../tomcat/util/modeler/LocalStrings.properties | 1 +
java/org/apache/tomcat/util/modeler/Registry.java | 32 ++++++--
.../modules/MbeansDescriptorsDigesterSource.java | 4 +-
.../TestMbeansDescriptorsDigesterSource.java | 86 ++++++++++++++++++++++
4 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/tomcat/util/modeler/LocalStrings.properties
b/java/org/apache/tomcat/util/modeler/LocalStrings.properties
index ed806f5a5b..379659557d 100644
--- a/java/org/apache/tomcat/util/modeler/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/modeler/LocalStrings.properties
@@ -43,6 +43,7 @@ modules.readDescriptorsError=Error reading descriptors
noJmxMBeanServer.mBeanNotFound=The MBean [{0}] was not found. JMX support is
disabled.
registry.cannotAccessRegistry=Guard object [{0}] does not allow access to the
registry
+registry.closeSourceError=Failed to close the source of the descriptors
registry.createdServer=Created MBeanServer
registry.existingServer=Using existing MBeanServer
registry.initError=Error initializing [{0}]
diff --git a/java/org/apache/tomcat/util/modeler/Registry.java
b/java/org/apache/tomcat/util/modeler/Registry.java
index 6a47fd6bfc..13e6c054e9 100644
--- a/java/org/apache/tomcat/util/modeler/Registry.java
+++ b/java/org/apache/tomcat/util/modeler/Registry.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.modeler;
import java.io.File;
import java.io.FileInputStream;
+import java.io.IOException;
import java.io.InputStream;
import java.lang.management.ManagementFactory;
import java.net.URL;
@@ -590,6 +591,12 @@ public class Registry implements RegistryMBean,
MBeanRegistration {
/**
* Load descriptors.
*
+ * <p>
+ * If the source is a {@link URL} or a {@link File}, the stream opened to
read it is closed before this method
+ * returns. If the source is an {@link InputStream}, that stream is closed
before this method returns as well. In
+ * both cases the stream is closed whether or not loading succeeds, so the
caller must not use a stream passed to
+ * this method after the call.
+ *
* @param sourceType The source type
* @param source The bean
* @param param A type to load
@@ -637,12 +644,27 @@ public class Registry implements RegistryMBean,
MBeanRegistration {
throw new
IllegalArgumentException(sm.getString("registry.invalidSource"));
}
- if (sourceType == null) {
- sourceType = "MbeansDescriptorsDigesterSource";
+ try {
+ if (sourceType == null) {
+ sourceType = "MbeansDescriptorsDigesterSource";
+ }
+ ModelerSource ds = getModelerSource(sourceType);
+
+ return ds.loadDescriptors(this, type, inputsource);
+ } finally {
+ // The source does not keep the stream. Close it here, whichever
way loading ended, since this method opened
+ // it or was given it.
+ if (inputsource instanceof InputStream) {
+ try {
+ ((InputStream) inputsource).close();
+ } catch (IOException e) {
+ // Do not hide the outcome of the load
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("registry.closeSourceError"),
e);
+ }
+ }
+ }
}
- ModelerSource ds = getModelerSource(sourceType);
-
- return ds.loadDescriptors(this, type, inputsource);
}
diff --git
a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsDigesterSource.java
b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsDigesterSource.java
index 4799dbc6cb..21c827e6d9 100644
---
a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsDigesterSource.java
+++
b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsDigesterSource.java
@@ -153,8 +153,8 @@ public class MbeansDescriptorsDigesterSource extends
ModelerSource {
digester = createDigester();
}
- // Process the input file to configure our registry
- try (stream) {
+ // Process the input file to configure our registry. The stream is
closed by the caller (Registry.load()).
+ try {
// Push our registry object onto the stack
digester.push(loadedMbeans);
digester.parse(stream);
diff --git
a/test/org/apache/tomcat/util/modeler/modules/TestMbeansDescriptorsDigesterSource.java
b/test/org/apache/tomcat/util/modeler/modules/TestMbeansDescriptorsDigesterSource.java
index ca7a0e56d5..f230da83c2 100644
---
a/test/org/apache/tomcat/util/modeler/modules/TestMbeansDescriptorsDigesterSource.java
+++
b/test/org/apache/tomcat/util/modeler/modules/TestMbeansDescriptorsDigesterSource.java
@@ -17,10 +17,15 @@
package org.apache.tomcat.util.modeler.modules;
import java.io.ByteArrayInputStream;
+import java.io.File;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -81,6 +86,87 @@ public class TestMbeansDescriptorsDigesterSource {
Assert.assertTrue(closed.get());
}
+ /*
+ * Registry.load() closes the stream whatever the outcome, including when
the modeler source can't be created or
+ * rejects the source.
+ */
+ @Test
+ public void testLoadClosesStreamWhenSourceTypeIsUnknown() throws Exception
{
+ AtomicBoolean closed = new AtomicBoolean();
+ Registry registry = Registry.getRegistry(null);
+
+ Assert.assertThrows(ClassNotFoundException.class,
+ () -> registry.load("NoSuchModelerSource",
createTrackingStream(XML, closed), null));
+
+ Assert.assertTrue(closed.get());
+ }
+
+ /*
+ * The stream that Registry.load() opens for a URL is closed too.
+ */
+ @Test
+ public void testLoadClosesStreamOpenedForUrl() throws Exception {
+ AtomicBoolean closed = new AtomicBoolean();
+ URL url = new URL(null, "memory:/mbeans-descriptors.xml", new
URLStreamHandler() {
+ @Override
+ protected URLConnection openConnection(URL resource) {
+ return new URLConnection(resource) {
+ @Override
+ public void connect() {
+ // NO-OP
+ }
+
+ @Override
+ public InputStream getInputStream() {
+ return createTrackingStream(XML, closed);
+ }
+ };
+ }
+ });
+ Registry registry = Registry.getRegistry(null);
+
+ registry.load(null, url, null);
+
+ Assert.assertTrue(closed.get());
+
Assert.assertNotNull(registry.findManagedBean("TestDigesterSourceBean"));
+ }
+
+ @Test
+ public void testLoadClosesStreamOpenedForFile() throws Exception {
+ File file = File.createTempFile("tomcat-test-", ".xml");
+ try {
+ Files.writeString(file.toPath(), XML);
+ Registry registry = Registry.getRegistry(null);
+
+ registry.load(null, file, null);
+
+
Assert.assertNotNull(registry.findManagedBean("TestDigesterSourceBean"));
+ } finally {
+ // Can't be deleted on some platforms if the stream is still open
+ Assert.assertTrue(file.delete());
+ }
+ }
+
+ /*
+ * A failure to close the stream must not hide the outcome of the load.
+ */
+ @Test
+ public void testFailureToCloseStreamDoesNotHideLoadFailure() throws
Exception {
+ AtomicBoolean closed = new AtomicBoolean();
+ InputStream stream = new FilterInputStream(new
ByteArrayInputStream(XML.getBytes(StandardCharsets.UTF_8))) {
+ @Override
+ public void close() throws IOException {
+ closed.set(true);
+ throw new IOException("Close failed");
+ }
+ };
+ Registry registry = Registry.getRegistry(null);
+
+ Assert.assertThrows(ClassNotFoundException.class, () ->
registry.load("NoSuchModelerSource", stream, null));
+
+ Assert.assertTrue(closed.get());
+ }
+
@Test
public void testExecuteRequiresInputStreamSource() throws Exception {
Registry registry = Registry.getRegistry(null);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
