From 75388ffffa7e77db65d38c83040257167d56c885 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 21 May 2026 07:35:36 -0700 Subject: [PATCH 1/4] Close URLConnection on error paths in ConfigurationSource In getConfigurationSource(URL), the URLConnection opened via UrlConnectionFactory.createConnection() was never closed or disconnected when an error occurred (FileNotFoundException, IOException, URISyntaxException). For HttpURLConnection this leaks a socket; for JarURLConnection this leaks a file handle. Fix: use a success flag with try-finally to ensure the connection is cleaned up on all error paths while leaving it open on the success path where the stream is handed off to ConfigurationSource. Also replace the redundant FileUtils.fileFromUri(url.toURI()) call on the return line with the existing 'file' variable. --- .../core/config/ConfigurationSource.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java index dd50987f257..ad7bc307f06 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java @@ -23,6 +23,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.net.HttpURLConnection; import java.net.JarURLConnection; import java.net.MalformedURLException; import java.net.URI; @@ -359,24 +360,39 @@ public String toString() { try { final File file = FileUtils.fileFromUri(url.toURI()); final URLConnection urlConnection = UrlConnectionFactory.createConnection(url); + boolean success = false; try { + final ConfigurationSource source; if (file != null) { - return new ConfigurationSource(urlConnection.getInputStream(), FileUtils.fileFromUri(url.toURI())); + source = new ConfigurationSource(urlConnection.getInputStream(), file); } else if (urlConnection instanceof JarURLConnection) { // Work around https://bugs.openjdk.java.net/browse/JDK-6956385. URL jarFileUrl = ((JarURLConnection) urlConnection).getJarFileURL(); File jarFile = new File(jarFileUrl.getFile()); long lastModified = jarFile.lastModified(); - return new ConfigurationSource(urlConnection.getInputStream(), url, lastModified); + source = new ConfigurationSource(urlConnection.getInputStream(), url, lastModified); } else { - return new ConfigurationSource( + source = new ConfigurationSource( urlConnection.getInputStream(), url, urlConnection.getLastModified()); } - } catch (FileNotFoundException ex) { + success = true; + return source; + } catch (final FileNotFoundException ex) { ConfigurationFactory.LOGGER.info("Unable to locate file {}, ignoring.", url.toString()); return null; + } finally { + if (!success) { + try { + urlConnection.getInputStream().close(); + } catch (final IOException ignored) { + // Best-effort cleanup; the stream may not have been opened + } + if (urlConnection instanceof HttpURLConnection) { + ((HttpURLConnection) urlConnection).disconnect(); + } + } } - } catch (IOException | URISyntaxException ex) { + } catch (final IOException | URISyntaxException ex) { ConfigurationFactory.LOGGER.warn( "Error accessing {} due to {}, ignoring.", url.toString(), ex.getMessage()); return null; From 3fde88bf083a7248c42978d9f6cb9bb373c6a86c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 21 May 2026 13:31:36 -0700 Subject: [PATCH 2/4] Add failure test and changelog entry Add testNoJarFileLeakOnFailure to verify that a failed JAR configuration load (missing entry) does not leak file descriptors or hold a lock on the JAR file. Add changelog entry for the ConfigurationSource URL leak fix. --- .../core/config/ConfigurationSourceTest.java | 22 +++++++++++++++++++ .../fix_configuration_source_url_leak.xml | 8 +++++++ 2 files changed, 30 insertions(+) create mode 100644 src/changelog/.2.x.x/fix_configuration_source_url_leak.xml diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java index 3015f4f6271..313fefb138f 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java @@ -104,6 +104,28 @@ void testLoadConfigurationSourceFromJarFile() throws Exception { } } + /** + * Verifies that a failed JAR configuration load (missing entry) does not leak file descriptors + * or hold a lock on the JAR file. + */ + @Test + void testNoJarFileLeakOnFailure() throws Exception { + final Path jarFile = prepareJarConfigURL(tempDir); + // Path inside JAR that does NOT exist + final URL brokenJarConfigURL = new URL("jar:" + jarFile.toUri().toURL() + "!/missing/file.xml"); + final long expectedFdCount = getOpenFileDescriptorCount(); + final ConfigurationSource configSource = ConfigurationSource.fromUri(brokenJarConfigURL.toURI()); + assertNull(configSource, "Source should be null for a missing JAR entry"); + // This can only fail on UNIX + assertEquals(expectedFdCount, getOpenFileDescriptorCount(), "File descriptors leaked after failed JAR load"); + // This can only fail on Windows + try { + Files.delete(jarFile); + } catch (IOException e) { + fail("JAR file locked after failed load: " + e.getMessage()); + } + } + private long getOpenFileDescriptorCount() { final OperatingSystemMXBean os = ManagementFactory.getOperatingSystemMXBean(); if (os instanceof UnixOperatingSystemMXBean) { diff --git a/src/changelog/.2.x.x/fix_configuration_source_url_leak.xml b/src/changelog/.2.x.x/fix_configuration_source_url_leak.xml new file mode 100644 index 00000000000..e7c57d5a1a1 --- /dev/null +++ b/src/changelog/.2.x.x/fix_configuration_source_url_leak.xml @@ -0,0 +1,8 @@ + + + + Fix resource leaks in `ConfigurationSource` when loading configuration via URL fails + From bbed9f27fe97d26ac3867b3b5d0bb069ad48ae55 Mon Sep 17 00:00:00 2001 From: Ramanathan Date: Fri, 22 May 2026 03:18:54 +0530 Subject: [PATCH 3/4] Fix potential URLConnection leak by ensuring InputStream is closed on error paths --- .../core/config/ConfigurationSourceTest.java | 23 ++++++++----------- .../core/config/ConfigurationSource.java | 21 ++++++++++------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java index 313fefb138f..b9300bb7af3 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java @@ -30,6 +30,7 @@ import java.io.OutputStream; import java.lang.management.ManagementFactory; import java.lang.management.OperatingSystemMXBean; +import java.net.URI; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -109,21 +110,17 @@ void testLoadConfigurationSourceFromJarFile() throws Exception { * or hold a lock on the JAR file. */ @Test - void testNoJarFileLeakOnFailure() throws Exception { + void testJarFileLeakOnFailure() throws Exception { final Path jarFile = prepareJarConfigURL(tempDir); - // Path inside JAR that does NOT exist - final URL brokenJarConfigURL = new URL("jar:" + jarFile.toUri().toURL() + "!/missing/file.xml"); + final String jarUriStr = "jar:" + jarFile.toUri().toASCIIString() + "!/" + java.util.UUID.randomUUID(); + final URI jarUri = URI.create(jarUriStr); + final long expectedFdCount = getOpenFileDescriptorCount(); - final ConfigurationSource configSource = ConfigurationSource.fromUri(brokenJarConfigURL.toURI()); - assertNull(configSource, "Source should be null for a missing JAR entry"); - // This can only fail on UNIX - assertEquals(expectedFdCount, getOpenFileDescriptorCount(), "File descriptors leaked after failed JAR load"); - // This can only fail on Windows - try { - Files.delete(jarFile); - } catch (IOException e) { - fail("JAR file locked after failed load: " + e.getMessage()); - } + + assertNull(ConfigurationSource.fromUri(jarUri)); + + assertEquals(expectedFdCount, getOpenFileDescriptorCount()); + Files.delete(jarFile); } private long getOpenFileDescriptorCount() { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java index ad7bc307f06..f8e61daa9a1 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java @@ -361,19 +361,22 @@ public String toString() { final File file = FileUtils.fileFromUri(url.toURI()); final URLConnection urlConnection = UrlConnectionFactory.createConnection(url); boolean success = false; + InputStream openedStream = null; try { final ConfigurationSource source; if (file != null) { - source = new ConfigurationSource(urlConnection.getInputStream(), file); + openedStream = urlConnection.getInputStream(); + source = new ConfigurationSource(openedStream, file); } else if (urlConnection instanceof JarURLConnection) { // Work around https://bugs.openjdk.java.net/browse/JDK-6956385. URL jarFileUrl = ((JarURLConnection) urlConnection).getJarFileURL(); File jarFile = new File(jarFileUrl.getFile()); long lastModified = jarFile.lastModified(); - source = new ConfigurationSource(urlConnection.getInputStream(), url, lastModified); + openedStream = urlConnection.getInputStream(); + source = new ConfigurationSource(openedStream, url, lastModified); } else { - source = new ConfigurationSource( - urlConnection.getInputStream(), url, urlConnection.getLastModified()); + openedStream = urlConnection.getInputStream(); + source = new ConfigurationSource(openedStream, url, urlConnection.getLastModified()); } success = true; return source; @@ -382,10 +385,12 @@ public String toString() { return null; } finally { if (!success) { - try { - urlConnection.getInputStream().close(); - } catch (final IOException ignored) { - // Best-effort cleanup; the stream may not have been opened + if (openedStream != null) { + try { + openedStream.close(); + } catch (final IOException ignored) { + // Best-effort cleanup + } } if (urlConnection instanceof HttpURLConnection) { ((HttpURLConnection) urlConnection).disconnect(); From 4bfa56c1fede9b03a0fe58acfcf3f0afeb9ce1fb Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 28 May 2026 06:18:09 -0700 Subject: [PATCH 4/4] Add negative test for HttpURLConnection error-path cleanup Verifies that getConfigurationSource() properly disconnects the HttpURLConnection on failure (e.g., 404) and does not call getInputStream() a second time in the finally block. Uses a custom URLStreamHandler to inject a counting HttpURLConnection that throws FileNotFoundException, then asserts: - disconnect() is called (fails against original code with no finally) - getInputStream() is called exactly once (fails against an intermediate fix that re-invoked getInputStream() in the finally block) --- .../core/config/ConfigurationSourceTest.java | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java index b9300bb7af3..1d34031e4a3 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java @@ -25,15 +25,22 @@ import com.sun.management.UnixOperatingSystemMXBean; import java.io.ByteArrayInputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.lang.management.ManagementFactory; import java.lang.management.OperatingSystemMXBean; +import java.lang.reflect.Method; +import java.net.HttpURLConnection; import java.net.URI; import java.net.URL; +import java.net.URLConnection; +import java.net.URLStreamHandler; import java.nio.file.Files; import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.jar.Attributes; import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; @@ -123,6 +130,90 @@ void testJarFileLeakOnFailure() throws Exception { Files.delete(jarFile); } + /** + * Verifies that the error-path cleanup in {@code getConfigurationSource} + * properly disconnects an {@link HttpURLConnection} on failure without + * calling {@link URLConnection#getInputStream()} a second time. + * + *

This test fails against the original code (no cleanup) because + * {@code disconnect()} is never called, and fails against an intermediate + * fix that added a {@code finally} block calling {@code getInputStream()} + * again to obtain a stream for closing. + */ + @Test + void getConfigurationSource_disconnectsHttpConnection_onError() throws Exception { + final AtomicInteger getInputStreamCalls = new AtomicInteger(); + final AtomicBoolean disconnected = new AtomicBoolean(); + final AtomicInteger openConnectionCalls = new AtomicInteger(); + + final URLStreamHandler handler = new URLStreamHandler() { + @Override + protected URLConnection openConnection(final URL u) { + openConnectionCalls.incrementAndGet(); + return new HttpURLConnection(u) { + @Override + public InputStream getInputStream() throws IOException { + getInputStreamCalls.incrementAndGet(); + throw new FileNotFoundException("Mocked 404"); + } + + @Override + public void disconnect() { + disconnected.set(true); + } + + @Override + public boolean usingProxy() { + return false; + } + + @Override + public void connect() {} + }; + } + }; + + final URL url = new URL("http", "example.com", 80, "/missing.xml", handler); + + // Allow the "http" protocol for this test. + final String propKey = "log4j2.Configuration.allowedProtocols"; + final String previous = System.getProperty(propKey); + System.setProperty(propKey, "file, https, jar, http"); + try { + openConnectionCalls.set(0); + getInputStreamCalls.set(0); + + final Method method = + ConfigurationSource.class.getDeclaredMethod("getConfigurationSource", URL.class); + method.setAccessible(true); + final Object result = method.invoke(null, url); + + assertTrue( + openConnectionCalls.get() > 0, + "Custom URLStreamHandler was not used by UrlConnectionFactory"); + + assertNull(result, "Expected null return for a 404 response"); + + // Negative‐test #1: the original code has no finally block, so + // disconnect() is never called on the HttpURLConnection. + assertTrue(disconnected.get(), "disconnect() must be called on the error path"); + + // Negative‐test #2: an intermediate fix called getInputStream() + // a second time in the finally block instead of closing the + // already-tracked stream reference. + assertEquals( + 1, + getInputStreamCalls.get(), + "getInputStream() should be called exactly once"); + } finally { + if (previous != null) { + System.setProperty(propKey, previous); + } else { + System.clearProperty(propKey); + } + } + } + private long getOpenFileDescriptorCount() { final OperatingSystemMXBean os = ManagementFactory.getOperatingSystemMXBean(); if (os instanceof UnixOperatingSystemMXBean) {