diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/jmx/LoggerContextAdminSetConfigLocationUriTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/jmx/LoggerContextAdminSetConfigLocationUriTest.java new file mode 100644 index 00000000000..7cf9456ce03 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/jmx/LoggerContextAdminSetConfigLocationUriTest.java @@ -0,0 +1,81 @@ +/* + * 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.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.logging.log4j.core.LoggerContext; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Regression for stream ownership in {@link LoggerContextAdmin#setConfigLocationUri(String)}. + * The method buffers configuration bytes and closes the underlying File/URL stream before + * ConfigurationFactory runs. + */ +class LoggerContextAdminSetConfigLocationUriTest { + + @TempDir + Path tempDir; + + @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)); + } + + private static void writeConfig(final Path config, final String appenderName) throws Exception { + final String xml = "\n" + + "\n" + + " \n" + + " \n" + + "\n"; + Files.write(config, xml.getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java index d8927d6737d..3152f99e8cc 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java @@ -19,6 +19,7 @@ import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -45,6 +46,7 @@ import org.apache.logging.log4j.core.config.ConfigurationSource; import org.apache.logging.log4j.core.internal.annotation.SuppressFBWarnings; import org.apache.logging.log4j.core.util.Closer; +import org.apache.logging.log4j.core.util.Source; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Strings; @@ -128,14 +130,26 @@ 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; + final ConfigurationSource configSource; + // Read the configuration into memory and close the underlying stream before handing + // off to ConfigurationFactory. ConfigurationSource(InputStream, File/URL) leaves stream + // ownership with the caller; without buffering, failures during getConfiguration/start + // can leave FileInputStream/URLConnection streams open. if (configFile.exists()) { LOGGER.debug("Opening config file {}", configFile.getAbsolutePath()); - configSource = new ConfigurationSource(new FileInputStream(configFile), configFile); + final byte[] data; + try (final InputStream in = new FileInputStream(configFile)) { + data = toByteArray(in); + } + configSource = new ConfigurationSource(new Source(configFile), data, configFile.lastModified()); } else { final URL configURL = new URL(configLocation); LOGGER.debug("Opening config URL {}", configURL); - configSource = new ConfigurationSource(configURL.openStream(), configURL); + final byte[] data; + try (final InputStream in = configURL.openStream()) { + data = toByteArray(in); + } + configSource = new ConfigurationSource(new Source(configURL), data, 0L); } final Configuration config = ConfigurationFactory.getInstance().getConfiguration(loggerContext, configSource); loggerContext.start(config); @@ -253,4 +267,14 @@ private long nextSeqNo() { private long now() { return System.currentTimeMillis(); } + + private static byte[] toByteArray(final InputStream inputStream) throws IOException { + final ByteArrayOutputStream contents = new ByteArrayOutputStream(Math.max(4096, inputStream.available())); + final byte[] buff = new byte[4096]; + int length; + while ((length = inputStream.read(buff)) >= 0) { + contents.write(buff, 0, length); + } + return contents.toByteArray(); + } } diff --git a/src/changelog/.2.x.x/fix_logger_context_admin_config_stream_leak.xml b/src/changelog/.2.x.x/fix_logger_context_admin_config_stream_leak.xml new file mode 100644 index 00000000000..32065280d59 --- /dev/null +++ b/src/changelog/.2.x.x/fix_logger_context_admin_config_stream_leak.xml @@ -0,0 +1,8 @@ + + + + Close configuration streams in JMX `LoggerContextAdmin.setConfigLocationUri` after buffering into `ConfigurationSource` +