fix: close streams in LoggerContextAdmin.setConfigLocationUri

log4j-core-test/src/test/java/org/apache/logging/log4j/core/jmx/LoggerContextAdminSetConfigLocationUriTest.java

@@ -0,0 +1,134 @@
+/*
+ * 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.logging.log4j.core.jmx;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationFactory;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.DefaultConfiguration;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Regression for caller-owned stream cleanup in
+ * {@link LoggerContextAdmin#setConfigLocationUri(String)}.
+ *
+ * <p>{@code ConfigurationSource(InputStream, File/URL)} leaves stream ownership
+ * with the caller. Built-in factories close {@code getInputStream()} when they
+ * consume it, but that is not a substitute for caller-side try-with-resources
+ * when a factory path never reads the stream.
+ */
+class LoggerContextAdminSetConfigLocationUriTest {
+
+    @TempDir
+    Path tempDir;
+
+    @AfterEach
+    void resetConfigurationFactory() {
+        ConfigurationFactory.resetConfigurationFactory();
+    }
+
+    @Test
+    void setConfigLocationUri_loadsValidFileAndReconfigures() throws Exception {
+        final Path config = tempDir.resolve("log4j2-test.xml");
+        writeConfig(config, "C");
+
+        final LoggerContext ctx = new LoggerContext("jmx-admin-stream-test");
+        final LoggerContextAdmin admin = new LoggerContextAdmin(ctx, Runnable::run);
+
+        assertDoesNotThrow(
+                () -> admin.setConfigLocationUri(config.toAbsolutePath().toString()));
+        assertTrue(ctx.getConfiguration().getAppenders().containsKey("C"));
+    }
+
+    @Test
+    void setConfigLocationUri_fileUrlFormAlsoLoads() throws Exception {
+        final Path config = tempDir.resolve("log4j2-url.xml");
+        writeConfig(config, "FromUrl");
+
+        final LoggerContext ctx = new LoggerContext("jmx-admin-file-url-test");
+        final LoggerContextAdmin admin = new LoggerContextAdmin(ctx, Runnable::run);
+        final String fileUrl = config.toUri().toURL().toString();
+        assertDoesNotThrow(() -> admin.setConfigLocationUri(fileUrl));
+        assertTrue(ctx.getConfiguration().getAppenders().containsKey("FromUrl"));
+    }
+
+    @Test
+    void setConfigLocationUri_rejectsBlankLocation() {
+        final LoggerContext ctx = new LoggerContext("jmx-admin-blank");
+        final LoggerContextAdmin admin = new LoggerContextAdmin(ctx, Runnable::run);
+        assertThrows(IllegalArgumentException.class, () -> admin.setConfigLocationUri(""));
+        assertThrows(IllegalArgumentException.class, () -> admin.setConfigLocationUri(null));
+    }
+
+    /**
+     * Portable red-green for caller try-with-resources: install a factory that
+     * returns without consuming the source stream, capture that stream, and
+     * assert it is closed after {@code setConfigLocationUri} returns.
+     */
+    @Test
+    void setConfigLocationUri_closesCallerOwnedStreamWhenFactoryDoesNotConsumeIt() throws Exception {
+        final Path config = tempDir.resolve("log4j2-unconsumed.xml");
+        writeConfig(config, "Unconsumed");
+
+        final AtomicReference<InputStream> input = new AtomicReference<>();
+        ConfigurationFactory.setConfigurationFactory(new ConfigurationFactory() {
+            @Override
+            public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) {
+                // Capture the caller-owned stream without consuming or closing it.
+                input.set(source.getInputStream());
+                return new DefaultConfiguration();
+            }
+
+            @Override
+            protected String[] getSupportedTypes() {
+                return new String[] {"*"};
+            }
+        });
+
+        final LoggerContext ctx = new LoggerContext("jmx-admin-stream-close");
+        final LoggerContextAdmin admin = new LoggerContextAdmin(ctx, Runnable::run);
+        admin.setConfigLocationUri(config.toAbsolutePath().toString());
+
+        final InputStream captured = input.get();
+        assertNotNull(captured);
+        // Closed streams throw on read; an unclosed stream would still read.
+        assertThrows(IOException.class, captured::read);
+    }
+
+    private static void writeConfig(final Path config, final String appenderName) throws Exception {
+        final String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<Configuration status=\"OFF\">\n"
+                + "  <Appenders><Console name=\"" + appenderName + "\" target=\"SYSTEM_OUT\"/></Appenders>\n"
+                + "  <Loggers><Root level=\"error\"><AppenderRef ref=\"" + appenderName + "\"/></Root></Loggers>\n"
+                + "</Configuration>\n";
+        Files.write(config, xml.getBytes(StandardCharsets.UTF_8));
+    }
+}

log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java

@@ -20,7 +20,6 @@
 import java.beans.PropertyChangeListener;
 import java.io.ByteArrayInputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -31,6 +30,7 @@
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.Executor;
@@ -128,17 +128,29 @@ public void setConfigLocationUri(final String configLocation) throws URISyntaxEx
         LOGGER.debug("---------");
         LOGGER.debug("Remote request to reconfigure using location " + configLocation);
         final File configFile = new File(configLocation);
-        ConfigurationSource configSource = null;
+        // ConfigurationSource(InputStream, File/URL) documents that the caller owns the stream.
+        // XmlConfiguration, JsonConfiguration, and PropertiesConfigurationFactory close
+        // getInputStream() when they consume it, but that is factory-side cleanup. Keep a
+        // stream-backed source (so resetInputStream/reconfigure still re-reads the file for
+        // monitorInterval) and always close the caller-owned stream via try-with-resources.
         if (configFile.exists()) {
             LOGGER.debug("Opening config file {}", configFile.getAbsolutePath());
-            configSource = new ConfigurationSource(new FileInputStream(configFile), configFile);
+            try (final InputStream in = Files.newInputStream(configFile.toPath())) {
+                final ConfigurationSource configSource = new ConfigurationSource(in, configFile);
+                final Configuration config =
+                        ConfigurationFactory.getInstance().getConfiguration(loggerContext, configSource);
+                loggerContext.start(config);
+            }
         } else {
             final URL configURL = new URL(configLocation);
             LOGGER.debug("Opening config URL {}", configURL);
-            configSource = new ConfigurationSource(configURL.openStream(), configURL);
+            try (final InputStream in = configURL.openStream()) {
+                final ConfigurationSource configSource = new ConfigurationSource(in, configURL);
+                final Configuration config =
+                        ConfigurationFactory.getInstance().getConfiguration(loggerContext, configSource);
+                loggerContext.start(config);
+            }
         }
-        final Configuration config = ConfigurationFactory.getInstance().getConfiguration(loggerContext, configSource);
-        loggerContext.start(config);
         LOGGER.debug("Completed remote request to reconfigure.");
     }
 

src/changelog/.2.x.x/fix_logger_context_admin_config_stream_leak.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+  <issue id="4218" link="https://github.com/apache/logging-log4j2/pull/4218"/>
+  <description format="asciidoc">
+    Fix file descriptor leak in the JMX integration
+  </description>
+</entry>