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..9df40fcc4e6 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,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) { 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..d2fdf0adf1a 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; @@ -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; + } } } 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 +