Fix StatusLogger time-zone issues and stack overflow

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerBufferCapacityTest.java

@@ -0,0 +1,60 @@
+/*
+ * 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.status;
+
+import static org.apache.logging.log4j.status.StatusLogger.DEFAULT_FALLBACK_LISTENER_BUFFER_CAPACITY;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+import org.junit.jupiter.api.Test;
+import uk.org.webcompere.systemstubs.SystemStubs;
+
+class StatusLoggerBufferCapacityTest {
+
+    @Test
+    void valid_buffer_capacity_should_be_effective() {
+
+        // Create a `StatusLogger` configuration
+        final Properties statusLoggerConfigProperties = new Properties();
+        final int bufferCapacity = 10;
+        assertThat(bufferCapacity).isNotEqualTo(DEFAULT_FALLBACK_LISTENER_BUFFER_CAPACITY);
+        statusLoggerConfigProperties.put(StatusLogger.MAX_STATUS_ENTRIES, "" + bufferCapacity);
+        final StatusLogger.Config statusLoggerConfig = new StatusLogger.Config(statusLoggerConfigProperties);
+
+        // Verify the buffer capacity
+        assertThat(statusLoggerConfig.bufferCapacity).isEqualTo(bufferCapacity);
+    }
+
+    @Test
+    void invalid_buffer_capacity_should_cause_fallback_to_defaults() throws Exception {
+
+        // Create a `StatusLogger` configuration using an invalid buffer capacity
+        final Properties statusLoggerConfigProperties = new Properties();
+        final int invalidBufferCapacity = -10;
+        statusLoggerConfigProperties.put(StatusLogger.MAX_STATUS_ENTRIES, "" + invalidBufferCapacity);
+        final StatusLogger.Config[] statusLoggerConfigRef = {null};
+        final String stderr = SystemStubs.tapSystemErr(
+                () -> statusLoggerConfigRef[0] = new StatusLogger.Config(statusLoggerConfigProperties));
+        final StatusLogger.Config statusLoggerConfig = statusLoggerConfigRef[0];
+
+        // Verify the stderr dump
+        assertThat(stderr).contains("Failed reading the buffer capacity");
+
+        // Verify the buffer capacity
+        assertThat(statusLoggerConfig.bufferCapacity).isEqualTo(DEFAULT_FALLBACK_LISTENER_BUFFER_CAPACITY);
+    }
+}

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerDateTest.java

@@ -0,0 +1,105 @@
+/*
+ * 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.status;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import edu.umd.cs.findbugs.annotations.Nullable;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
+import java.util.Properties;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import uk.org.webcompere.systemstubs.SystemStubs;
+
+class StatusLoggerDateTest {
+
+    @ParameterizedTest
+    @CsvSource({"yyyy-MM-dd", "HH:mm:ss", "HH:mm:ss.SSS"})
+    void common_date_patterns_should_work(final String instantPattern) {
+
+        // Create a `StatusLogger` configuration
+        final Properties statusLoggerConfigProperties = new Properties();
+        statusLoggerConfigProperties.put(StatusLogger.STATUS_DATE_FORMAT, instantPattern);
+        final ZoneId zoneId = ZoneId.of("UTC");
+        statusLoggerConfigProperties.put(StatusLogger.STATUS_DATE_FORMAT_ZONE, zoneId.toString());
+        final StatusLogger.Config statusLoggerConfig = new StatusLogger.Config(statusLoggerConfigProperties);
+
+        // Verify the formatter
+        final DateTimeFormatter formatter =
+                DateTimeFormatter.ofPattern(instantPattern).withZone(zoneId);
+        verifyFormatter(statusLoggerConfig.instantFormatter, formatter);
+    }
+
+    @Test
+    void invalid_date_format_should_cause_fallback_to_defaults() throws Exception {
+        final String invalidFormat = "l";
+        verifyInvalidDateFormatAndZone(invalidFormat, "UTC", "failed reading the instant format", null);
+    }
+
+    @Test
+    void invalid_date_format_zone_should_cause_fallback_to_defaults() throws Exception {
+        final String invalidZone = "XXX";
+        final String format = "yyyy";
+        verifyInvalidDateFormatAndZone(
+                format,
+                invalidZone,
+                "Failed reading the instant formatting zone ID",
+                DateTimeFormatter.ofPattern(format).withZone(ZoneId.systemDefault()));
+    }
+
+    private static void verifyInvalidDateFormatAndZone(
+            final String format,
+            final String zone,
+            final String stderrMessage,
+            @Nullable final DateTimeFormatter formatter)
+            throws Exception {
+
+        // Create a `StatusLogger` configuration using invalid input
+        final Properties statusLoggerConfigProperties = new Properties();
+        statusLoggerConfigProperties.put(StatusLogger.STATUS_DATE_FORMAT, format);
+        statusLoggerConfigProperties.put(StatusLogger.STATUS_DATE_FORMAT_ZONE, zone);
+        final StatusLogger.Config[] statusLoggerConfigRef = {null};
+        final String stderr = SystemStubs.tapSystemErr(
+                () -> statusLoggerConfigRef[0] = new StatusLogger.Config(statusLoggerConfigProperties));
+        final StatusLogger.Config statusLoggerConfig = statusLoggerConfigRef[0];
+
+        // Verify the stderr dump
+        assertThat(stderr).contains(stderrMessage);
+
+        // Verify the formatter
+        verifyFormatter(statusLoggerConfig.instantFormatter, formatter);
+    }
+
+    /**
+     * {@link DateTimeFormatter} doesn't have an {@link Object#equals(Object)} implementation, hence <a href="https://stackoverflow.com/a/63887712/1278899">this manual <em>behavioral</em> comparison</a>.
+     *
+     * @param actual the actual formatter
+     * @param expected the expected formatter
+     */
+    private static void verifyFormatter(@Nullable DateTimeFormatter actual, @Nullable DateTimeFormatter expected) {
+        if (expected == null) {
+            assertThat(actual).isNull();
+        } else {
+            assertThat(actual).isNotNull();
+            final Instant instant = Instant.now();
+            assertThat(actual.format(instant)).isEqualTo(expected.format(instant));
+        }
+    }
+}

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerFailingListenerTest.java

