Fix resource leaks in ConfigurationSource when loading via URL

log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSourceTest.java

@@ -25,14 +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;
@@ -104,6 +112,108 @@ 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 testJarFileLeakOnFailure() throws Exception {
+        final Path jarFile = prepareJarConfigURL(tempDir);
+        final String jarUriStr = "jar:" + jarFile.toUri().toASCIIString() + "!/" + java.util.UUID.randomUUID();
+        final URI jarUri = URI.create(jarUriStr);
+
+        final long expectedFdCount = getOpenFileDescriptorCount();
+
+        assertNull(ConfigurationSource.fromUri(jarUri));
+
+        assertEquals(expectedFdCount, getOpenFileDescriptorCount());
+        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.
+     *
+     * <p>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) {

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,44 @@ public String toString() {
         try {
             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) {
-                    return new ConfigurationSource(urlConnection.getInputStream(), FileUtils.fileFromUri(url.toURI()));
+                    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();
-                    return new ConfigurationSource(urlConnection.getInputStream(), url, lastModified);
+                    openedStream = urlConnection.getInputStream();
+                    source = new ConfigurationSource(openedStream, url, lastModified);
                 } else {
-                    return new ConfigurationSource(
-                            urlConnection.getInputStream(), url, urlConnection.getLastModified());
+                    openedStream = urlConnection.getInputStream();
+                    source = new ConfigurationSource(openedStream, 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) {
+                    if (openedStream != null) {
+                        try {
+                            openedStream.close();
+                        } catch (final IOException ignored) {
+                            // Best-effort cleanup
+                        }
+                    }
+                    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;

src/changelog/.2.x.x/fix_configuration_source_url_leak.xml

@@ -0,0 +1,8 @@
+<?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="4127" link="https://github.com/apache/logging-log4j2/pull/4127"/>
+  <description format="asciidoc">Fix resource leaks in `ConfigurationSource` when loading configuration via URL fails</description>
+</entry>