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..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,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. + * + *

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) { 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..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 @@ -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; 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 +