@@ -0,0 +1,66 @@
+/*
+ * 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.status;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.logging.log4j.Level;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.parallel.ResourceLock;
+import uk.org.webcompere.systemstubs.SystemStubs;
+import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension;
+
+@ExtendWith(SystemStubsExtension.class)
+@ResourceLock("log4j2.StatusLogger")
+class StatusLoggerFailingListenerTest {
+
+    public static final StatusLogger STATUS_LOGGER = StatusLogger.getLogger();
+
+    private StatusListener listener;
+
+    @BeforeEach
+    void createAndRegisterListener() {
+        listener = mock(StatusListener.class);
+        STATUS_LOGGER.registerListener(listener);
+    }
+
+    @AfterEach
+    void unregisterListener() {
+        STATUS_LOGGER.removeListener(listener);
+    }
+
+    @Test
+    void logging_with_failing_listener_should_not_cause_stack_overflow() throws Exception {
+
+        // Set up a failing listener on `log(StatusData)`
+        when(listener.getStatusLevel()).thenReturn(Level.ALL);
+        final Exception listenerFailure = new RuntimeException("test failure " + Math.random());
+        doThrow(listenerFailure).when(listener).log(any());
+
+        // Log something and verify exception dump
+        final String stderr = SystemStubs.tapSystemErr(() -> STATUS_LOGGER.error("foo"));
+        final String listenerFailureClassName = listenerFailure.getClass().getCanonicalName();
+        assertThat(stderr).contains(listenerFailureClassName + ": " + listenerFailure.getMessage());
+    }
+}

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerLevelTest.java

@@ -16,21 +16,24 @@
  */
 package org.apache.logging.log4j.status;
 
+import static org.apache.logging.log4j.status.StatusLogger.DEFAULT_FALLBACK_LISTENER_LEVEL;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.util.Properties;
 import org.apache.logging.log4j.Level;
 import org.junit.jupiter.api.Test;
