You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2024/09/10 17:46:21 UTC
(logging-log4j2) 01/01: Fix reloading of the configuration from HTTP(S)
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch fix/2.x/2937-http-watcher
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit e2c33cfad328d14a16ad1fda6b8d981c0a67ada1
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Tue Sep 10 19:46:11 2024 +0200
Fix reloading of the configuration from HTTP(S)
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
---
.../logging/log4j/core/util/HttpWatcherTest.java | 161 +++++++++++++++++++
.../logging/log4j/core/util/WatchHttpTest.java | 156 ------------------
.../logging/log4j/core/util/WatchManagerTest.java | 177 +++++++++++----------
.../apache/logging/log4j/core/LoggerContext.java | 19 +--
.../log4j/core/config/AbstractConfiguration.java | 17 +-
.../log4j/core/config/ConfigurationSource.java | 63 ++++----
.../logging/log4j/core/config/HttpWatcher.java | 32 ++--
.../log4j/core/config/xml/XmlConfiguration.java | 12 +-
.../logging/log4j/core/util/WatchManager.java | 23 ++-
.../core/util/internal/HttpInputStreamUtil.java | 80 +++++++---
src/changelog/.2.x.x/2937-http-watcher.xml | 8 +
11 files changed, 411 insertions(+), 337 deletions(-)
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/HttpWatcherTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/HttpWatcherTest.java
new file mode 100644
index 0000000000..20b80ac731
--- /dev/null
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/HttpWatcherTest.java
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.util;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+import com.github.tomakehurst.wiremock.stubbing.StubMapping;
+import java.net.URL;
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+import org.apache.logging.log4j.core.config.AbstractConfiguration;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationListener;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.HttpWatcher;
+import org.apache.logging.log4j.core.config.Reconfigurable;
+import org.apache.logging.log4j.core.net.UrlConnectionFactory;
+import org.apache.logging.log4j.test.junit.SetTestProperty;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test the WatchManager
+ */
+@SetTestProperty(key = UrlConnectionFactory.ALLOWED_PROTOCOLS, value = "http,https")
+@WireMockTest
+class HttpWatcherTest {
+
+ private static final DateTimeFormatter formatter = DateTimeFormatter.RFC_1123_DATE_TIME.withZone(ZoneOffset.UTC);
+ private static final String XML = "application/xml";
+
+ @Test
+ void testModified(final WireMockRuntimeInfo info) throws Exception {
+ final WireMock wireMock = info.getWireMock();
+
+ final BlockingQueue<String> queue = new LinkedBlockingQueue<>();
+ List<ConfigurationListener> listeners = singletonList(new TestConfigurationListener(queue, "log4j-test1.xml"));
+ // HTTP Last-Modified is in seconds
+ Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
+ Instant previous = now.minus(5, ChronoUnit.MINUTES);
+ final URL url = new URL(info.getHttpBaseUrl() + "/log4j-test1.xml");
+ final Configuration configuration = createConfiguration(url);
+
+ final StubMapping stubMapping = wireMock.register(get(urlPathEqualTo("/log4j-test1.xml"))
+ .willReturn(aResponse()
+ .withBodyFile("log4j-test1.xml")
+ .withStatus(200)
+ .withHeader("Last-Modified", formatter.format(previous))
+ .withHeader("Content-Type", XML)));
+ Watcher watcher = new HttpWatcher(configuration, null, listeners, previous.toEpochMilli());
+ watcher.watching(new Source(url));
+ try {
+ assertThat(watcher.isModified()).as("File was modified").isTrue();
+ assertThat(watcher.getLastModified()).as("File modification time").isEqualTo(previous.toEpochMilli());
+ // Check if listeners are correctly called
+ // Note: listeners are called asynchronously
+ watcher.modified();
+ String str = queue.poll(1, TimeUnit.SECONDS);
+ assertThat(str).isEqualTo("log4j-test1.xml");
+ ConfigurationSource configurationSource = configuration.getConfigurationSource();
+ // Check that the last modified time of the ConfigurationSource was modified as well
+ // See: https://github.com/apache/logging-log4j2/issues/2937
+ assertThat(configurationSource.getLastModified())
+ .as("Last modification time of current ConfigurationSource")
+ .isEqualTo(0L);
+ configurationSource = configurationSource.resetInputStream();
+ assertThat(configurationSource.getLastModified())
+ .as("Last modification time of next ConfigurationSource")
+ .isEqualTo(previous.toEpochMilli());
+ } finally {
+ wireMock.removeStubMapping(stubMapping);
+ }
+ }
+
+ @Test
+ void testNotModified(final WireMockRuntimeInfo info) throws Exception {
+ final WireMock wireMock = info.getWireMock();
+
+ final BlockingQueue<String> queue = new LinkedBlockingQueue<>();
+ final List<ConfigurationListener> listeners =
+ singletonList(new TestConfigurationListener(queue, "log4j-test2.xml"));
+ // HTTP Last-Modified is in seconds
+ Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
+ Instant previous = now.minus(5, ChronoUnit.MINUTES);
+ final URL url = new URL(info.getHttpBaseUrl() + "/log4j-test2.xml");
+ final Configuration configuration = createConfiguration(url);
+
+ final StubMapping stubMapping = wireMock.register(get(urlPathEqualTo("/log4j-test2.xml"))
+ .willReturn(aResponse()
+ .withStatus(304)
+ .withHeader("Last-Modified", formatter.format(now) + " GMT")
+ .withHeader("Content-Type", XML)));
+ Watcher watcher = new HttpWatcher(configuration, null, listeners, previous.toEpochMilli());
+ watcher.watching(new Source(url));
+ try {
+ assertThat(watcher.isModified()).as("File was modified").isFalse();
+ // If the file was not modified, neither should be the last modification time
+ assertThat(watcher.getLastModified()).isEqualTo(previous.toEpochMilli());
+ // Check that the last modified time of the ConfigurationSource was not modified either
+ ConfigurationSource configurationSource = configuration.getConfigurationSource();
+ assertThat(configurationSource.getLastModified())
+ .as("Last modification time of current ConfigurationSource")
+ .isEqualTo(0L);
+ configurationSource = configurationSource.resetInputStream();
+ assertThat(configurationSource.getLastModified())
+ .as("Last modification time of next ConfigurationSource")
+ .isEqualTo(0L);
+ } finally {
+ wireMock.removeStubMapping(stubMapping);
+ }
+ }
+
+ // Creates a configuration with a predefined configuration source
+ private static Configuration createConfiguration(URL url) {
+ ConfigurationSource configurationSource = new ConfigurationSource(new Source(url), new byte[0], 0L);
+ return new AbstractConfiguration(null, configurationSource) {};
+ }
+
+ private static class TestConfigurationListener implements ConfigurationListener {
+ private final Queue<String> queue;
+ private final String name;
+
+ public TestConfigurationListener(final Queue<String> queue, final String name) {
+ this.queue = queue;
+ this.name = name;
+ }
+
+ @Override
+ public void onChange(final Reconfigurable reconfigurable) {
+ // System.out.println("Reconfiguration detected for " + name);
+ queue.add(name);
+ }
+ }
+}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java
deleted file mode 100644
index c433f83486..0000000000
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java
+++ /dev/null
@@ -1,156 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to you under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.logging.log4j.core.util;
-
-import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
-import static com.github.tomakehurst.wiremock.client.WireMock.get;
-import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
-
-import com.github.tomakehurst.wiremock.client.WireMock;
-import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
-import com.github.tomakehurst.wiremock.junit5.WireMockTest;
-import com.github.tomakehurst.wiremock.stubbing.StubMapping;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Calendar;
-import java.util.List;
-import java.util.Queue;
-import java.util.TimeZone;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.TimeUnit;
-import org.apache.logging.log4j.core.config.Configuration;
-import org.apache.logging.log4j.core.config.ConfigurationListener;
-import org.apache.logging.log4j.core.config.ConfigurationScheduler;
-import org.apache.logging.log4j.core.config.DefaultConfiguration;
-import org.apache.logging.log4j.core.config.HttpWatcher;
-import org.apache.logging.log4j.core.config.Reconfigurable;
-import org.apache.logging.log4j.core.net.UrlConnectionFactory;
-import org.apache.logging.log4j.core.util.datetime.FastDateFormat;
-import org.apache.logging.log4j.test.junit.SetTestProperty;
-import org.apache.logging.log4j.util.PropertiesUtil;
-import org.junit.jupiter.api.Test;
-
-/**
- * Test the WatchManager
- */
-@SetTestProperty(key = UrlConnectionFactory.ALLOWED_PROTOCOLS, value = "http,https")
-@WireMockTest
-public class WatchHttpTest {
-
- private static final String FORCE_RUN_KEY = WatchHttpTest.class.getSimpleName() + ".forceRun";
- private final String file = "log4j-test1.xml";
- private static final TimeZone UTC = TimeZone.getTimeZone("UTC");
- private static FastDateFormat formatter = FastDateFormat.getInstance("EEE, dd MMM yyyy HH:mm:ss", UTC);
- private static final String XML = "application/xml";
-
- private static final boolean IS_WINDOWS = PropertiesUtil.getProperties().isOsWindows();
-
- @Test
- public void testWatchManager(final WireMockRuntimeInfo info) throws Exception {
- assumeTrue(!IS_WINDOWS || Boolean.getBoolean(FORCE_RUN_KEY));
- final WireMock wireMock = info.getWireMock();
-
- final BlockingQueue<String> queue = new LinkedBlockingQueue<>();
- final List<ConfigurationListener> listeners = new ArrayList<>();
- listeners.add(new TestConfigurationListener(queue, "log4j-test1.xml"));
- final Calendar now = Calendar.getInstance(UTC);
- final Calendar previous = now;
- previous.add(Calendar.MINUTE, -5);
- final Configuration configuration = new DefaultConfiguration();
- final URL url = new URL(info.getHttpBaseUrl() + "/log4j-test1.xml");
- final StubMapping stubMapping = wireMock.register(get(urlPathEqualTo("/log4j-test1.xml"))
- .willReturn(aResponse()
- .withBodyFile(file)
- .withStatus(200)
- .withHeader("Last-Modified", formatter.format(previous) + " GMT")
- .withHeader("Content-Type", XML)));
- final ConfigurationScheduler scheduler = new ConfigurationScheduler();
- scheduler.incrementScheduledItems();
- final WatchManager watchManager = new WatchManager(scheduler);
- watchManager.setIntervalSeconds(1);
- scheduler.start();
- watchManager.start();
- try {
- watchManager.watch(
- new Source(url), new HttpWatcher(configuration, null, listeners, previous.getTimeInMillis()));
- final String str = queue.poll(3, TimeUnit.SECONDS);
- assertNotNull("File change not detected", str);
- } finally {
- watchManager.stop();
- scheduler.stop();
- wireMock.removeStubMapping(stubMapping);
- }
- }
-
- @Test
- public void testNotModified(final WireMockRuntimeInfo info) throws Exception {
- assumeTrue(!IS_WINDOWS || Boolean.getBoolean(FORCE_RUN_KEY));
- final WireMock wireMock = info.getWireMock();
-
- final BlockingQueue<String> queue = new LinkedBlockingQueue<>();
- final List<ConfigurationListener> listeners = new ArrayList<>();
- listeners.add(new TestConfigurationListener(queue, "log4j-test2.xml"));
- final TimeZone timeZone = TimeZone.getTimeZone("UTC");
- final Calendar now = Calendar.getInstance(timeZone);
- final Calendar previous = now;
- previous.add(Calendar.MINUTE, -5);
- final Configuration configuration = new DefaultConfiguration();
- final URL url = new URL(info.getHttpBaseUrl() + "/log4j-test2.xml");
- final StubMapping stubMapping = wireMock.register(get(urlPathEqualTo("/log4j-test2.xml"))
- .willReturn(aResponse()
- .withBodyFile(file)
- .withStatus(304)
- .withHeader("Last-Modified", formatter.format(now) + " GMT")
- .withHeader("Content-Type", XML)));
- final ConfigurationScheduler scheduler = new ConfigurationScheduler();
- scheduler.incrementScheduledItems();
- final WatchManager watchManager = new WatchManager(scheduler);
- watchManager.setIntervalSeconds(1);
- scheduler.start();
- watchManager.start();
- try {
- watchManager.watch(
- new Source(url), new HttpWatcher(configuration, null, listeners, previous.getTimeInMillis()));
- final String str = queue.poll(2, TimeUnit.SECONDS);
- assertNull("File changed.", str);
- } finally {
- watchManager.stop();
- scheduler.stop();
- wireMock.removeStubMapping(stubMapping);
- }
- }
-
- private static class TestConfigurationListener implements ConfigurationListener {
- private final Queue<String> queue;
- private final String name;
-
- public TestConfigurationListener(final Queue<String> queue, final String name) {
- this.queue = queue;
- this.name = name;
- }
-
- @Override
- public void onChange(final Reconfigurable reconfigurable) {
- // System.out.println("Reconfiguration detected for " + name);
- queue.add(name);
- }
- }
-}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java
index 68b3baaca6..49269d7134 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java
@@ -18,6 +18,12 @@ package org.apache.logging.log4j.core.util;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import java.io.File;
import java.io.FileOutputStream;
@@ -30,6 +36,9 @@ import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import org.apache.logging.log4j.core.config.ConfigurationScheduler;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
@@ -40,106 +49,110 @@ import org.junit.jupiter.api.condition.OS;
*/
@DisabledOnOs(OS.WINDOWS)
@EnabledIfSystemProperty(named = "WatchManagerTest.forceRun", matches = "true")
-public class WatchManagerTest {
+class WatchManagerTest {
private final String testFile = "target/testWatchFile";
private final String originalFile = "target/test-classes/log4j-test1.xml";
private final String newFile = "target/test-classes/log4j-test1.yaml";
- @Test
- public void testWatchManager() throws Exception {
- final ConfigurationScheduler scheduler = new ConfigurationScheduler();
+ private ConfigurationScheduler scheduler;
+ private WatchManager watchManager;
+
+ @BeforeEach
+ void setUp() {
+ scheduler = new ConfigurationScheduler();
scheduler.incrementScheduledItems();
- final WatchManager watchManager = new WatchManager(scheduler);
+ watchManager = new WatchManager(scheduler);
watchManager.setIntervalSeconds(1);
scheduler.start();
watchManager.start();
- try {
- final File sourceFile = new File(originalFile);
- Path source = Paths.get(sourceFile.toURI());
- try (final FileOutputStream targetStream = new FileOutputStream(testFile)) {
- Files.copy(source, targetStream);
- }
- final File updateFile = new File(newFile);
- final File targetFile = new File(testFile);
- final BlockingQueue<File> queue = new LinkedBlockingQueue<>();
- watchManager.watchFile(targetFile, new TestWatcher(queue));
- Thread.sleep(1000);
- source = Paths.get(updateFile.toURI());
- Files.copy(source, Paths.get(targetFile.toURI()), StandardCopyOption.REPLACE_EXISTING);
- Thread.sleep(1000);
- final File f = queue.poll(1, TimeUnit.SECONDS);
- assertNotNull(f, "File change not detected");
- } finally {
- watchManager.stop();
- scheduler.stop();
- }
+ }
+
+ @AfterEach
+ void tearDown() {
+ watchManager.stop();
+ scheduler.stop();
+ watchManager = null;
+ scheduler = null;
}
@Test
- public void testWatchManagerReset() throws Exception {
- final ConfigurationScheduler scheduler = new ConfigurationScheduler();
- scheduler.incrementScheduledItems();
- final WatchManager watchManager = new WatchManager(scheduler);
- watchManager.setIntervalSeconds(1);
- scheduler.start();
- watchManager.start();
- try {
- final File sourceFile = new File(originalFile);
- Path source = Paths.get(sourceFile.toURI());
- try (final FileOutputStream targetStream = new FileOutputStream(testFile)) {
- Files.copy(source, targetStream);
- }
- final File updateFile = new File(newFile);
- final File targetFile = new File(testFile);
- final BlockingQueue<File> queue = new LinkedBlockingQueue<>();
- watchManager.watchFile(targetFile, new TestWatcher(queue));
- watchManager.stop();
- Thread.sleep(1000);
- source = Paths.get(updateFile.toURI());
- Files.copy(source, Paths.get(targetFile.toURI()), StandardCopyOption.REPLACE_EXISTING);
- watchManager.reset();
- watchManager.start();
- Thread.sleep(1000);
- final File f = queue.poll(1, TimeUnit.SECONDS);
- assertNull(f, "File change detected");
- } finally {
- watchManager.stop();
- scheduler.stop();
+ void testWatchManager() throws Exception {
+ final File sourceFile = new File(originalFile);
+ Path source = Paths.get(sourceFile.toURI());
+ try (final FileOutputStream targetStream = new FileOutputStream(testFile)) {
+ Files.copy(source, targetStream);
}
+ final File updateFile = new File(newFile);
+ final File targetFile = new File(testFile);
+ final BlockingQueue<File> queue = new LinkedBlockingQueue<>();
+ watchManager.watchFile(targetFile, new TestWatcher(queue));
+ Thread.sleep(1000);
+ source = Paths.get(updateFile.toURI());
+ Files.copy(source, Paths.get(targetFile.toURI()), StandardCopyOption.REPLACE_EXISTING);
+ Thread.sleep(1000);
+ final File f = queue.poll(1, TimeUnit.SECONDS);
+ assertNotNull(f, "File change not detected");
}
@Test
- public void testWatchManagerResetFile() throws Exception {
- final ConfigurationScheduler scheduler = new ConfigurationScheduler();
- scheduler.incrementScheduledItems();
- final WatchManager watchManager = new WatchManager(scheduler);
- watchManager.setIntervalSeconds(1);
- scheduler.start();
+ void testWatchManagerReset() throws Exception {
+ final File sourceFile = new File(originalFile);
+ Path source = Paths.get(sourceFile.toURI());
+ try (final FileOutputStream targetStream = new FileOutputStream(testFile)) {
+ Files.copy(source, targetStream);
+ }
+ final File updateFile = new File(newFile);
+ final File targetFile = new File(testFile);
+ final BlockingQueue<File> queue = new LinkedBlockingQueue<>();
+ watchManager.watchFile(targetFile, new TestWatcher(queue));
+ watchManager.stop();
+ Thread.sleep(1000);
+ source = Paths.get(updateFile.toURI());
+ Files.copy(source, Paths.get(targetFile.toURI()), StandardCopyOption.REPLACE_EXISTING);
+ watchManager.reset();
watchManager.start();
- try {
- final File sourceFile = new File(originalFile);
- Path source = Paths.get(sourceFile.toURI());
- try (final FileOutputStream targetStream = new FileOutputStream(testFile)) {
- Files.copy(source, targetStream);
- }
- final File updateFile = new File(newFile);
- final File targetFile = new File(testFile);
- final BlockingQueue<File> queue = new LinkedBlockingQueue<>();
- watchManager.watchFile(targetFile, new TestWatcher(queue));
- watchManager.stop();
- Thread.sleep(1000);
- source = Paths.get(updateFile.toURI());
- Files.copy(source, Paths.get(targetFile.toURI()), StandardCopyOption.REPLACE_EXISTING);
- watchManager.reset(targetFile);
- watchManager.start();
- Thread.sleep(1000);
- final File f = queue.poll(1, TimeUnit.SECONDS);
- assertNull(f, "File change detected");
- } finally {
- watchManager.stop();
- scheduler.stop();
+ Thread.sleep(1000);
+ final File f = queue.poll(1, TimeUnit.SECONDS);
+ assertNull(f, "File change detected");
+ }
+
+ @Test
+ void testWatchManagerResetFile() throws Exception {
+ final File sourceFile = new File(originalFile);
+ Path source = Paths.get(sourceFile.toURI());
+ try (final FileOutputStream targetStream = new FileOutputStream(testFile)) {
+ Files.copy(source, targetStream);
}
+ final File updateFile = new File(newFile);
+ final File targetFile = new File(testFile);
+ final BlockingQueue<File> queue = new LinkedBlockingQueue<>();
+ watchManager.watchFile(targetFile, new TestWatcher(queue));
+ watchManager.stop();
+ Thread.sleep(1000);
+ source = Paths.get(updateFile.toURI());
+ Files.copy(source, Paths.get(targetFile.toURI()), StandardCopyOption.REPLACE_EXISTING);
+ watchManager.reset(targetFile);
+ watchManager.start();
+ Thread.sleep(1000);
+ final File f = queue.poll(1, TimeUnit.SECONDS);
+ assertNull(f, "File change detected");
+ }
+
+ /**
+ * Verify the
+ */
+ @Test
+ void testWatchManagerCallsWatcher() {
+ Watcher watcher = mock(Watcher.class);
+ when(watcher.isModified()).thenReturn(false);
+ watchManager.watch(new Source(ConfigurationSource.NULL_SOURCE), watcher);
+ verify(watcher, timeout(2000)).isModified();
+ verify(watcher, never()).modified();
+ when(watcher.isModified()).thenReturn(true);
+ clearInvocations(watcher);
+ verify(watcher, timeout(2000)).isModified();
+ verify(watcher).modified();
}
private static class TestWatcher implements FileWatcher {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
index 07cd2d1f59..ea9e13cf51 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
@@ -87,7 +87,7 @@ public class LoggerContext extends AbstractLifeCycle
private volatile Configuration configuration = new DefaultConfiguration();
private static final String EXTERNAL_CONTEXT_KEY = "__EXTERNAL_CONTEXT_KEY__";
- private ConcurrentMap<String, Object> externalMap = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, Object> externalMap = new ConcurrentHashMap<>();
private String contextName;
private volatile URI configLocation;
private Cancellable shutdownCallback;
@@ -122,9 +122,7 @@ public class LoggerContext extends AbstractLifeCycle
*/
public LoggerContext(final String name, final Object externalContext, final URI configLocn) {
this.contextName = name;
- if (externalContext == null) {
- externalMap.remove(EXTERNAL_CONTEXT_KEY);
- } else {
+ if (externalContext != null) {
externalMap.put(EXTERNAL_CONTEXT_KEY, externalContext);
}
this.configLocation = configLocn;
@@ -143,9 +141,7 @@ public class LoggerContext extends AbstractLifeCycle
justification = "The configLocn comes from a secure source (Log4j properties)")
public LoggerContext(final String name, final Object externalContext, final String configLocn) {
this.contextName = name;
- if (externalContext == null) {
- externalMap.remove(EXTERNAL_CONTEXT_KEY);
- } else {
+ if (externalContext != null) {
externalMap.put(EXTERNAL_CONTEXT_KEY, externalContext);
}
if (configLocn != null) {
@@ -166,7 +162,7 @@ public class LoggerContext extends AbstractLifeCycle
if (listeners == null) {
synchronized (this) {
if (listeners == null) {
- listeners = new CopyOnWriteArrayList<LoggerContextShutdownAware>();
+ listeners = new CopyOnWriteArrayList<>();
}
}
}
@@ -277,7 +273,7 @@ public class LoggerContext extends AbstractLifeCycle
* @param config The new Configuration.
*/
public void start(final Configuration config) {
- LOGGER.debug("Starting LoggerContext[name={}, {}] with configuration {}...", getName(), this, config);
+ LOGGER.info("Starting {}[name={}] with configuration {}...", getClass().getSimpleName(), getName(), config);
if (configLock.tryLock()) {
try {
if (this.isInitialized() || this.isStopped()) {
@@ -291,7 +287,7 @@ public class LoggerContext extends AbstractLifeCycle
}
}
setConfiguration(config);
- LOGGER.debug("LoggerContext[name={}, {}] started OK with configuration {}.", getName(), this, config);
+ LOGGER.info("{}[name={}] started with configuration {}.", getClass().getSimpleName(), getName(), config);
}
private void setUpShutdownHook() {
@@ -306,7 +302,6 @@ public class LoggerContext extends AbstractLifeCycle
this.shutdownCallback = ((ShutdownCallbackRegistry) factory).addShutdownCallback(new Runnable() {
@Override
public void run() {
- @SuppressWarnings("resource")
final LoggerContext context = LoggerContext.this;
LOGGER.debug(
SHUTDOWN_HOOK_MARKER,
@@ -708,7 +703,7 @@ public class LoggerContext extends AbstractLifeCycle
*/
private void reconfigure(final URI configURI) {
final Object externalContext = externalMap.get(EXTERNAL_CONTEXT_KEY);
- final ClassLoader cl = ClassLoader.class.isInstance(externalContext) ? (ClassLoader) externalContext : null;
+ final ClassLoader cl = externalContext instanceof ClassLoader ? (ClassLoader) externalContext : null;
LOGGER.debug(
"Reconfiguration started for context[name={}] at URI {} ({}) with optional ClassLoader: {}",
contextName,
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
index 3c5069f766..4b160f98ef 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
@@ -281,9 +281,9 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
watchManager.setIntervalSeconds(monitorIntervalSeconds);
if (configSource.getFile() != null) {
final Source cfgSource = new Source(configSource);
- final long lastModifeid = configSource.getFile().lastModified();
+ final long lastModified = configSource.getFile().lastModified();
final ConfigurationFileWatcher watcher =
- new ConfigurationFileWatcher(this, reconfigurable, listeners, lastModifeid);
+ new ConfigurationFileWatcher(this, reconfigurable, listeners, lastModified);
watchManager.watch(cfgSource, watcher);
} else if (configSource.getURL() != null) {
monitorSource(reconfigurable, configSource);
@@ -318,9 +318,13 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
if (getState() == State.INITIALIZING) {
initialize();
}
- LOGGER.debug("Starting configuration {}", this);
+ LOGGER.info("Starting configuration {}...", this);
this.setStarting();
if (watchManager.getIntervalSeconds() >= 0) {
+ LOGGER.info(
+ "Start watching for changes to {} every {} seconds",
+ getConfigurationSource(),
+ watchManager.getIntervalSeconds());
watchManager.start();
}
if (hasAsyncLoggers()) {
@@ -338,7 +342,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
root.start(); // LOG4J2-336
}
super.start();
- LOGGER.debug("Started configuration {} OK.", this);
+ LOGGER.info("Configuration {} started.", this);
}
private boolean hasAsyncLoggers() {
@@ -358,9 +362,9 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
*/
@Override
public boolean stop(final long timeout, final TimeUnit timeUnit) {
+ LOGGER.info("Stopping configuration {}...", this);
this.setStopping();
super.stop(timeout, timeUnit, false);
- LOGGER.trace("Stopping {}...", this);
// Stop the components that are closest to the application first:
// 1. Notify all LoggerConfigs' ReliabilityStrategy that the configuration will be stopped.
@@ -456,7 +460,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
advertiser.unadvertise(advertisement);
}
setStopped();
- LOGGER.debug("Stopped {} OK", this);
+ LOGGER.info("Configuration {} stopped.", this);
return true;
}
@@ -484,6 +488,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
// default does nothing, subclasses do work.
}
+ @SuppressWarnings("deprecation")
protected Level getDefaultStatus() {
final PropertiesUtil properties = PropertiesUtil.getProperties();
String statusLevel = properties.getStringProperty(StatusLogger.DEFAULT_STATUS_LISTENER_LEVEL);
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 fbcf278a07..f31f9b9fd2 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
@@ -33,10 +33,12 @@ import java.net.URLConnection;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Objects;
+import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.net.UrlConnectionFactory;
import org.apache.logging.log4j.core.util.FileUtils;
import org.apache.logging.log4j.core.util.Loader;
import org.apache.logging.log4j.core.util.Source;
+import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.Constants;
import org.apache.logging.log4j.util.LoaderUtil;
@@ -45,6 +47,8 @@ import org.apache.logging.log4j.util.LoaderUtil;
*/
public class ConfigurationSource {
+ private static final Logger LOGGER = StatusLogger.getLogger();
+
/**
* ConfigurationSource to use with Configurations that do not require a "real" configuration source.
*/
@@ -58,7 +62,7 @@ public class ConfigurationSource {
private final InputStream stream;
private volatile byte[] data;
- private volatile Source source;
+ private final Source source;
private final long lastModified;
// Set when the configuration has been updated so reset can use it for the next lastModified timestamp.
private volatile long modifiedMillis;
@@ -80,7 +84,7 @@ public class ConfigurationSource {
} catch (Exception ex) {
// There is a problem with the file. It will be handled somewhere else.
}
- this.lastModified = modified;
+ this.modifiedMillis = this.lastModified = modified;
}
/**
@@ -100,7 +104,7 @@ public class ConfigurationSource {
} catch (Exception ex) {
// There is a problem with the file. It will be handled somewhere else.
}
- this.lastModified = modified;
+ this.modifiedMillis = this.lastModified = modified;
}
/**
@@ -111,10 +115,7 @@ public class ConfigurationSource {
* @param url the URL where the input stream originated
*/
public ConfigurationSource(final InputStream stream, final URL url) {
- this.stream = Objects.requireNonNull(stream, "stream is null");
- this.data = null;
- this.lastModified = 0;
- this.source = new Source(url);
+ this(stream, url, 0);
}
/**
@@ -128,7 +129,7 @@ public class ConfigurationSource {
public ConfigurationSource(final InputStream stream, final URL url, final long lastModified) {
this.stream = Objects.requireNonNull(stream, "stream is null");
this.data = null;
- this.lastModified = lastModified;
+ this.modifiedMillis = this.lastModified = lastModified;
this.source = new Source(url);
}
@@ -154,16 +155,17 @@ public class ConfigurationSource {
Objects.requireNonNull(source, "source is null");
this.data = Objects.requireNonNull(data, "data is null");
this.stream = new ByteArrayInputStream(data);
- this.lastModified = lastModified;
+ this.modifiedMillis = this.lastModified = lastModified;
this.source = source;
}
private ConfigurationSource(final byte[] data, final URL url, final long lastModified) {
this.data = Objects.requireNonNull(data, "data is null");
this.stream = new ByteArrayInputStream(data);
- this.lastModified = lastModified;
+ this.modifiedMillis = this.lastModified = lastModified;
if (url == null) {
this.data = data;
+ this.source = null;
} else {
this.source = new Source(url);
}
@@ -199,16 +201,8 @@ public class ConfigurationSource {
return source == null ? null : source.getFile();
}
- private boolean isFile() {
- return source == null ? false : source.getFile() != null;
- }
-
- private boolean isURL() {
- return source == null ? false : source.getURI() != null;
- }
-
private boolean isLocation() {
- return source == null ? false : source.getLocation() != null;
+ return source != null && source.getLocation() != null;
}
/**
@@ -222,11 +216,11 @@ public class ConfigurationSource {
}
/**
- * @deprecated Not used internally, no replacement. TODO remove and make source final.
+ * @deprecated Not used internally, no replacement.
*/
@Deprecated
- public void setSource(final Source source) {
- this.source = source;
+ public void setSource(final Source ignored) {
+ LOGGER.warn("Ignoring call of deprecated method `ConfigurationSource#setSource()`.");
}
public void setData(final byte[] data) {
@@ -280,16 +274,23 @@ public class ConfigurationSource {
*/
public ConfigurationSource resetInputStream() throws IOException {
if (source != null && data != null) {
- return new ConfigurationSource(source, data, this.lastModified);
- } else if (isFile()) {
- return new ConfigurationSource(new FileInputStream(getFile()), getFile());
- } else if (isURL() && data != null) {
+ return new ConfigurationSource(source, data, modifiedMillis);
+ }
+ File file = getFile();
+ if (file != null) {
+ return new ConfigurationSource(Files.newInputStream(file.toPath()), getFile());
+ }
+ URL url = getURL();
+ if (url != null && data != null) {
// Creates a ConfigurationSource without accessing the URL since the data was provided.
- return new ConfigurationSource(data, getURL(), modifiedMillis == 0 ? lastModified : modifiedMillis);
- } else if (isURL()) {
- return fromUri(getURI());
- } else if (data != null) {
- return new ConfigurationSource(data, null, lastModified);
+ return new ConfigurationSource(data, url, modifiedMillis);
+ }
+ URI uri = getURI();
+ if (uri != null) {
+ return fromUri(uri);
+ }
+ if (data != null) {
+ return new ConfigurationSource(data, null, modifiedMillis);
}
return null;
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/HttpWatcher.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/HttpWatcher.java
index 08e840d93b..545409a4e4 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/HttpWatcher.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/HttpWatcher.java
@@ -16,16 +16,17 @@
*/
package org.apache.logging.log4j.core.config;
+import static org.apache.logging.log4j.util.Strings.toRootUpperCase;
+
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
+import java.time.Instant;
import java.util.List;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginAliases;
-import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
-import org.apache.logging.log4j.core.net.ssl.SslConfigurationFactory;
import org.apache.logging.log4j.core.util.AbstractWatcher;
import org.apache.logging.log4j.core.util.AuthorizationProvider;
import org.apache.logging.log4j.core.util.Source;
@@ -44,7 +45,6 @@ public class HttpWatcher extends AbstractWatcher {
private final Logger LOGGER = StatusLogger.getLogger();
- private final SslConfiguration sslConfiguration;
private AuthorizationProvider authorizationProvider;
private URL url;
private volatile long lastModifiedMillis;
@@ -57,7 +57,6 @@ public class HttpWatcher extends AbstractWatcher {
final List<ConfigurationListener> configurationListeners,
final long lastModifiedMillis) {
super(configuration, reconfigurable, configurationListeners);
- sslConfiguration = SslConfigurationFactory.getSslConfiguration();
this.lastModifiedMillis = lastModifiedMillis;
}
@@ -103,9 +102,11 @@ public class HttpWatcher extends AbstractWatcher {
try {
final LastModifiedSource source = new LastModifiedSource(url.toURI(), lastModifiedMillis);
final HttpInputStreamUtil.Result result = HttpInputStreamUtil.getInputStream(source, authorizationProvider);
+ // Update lastModifiedMillis
+ // https://github.com/apache/logging-log4j2/issues/2937
+ lastModifiedMillis = source.getLastModified();
switch (result.getStatus()) {
case NOT_MODIFIED: {
- LOGGER.debug("Configuration Not Modified");
return false;
}
case SUCCESS: {
@@ -113,24 +114,35 @@ public class HttpWatcher extends AbstractWatcher {
try {
configSource.setData(HttpInputStreamUtil.readStream(result.getInputStream()));
configSource.setModifiedMillis(source.getLastModified());
- LOGGER.debug("Content was modified for {}", url.toString());
+ LOGGER.info(
+ "{} resource at {} was modified on {}",
+ () -> toRootUpperCase(url.getProtocol()),
+ () -> url.toExternalForm(),
+ () -> Instant.ofEpochMilli(source.getLastModified()));
return true;
} catch (final IOException e) {
- LOGGER.error("Error accessing configuration at {}: {}", url, e.getMessage());
+ // Dead code since result.getInputStream() is a ByteArrayInputStream
+ LOGGER.error("Error accessing configuration at {}", url.toExternalForm(), e);
return false;
}
}
case NOT_FOUND: {
- LOGGER.info("Unable to locate configuration at {}", url.toString());
+ LOGGER.warn(
+ "{} resource at {} was not found",
+ () -> toRootUpperCase(url.getProtocol()),
+ () -> url.toExternalForm());
return false;
}
default: {
- LOGGER.warn("Unexpected error accessing configuration at {}", url.toString());
+ LOGGER.warn(
+ "Unexpected error retrieving {} resource at {}",
+ () -> toRootUpperCase(url.getProtocol()),
+ () -> url.toExternalForm());
return false;
}
}
} catch (final URISyntaxException ex) {
- LOGGER.error("Bad configuration URL: {}, {}", url.toString(), ex.getMessage());
+ LOGGER.error("Bad configuration file URL {}", url.toExternalForm(), ex);
return false;
}
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java
index 48502de22e..31e42d843f 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java
@@ -18,9 +18,9 @@ package org.apache.logging.log4j.core.config.xml;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.ByteArrayInputStream;
-import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -73,7 +73,6 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu
justification = "The `newDocumentBuilder` method disables DTD processing.")
public XmlConfiguration(final LoggerContext loggerContext, final ConfigurationSource configSource) {
super(loggerContext, configSource);
- final File configFile = configSource.getFile();
byte[] buffer = null;
try {
@@ -175,7 +174,7 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu
*
* @param xIncludeAware enabled XInclude
* @return a new DocumentBuilder
- * @throws ParserConfigurationException
+ * @throws ParserConfigurationException if a DocumentBuilder cannot be created, which satisfies the configuration requested.
*/
static DocumentBuilder newDocumentBuilder(final boolean xIncludeAware) throws ParserConfigurationException {
final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
@@ -241,7 +240,7 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu
return;
}
constructHierarchy(rootNode, rootElement);
- if (status.size() > 0) {
+ if (!status.isEmpty()) {
for (final Status s : status) {
LOGGER.error("Error processing element {} ({}): {}", s.name, s.element, s.errorType);
}
@@ -295,7 +294,7 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu
}
final String text = buffer.toString().trim();
- if (text.length() > 0 || (!node.hasChildren() && !node.isRoot())) {
+ if (!text.isEmpty() || (!node.hasChildren() && !node.isRoot())) {
node.setValue(text);
}
}
@@ -337,7 +336,8 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu
@Override
public String toString() {
- return getClass().getSimpleName() + "[location=" + getConfigurationSource() + "]";
+ return getClass().getSimpleName() + "[location=" + getConfigurationSource() + ", lastModified="
+ + Instant.ofEpochMilli(getConfigurationSource().getLastModified()) + "]";
}
/**
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java
index 15e9009b75..7165bfe6f1 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java
@@ -20,6 +20,7 @@ import aQute.bnd.annotation.Cardinality;
import aQute.bnd.annotation.Resolution;
import aQute.bnd.annotation.spi.ServiceConsumer;
import java.io.File;
+import java.time.Instant;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
@@ -49,8 +50,9 @@ import org.apache.logging.log4j.util.ServiceLoaderUtil;
@ServiceConsumer(value = WatchEventService.class, resolution = Resolution.OPTIONAL, cardinality = Cardinality.MULTIPLE)
public class WatchManager extends AbstractLifeCycle {
- private final class ConfigurationMonitor {
+ private static final class ConfigurationMonitor {
private final Watcher watcher;
+ // Only used for logging
private volatile long lastModifiedMillis;
public ConfigurationMonitor(final long lastModifiedMillis, final Watcher watcher) {
@@ -76,7 +78,6 @@ public class WatchManager extends AbstractLifeCycle {
private static final long LOW_MASK = 0xffffffffL;
private static final long MID_MASK = 0xffff00000000L;
private static final long HIGH_MASK = 0xfff000000000000L;
- private static final int NODE_SIZE = 8;
private static final int SHIFT_2 = 16;
private static final int SHIFT_4 = 32;
private static final int SHIFT_6 = 48;
@@ -84,8 +85,6 @@ public class WatchManager extends AbstractLifeCycle {
private static final long NUM_100NS_INTERVALS_SINCE_UUID_EPOCH = 0x01b21dd213814000L;
private static final AtomicInteger COUNT = new AtomicInteger(0);
private static final long TYPE1 = 0x1000L;
- private static final byte VARIANT = (byte) 0x80;
- private static final int SEQUENCE_MASK = 0x3FFF;
public static UUID get() {
final long time =
@@ -112,15 +111,11 @@ public class WatchManager extends AbstractLifeCycle {
final ConfigurationMonitor monitor = entry.getValue();
if (monitor.getWatcher().isModified()) {
final long lastModified = monitor.getWatcher().getLastModified();
- if (logger.isInfoEnabled()) {
- logger.info(
- "Source '{}' was modified on {} ({}), previous modification was on {} ({})",
- source,
- millisToString(lastModified),
- lastModified,
- millisToString(monitor.lastModifiedMillis),
- monitor.lastModifiedMillis);
- }
+ logger.info(
+ "Configuration source at {} was modified on {}, previous modification was on {}",
+ () -> source,
+ () -> Instant.ofEpochMilli(lastModified),
+ () -> Instant.ofEpochMilli(monitor.lastModifiedMillis));
monitor.lastModifiedMillis = lastModified;
monitor.getWatcher().modified();
}
@@ -200,7 +195,7 @@ public class WatchManager extends AbstractLifeCycle {
}
public boolean hasEventListeners() {
- return eventServiceList.size() > 0;
+ return !eventServiceList.isEmpty();
}
private String millisToString(final long millis) {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/HttpInputStreamUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/HttpInputStreamUtil.java
index c146958f9f..6e4f8187b6 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/HttpInputStreamUtil.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/HttpInputStreamUtil.java
@@ -16,17 +16,22 @@
*/
package org.apache.logging.log4j.core.util.internal;
+import static org.apache.logging.log4j.util.Strings.toRootUpperCase;
+
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
+import java.time.Instant;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.config.ConfigurationException;
import org.apache.logging.log4j.core.net.UrlConnectionFactory;
import org.apache.logging.log4j.core.net.ssl.SslConfigurationFactory;
import org.apache.logging.log4j.core.util.AuthorizationProvider;
+import org.apache.logging.log4j.core.util.Source;
import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.Supplier;
/**
* Utility method for reading data from an HTTP InputStream.
@@ -36,10 +41,21 @@ public final class HttpInputStreamUtil {
private static final Logger LOGGER = StatusLogger.getLogger();
private static final int NOT_MODIFIED = 304;
private static final int NOT_AUTHORIZED = 401;
+ private static final int FORBIDDEN = 403;
private static final int NOT_FOUND = 404;
private static final int OK = 200;
private static final int BUF_SIZE = 1024;
+ /**
+ * Retrieves an HTTP resource if it has been modified.
+ * <p>
+ * Side effects: if the request is successful, the last modified time of the {@code source}
+ * parameter is modified.
+ * </p>
+ * @param source The location of the HTTP resource
+ * @param authorizationProvider The authentication data for the HTTP request
+ * @return A {@link Result} object containing the status code and body of the response
+ */
public static Result getInputStream(
final LastModifiedSource source, final AuthorizationProvider authorizationProvider) {
final Result result = new Result();
@@ -55,12 +71,16 @@ public final class HttpInputStreamUtil {
final int code = connection.getResponseCode();
switch (code) {
case NOT_MODIFIED: {
- LOGGER.debug("Configuration not modified");
+ LOGGER.debug(
+ "{} resource {}: not modified since {}",
+ formatProtocol(source),
+ () -> source,
+ () -> Instant.ofEpochMilli(lastModified));
result.status = Status.NOT_MODIFIED;
return result;
}
case NOT_FOUND: {
- LOGGER.debug("Unable to access {}: Not Found", source.toString());
+ LOGGER.debug("{} resource {}: not found", formatProtocol(source), () -> source);
result.status = Status.NOT_FOUND;
return result;
}
@@ -68,45 +88,65 @@ public final class HttpInputStreamUtil {
try (final InputStream is = connection.getInputStream()) {
source.setLastModified(connection.getLastModified());
LOGGER.debug(
- "Content was modified for {}. previous lastModified: {}, new lastModified: {}",
- source.toString(),
- lastModified,
- connection.getLastModified());
+ "{} resource {}: last modified on {}",
+ formatProtocol(source),
+ () -> source,
+ () -> Instant.ofEpochMilli(connection.getLastModified()));
result.status = Status.SUCCESS;
- result.inputStream = new ByteArrayInputStream(readStream(is));
+ result.bytes = readStream(is);
return result;
} catch (final IOException e) {
try (final InputStream es = connection.getErrorStream()) {
- LOGGER.info(
- "Error accessing configuration at {}: {}", source.toString(), readStream(es));
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug(
+ "Error accessing {} resource at {}: {}",
+ formatProtocol(source).get(),
+ source,
+ readStream(es),
+ e);
+ }
} catch (final IOException ioe) {
- LOGGER.error(
- "Error accessing configuration at {}: {}", source.toString(), e.getMessage());
+ LOGGER.debug(
+ "Error accessing {} resource at {}",
+ formatProtocol(source),
+ () -> source,
+ () -> e);
}
- throw new ConfigurationException("Unable to access " + source.toString(), e);
+ throw new ConfigurationException("Unable to access " + source, e);
}
}
case NOT_AUTHORIZED: {
- throw new ConfigurationException("Authorization failed");
+ throw new ConfigurationException("Authentication required for " + source);
+ }
+ case FORBIDDEN: {
+ throw new ConfigurationException("Access denied to " + source);
}
default: {
if (code < 0) {
- LOGGER.info("Invalid response code returned");
+ LOGGER.debug("{} resource {}: invalid response code", formatProtocol(source), source);
} else {
- LOGGER.info("Unexpected response code returned {}", code);
+ LOGGER.debug(
+ "{} resource {}: unexpected response code {}",
+ formatProtocol(source),
+ source,
+ code);
}
- throw new ConfigurationException("Unable to access " + source.toString());
+ throw new ConfigurationException("Unable to access " + source);
}
}
} finally {
connection.disconnect();
}
} catch (IOException e) {
- LOGGER.warn("Error accessing {}: {}", source.toString(), e.getMessage());
- throw new ConfigurationException("Unable to access " + source.toString(), e);
+ LOGGER.debug("Error accessing {} resource at {}", formatProtocol(source), source, e);
+ throw new ConfigurationException("Unable to access " + source, e);
}
}
+ private static Supplier<String> formatProtocol(Source source) {
+ return () -> toRootUpperCase(source.getURI().getScheme());
+ }
+
public static byte[] readStream(final InputStream is) throws IOException {
final ByteArrayOutputStream result = new ByteArrayOutputStream();
final byte[] buffer = new byte[BUF_SIZE];
@@ -119,7 +159,7 @@ public final class HttpInputStreamUtil {
public static class Result {
- private InputStream inputStream;
+ private byte[] bytes;
private Status status;
public Result() {}
@@ -129,7 +169,7 @@ public final class HttpInputStreamUtil {
}
public InputStream getInputStream() {
- return inputStream;
+ return new ByteArrayInputStream(bytes);
}
public Status getStatus() {
diff --git a/src/changelog/.2.x.x/2937-http-watcher.xml b/src/changelog/.2.x.x/2937-http-watcher.xml
new file mode 100644
index 0000000000..ec3e1a1426
--- /dev/null
+++ b/src/changelog/.2.x.x/2937-http-watcher.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="2937" link="https://github.com/apache/logging-log4j2/issues/2937"/>
+ <description format="asciidoc">Fix reloading of the configuration from an HTTP(S) source</description>
+</entry>