Skip to content

Commit

Permalink
Fix reloading of the configuration from HTTP(S)
Browse files Browse the repository at this point in the history
The `HttpWatcher` didn't propagate the observed
last modification time back to the configuration.
As a result, each new configuration was already
deprecated when it started and the reconfiguration
process looped.

Closes #2937

Rewrite Jetty tests using WireMock

Closes #2813

Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
  • Loading branch information
ppkarwasz and vy committed Oct 2, 2024
1 parent 981ae1e commit c791f0c
Show file tree
Hide file tree
Showing 30 changed files with 1,075 additions and 828 deletions.
46 changes: 34 additions & 12 deletions log4j-core-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,34 @@
java.allocation.instrumenter;substitute="java-allocation-instrumenter",
spring.test;substitute="spring-test"
</bnd-extra-module-options>

<!-- Dependency versions -->
<slf4j2.version>2.0.16</slf4j2.version>
<wiremock.version>3.9.1</wiremock.version>
</properties>

<dependencyManagement>
<dependencies>

<!-- Transitive dependency of `wiremock` -->
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>${slf4j2.version}</version>
</dependency>

<!-- Pinned transitive dependencies with CVEs -->

<!-- Transitive dependency of `log4j2-custom-layout` -->
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.11.0</version>
</dependency>

</dependencies>
</dependencyManagement>

<dependencies>

<dependency>
Expand Down Expand Up @@ -241,13 +267,6 @@
<scope>test</scope>
</dependency>

<!-- Log4j 1.2 tests -->
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.github.ivandzf</groupId>
<artifactId>log4j2-custom-layout</artifactId>
Expand Down Expand Up @@ -288,11 +307,11 @@
<scope>test</scope>
</dependency>

<!-- SLF4J tests -->
<!-- Switch to `slf4j-simple` to enable HTTP logging -->
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<scope>test</scope>
<artifactId>slf4j-nop</artifactId>
<version>${slf4j2.version}</version>
</dependency>

<dependency>
Expand All @@ -303,8 +322,9 @@

<!-- Used for testing HTTP Watcher -->
<dependency>
<groupId>com.github.tomakehurst</groupId>
<artifactId>wiremock-jre8</artifactId>
<groupId>org.wiremock</groupId>
<artifactId>wiremock</artifactId>
<version>${wiremock.version}</version>
<scope>test</scope>
</dependency>

Expand Down Expand Up @@ -345,6 +365,8 @@
<runOrder>random</runOrder>
<systemPropertyVariables>
<Web.isWebApp>false</Web.isWebApp>
<!-- Enables logging of HTTP requests, if `slf4j-nop` is replaced by `slf4j-simple` above-->
<org.slf4j.simpleLogger.log.org.eclipse.jetty.server.HttpChannel>DEBUG</org.slf4j.simpleLogger.log.org.eclipse.jetty.server.HttpChannel>
</systemPropertyVariables>
<useModulePath>false</useModulePath>
</configuration>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.config;

import static java.util.Objects.requireNonNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand All @@ -32,50 +33,57 @@
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
import java.util.jar.Manifest;
import org.apache.commons.io.IOUtils;
import org.apache.logging.log4j.core.net.UrlConnectionFactory;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

public class ConfigurationSourceTest {
private static final Path JAR_FILE = Paths.get("target", "test-classes", "jarfile.jar");
private static final Path CONFIG_FILE = Paths.get("target", "test-classes", "log4j2-console.xml");
private static final byte[] buffer = new byte[1024];
/**
* The path inside the jar created by {@link #prepareJarConfigURL} containing the configuration.
*/
public static final String PATH_IN_JAR = "/config/console.xml";

private static final String CONFIG_FILE = "/config/ConfigurationSourceTest.xml";

@TempDir
private Path tempDir;

@Test
public void testJira_LOG4J2_2770_byteArray() throws Exception {
void testJira_LOG4J2_2770_byteArray() throws Exception {
final ConfigurationSource configurationSource =
new ConfigurationSource(new ByteArrayInputStream(new byte[] {'a', 'b'}));
assertNotNull(configurationSource.resetInputStream());
}

/**
* Checks if the usage of 'jar:' URLs does not increase the file descriptor
* count and the jar file can be deleted.
*
* @throws Exception
* count, and the jar file can be deleted.
*/
@Test
public void testNoJarFileLeak() throws Exception {
final URL jarConfigURL = prepareJarConfigURL();
void testNoJarFileLeak() throws Exception {
final Path jarFile = prepareJarConfigURL(tempDir);
final URL jarConfigURL = new URL("jar:" + jarFile.toUri().toURL() + "!" + PATH_IN_JAR);
final long expected = getOpenFileDescriptorCount();
UrlConnectionFactory.createConnection(jarConfigURL).getInputStream().close();
// This can only fail on UNIX
assertEquals(expected, getOpenFileDescriptorCount());
// This can only fail on Windows
try {
Files.delete(JAR_FILE);
Files.delete(jarFile);
} catch (IOException e) {
fail(e);
}
}

@Test
public void testLoadConfigurationSourceFromJarFile() throws Exception {
final URL jarConfigURL = prepareJarConfigURL();
final Path jarFile = prepareJarConfigURL(tempDir);
final URL jarConfigURL = new URL("jar:" + jarFile.toUri().toURL() + "!" + PATH_IN_JAR);
final long expectedFdCount = getOpenFileDescriptorCount();
ConfigurationSource configSource = ConfigurationSource.fromUri(jarConfigURL.toURI());
assertNotNull(configSource);
Expand All @@ -90,7 +98,7 @@ public void testLoadConfigurationSourceFromJarFile() throws Exception {
assertEquals(expectedFdCount, getOpenFileDescriptorCount());
// This can only fail on Windows
try {
Files.delete(JAR_FILE);
Files.delete(jarFile);
} catch (IOException e) {
fail(e);
}
Expand All @@ -104,22 +112,18 @@ private long getOpenFileDescriptorCount() {
return 0L;
}

public static URL prepareJarConfigURL() throws IOException {
if (!Files.exists(JAR_FILE)) {
final Manifest manifest = new Manifest();
manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
try (final OutputStream os = Files.newOutputStream(JAR_FILE);
final JarOutputStream jar = new JarOutputStream(os, manifest);
final InputStream config = Files.newInputStream(CONFIG_FILE)) {
final JarEntry jarEntry = new JarEntry("config/console.xml");
jar.putNextEntry(jarEntry);
int len;
while ((len = config.read(buffer)) != -1) {
jar.write(buffer, 0, len);
}
jar.closeEntry();
}
public static Path prepareJarConfigURL(Path dir) throws IOException {
Path jarFile = dir.resolve("jarFile.jar");
final Manifest manifest = new Manifest();
manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
try (final OutputStream os = Files.newOutputStream(jarFile);
final JarOutputStream jar = new JarOutputStream(os, manifest);
final InputStream config =
requireNonNull(ConfigurationSourceTest.class.getResourceAsStream(CONFIG_FILE))) {
final JarEntry jarEntry = new JarEntry("config/console.xml");
jar.putNextEntry(jarEntry);
IOUtils.copy(config, os);
}
return new URL("jar:" + JAR_FILE.toUri().toURL() + "!/config/console.xml");
return jarFile;
}
}

This file was deleted.

Loading

0 comments on commit c791f0c

Please sign in to comment.