+import uk.org.webcompere.systemstubs.SystemStubs;
 
 class StatusLoggerLevelTest {
 
     @Test
     void effective_level_should_be_the_least_specific_one() {
 
         // Verify the initial level
-        final StatusLogger logger = StatusLogger.getLogger();
-        final Level fallbackListenerLevel = Level.ERROR;
+        final StatusLogger logger = new StatusLogger();
+        final Level fallbackListenerLevel = DEFAULT_FALLBACK_LISTENER_LEVEL;
         assertThat(logger.getLevel()).isEqualTo(fallbackListenerLevel);
 
         // Register a less specific listener
@@ -82,4 +85,23 @@ void effective_level_should_be_the_least_specific_one() {
         logger.removeListener(listener1);
         assertThat(logger.getLevel()).isEqualTo(fallbackListenerLevel); // Verify that the level is changed
     }
+
+    @Test
+    void invalid_level_should_cause_fallback_to_defaults() throws Exception {
+
+        // Create a `StatusLogger` configuration using an invalid level
+        final Properties statusLoggerConfigProperties = new Properties();
+        final String invalidLevelName = "FOO";
+        statusLoggerConfigProperties.put(StatusLogger.DEFAULT_STATUS_LISTENER_LEVEL, invalidLevelName);
+        final StatusLogger.Config[] statusLoggerConfigRef = {null};
+        final String stderr = SystemStubs.tapSystemErr(
+                () -> statusLoggerConfigRef[0] = new StatusLogger.Config(statusLoggerConfigProperties));
+        final StatusLogger.Config statusLoggerConfig = statusLoggerConfigRef[0];
+
+        // Verify the stderr dump
+        assertThat(stderr).contains("Failed reading the level");
+
+        // Verify the level
+        assertThat(statusLoggerConfig.fallbackListenerLevel).isEqualTo(DEFAULT_FALLBACK_LISTENER_LEVEL);
+    }
 }

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerPropertiesUtilDoubleTest.java

@@ -0,0 +1,112 @@
+/*
+ * 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.status;
+
+import static org.apache.logging.log4j.status.StatusLogger.PropertiesUtilsDouble.readAllAvailableProperties;
+import static org.apache.logging.log4j.status.StatusLogger.PropertiesUtilsDouble.readProperty;
+import static org.assertj.core.api.Assertions.assertThat;
+import static uk.org.webcompere.systemstubs.SystemStubs.restoreSystemProperties;
+import static uk.org.webcompere.systemstubs.SystemStubs.withEnvironmentVariable;
+
+import java.util.Arrays;
+import java.util.Map;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.parallel.ResourceAccessMode;
+import org.junit.jupiter.api.parallel.ResourceLock;
+import org.junit.jupiter.api.parallel.Resources;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class StatusLoggerPropertiesUtilDoubleTest {
+
+    private static final String[] MATCHING_PROPERTY_NAMES = new String[] {
+        // System properties for version range `[, 2.10)`
+        "log4j2.StatusLogger.DateFormat",
+        // System properties for version range `[2.10, 3)`
+        "log4j2.statusLoggerDateFormat",
+        // System properties for version range `[3,)`
+        "log4j2.StatusLogger.dateFormat",
+        // Environment variables
+        "LOG4J_STATUS_LOGGER_DATE_FORMAT"
+    };
+
+    private static final String[] NOT_MATCHING_PROPERTY_NAMES =
+            new String[] {"log4j2.StatusLogger$DateFormat", "log4j2.StàtusLögger.DateFormat"};
+
+    private static final class TestCase {
+
+        private final boolean matching;
+
+        private final String propertyName;
+
+        private final String userProvidedPropertyName;
+
+        private TestCase(final boolean matching, final String propertyName, final String userProvidedPropertyName) {
+            this.matching = matching;
+            this.propertyName = propertyName;
+            this.userProvidedPropertyName = userProvidedPropertyName;
+        }
+
+        @Override
+        public String toString() {
+            return String.format("`%s` %s `%s`", propertyName, matching ? "==" : "!=", userProvidedPropertyName);
+        }
+    }
+
+    static Stream<TestCase> testCases() {
+        return Stream.concat(
+                testCases(true, MATCHING_PROPERTY_NAMES, MATCHING_PROPERTY_NAMES),
+                testCases(false, MATCHING_PROPERTY_NAMES, NOT_MATCHING_PROPERTY_NAMES));
+    }
+
+    private static Stream<TestCase> testCases(
+            final boolean matching, final String[] propertyNames, final String[] userProvidedPropertyNames) {
+        return Arrays.stream(propertyNames).flatMap(propertyName -> Arrays.stream(userProvidedPropertyNames)
+                .map(userProvidedPropertyName -> new TestCase(matching, propertyName, userProvidedPropertyName)));
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCases")
+    @ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ_WRITE)
+    void system_properties_should_work(final TestCase testCase) throws Exception {
+        restoreSystemProperties(() -> {
+            final String expectedValue = "foo";
+            System.setProperty(testCase.propertyName, expectedValue);
+            verifyProperty(testCase, expectedValue);
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCases")
+    @ResourceLock(value = Resources.GLOBAL, mode = ResourceAccessMode.READ_WRITE)
+    void environment_variables_should_work(final TestCase testCase) throws Exception {
+        final String expectedValue = "bar";
+        withEnvironmentVariable(testCase.propertyName, expectedValue).execute(() -> {
+            verifyProperty(testCase, expectedValue);
+        });
+    }
+
+    private static void verifyProperty(final TestCase testCase, final String expectedValue) {
+        final Map<String, Object> normalizedProperties = readAllAvailableProperties();
+        final String actualValue = readProperty(normalizedProperties, testCase.userProvidedPropertyName);
+        if (testCase.matching) {
+            assertThat(actualValue).describedAs("" + testCase).isEqualTo(expectedValue);
+        } else {
+            assertThat(actualValue).describedAs("" + testCase).isNull();
+        }
+    }
+}