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,92 @@ 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.
+     */
+    @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");
+
+            assertTrue(disconnected.get(), "disconnect() must be called on the error path");
+
+            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;
@@ -356,30 +357,40 @@ public String toString() {
             value = "PATH_TRAVERSAL_IN",
             justification = "The name of the accessed files is based on a configuration value.")
     private static /*@Nullable*/ ConfigurationSource getConfigurationSource(final URL url) {
+        final File file;
+        final URLConnection urlConnection;
         try {
-            final File file = FileUtils.fileFromUri(url.toURI());
-            final URLConnection urlConnection = UrlConnectionFactory.createConnection(url);
-            try {
-                if (file != null) {
-                    return new ConfigurationSource(urlConnection.getInputStream(), FileUtils.fileFromUri(url.toURI()));
-                } 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);
-                } else {
-                    return new ConfigurationSource(
-                            urlConnection.getInputStream(), url, urlConnection.getLastModified());
-                }
-            } catch (FileNotFoundException ex) {
-                ConfigurationFactory.LOGGER.info("Unable to locate file {}, ignoring.", url.toString());
-                return null;
-            }
-        } catch (IOException | URISyntaxException ex) {
+            file = FileUtils.fileFromUri(url.toURI());
+            urlConnection = UrlConnectionFactory.createConnection(url);
+        } catch (final IOException | URISyntaxException ex) {
             ConfigurationFactory.LOGGER.warn(
                     "Error accessing {} due to {}, ignoring.", url.toString(), ex.getMessage());
             return null;
         }
+        try {
+            if (file != null) {
+                return new ConfigurationSource(urlConnection.getInputStream(), file);
+            } else if (urlConnection instanceof JarURLConnection) {
+                // Work around https://bugs.openjdk.java.net/browse/JDK-6956385.
+                final URL jarFileUrl = ((JarURLConnection) urlConnection).getJarFileURL();
+                final File jarFile = new File(jarFileUrl.getFile());
+                final long lastModified = jarFile.lastModified();
+                return new ConfigurationSource(urlConnection.getInputStream(), url, lastModified);
+            } else {
+                final long lastModified = urlConnection.getLastModified();
+                return new ConfigurationSource(urlConnection.getInputStream(), url, lastModified);
+            }
+        } catch (final IOException ex) {
+            if (ex instanceof FileNotFoundException) {
+                ConfigurationFactory.LOGGER.info("Unable to locate file {}, ignoring.", url.toString());
+            } else {
+                ConfigurationFactory.LOGGER.warn(
+                        "Error accessing {} due to {}, ignoring.", url.toString(), ex.getMessage());
+            }
+            if (urlConnection instanceof HttpURLConnection) {
+                ((HttpURLConnection) urlConnection).disconnect();
+            }
+            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>