Consolidate stack trace rendering in Pattern Layout (apache#2691)

Co-authored-by: Piotr P. Karwasz <[email protected]>
Co-authored-by: Volkan Yazıcı <[email protected]>

Author: 3 people

Diff for: log4j-api-test/pom.xml

@@ -144,6 +144,11 @@
       <artifactId>mockito-inline</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+      <scope>test</scope>
+    </dependency>
     <!-- Used by ServiceLoaderUtilTest -->
     <dependency>
       <groupId>org.osgi</groupId>

Diff for: log4j-core-test/pom.xml

@@ -225,6 +225,11 @@
       <artifactId>javax.jms-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>com.sun.mail</groupId>
       <artifactId>javax.mail</artifactId>

Diff for: log4j-core-test/src/test/java/foo/TestFriendlyException.java

@@ -0,0 +1,155 @@
+/*
+ * 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 foo;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.Socket;
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.logging.log4j.util.Constants;
+
+/**
+ * A testing friendly exception featuring
+ * <ul>
+ * <li>Distinct localized message</li>
+ * <li>Non-Log4j package origin<sup>1</sup></li>
+ * <li>Sufficient causal chain depth</li>
+ * <li>Cyclic causal chain</li>
+ * <li>Suppressed exceptions</li>
+ * <li>Clutter-free stack trace (i.e., elements from JUnit, JDK, etc.)</li>
+ * <li>Stack trace elements from named modules<sup>2</sup></li>
+ * </ul>
+ * <p>
+ * <sup>1</sup> Helps with observing stack trace manipulation effects of Log4j.
+ * </p>
+ * <p>
+ * <sup>2</sup> Helps with testing module name serialization.
+ * </p>
+ */
+public final class TestFriendlyException extends RuntimeException {
+
+    static {
+        // Ensure the distinct packaging
+        assertThat(TestFriendlyException.class.getPackage().getName()).doesNotStartWith("org.apache");
+    }
+
+    public static final StackTraceElement NAMED_MODULE_STACK_TRACE_ELEMENT = namedModuleStackTraceElement();
+
+    @SuppressWarnings("resource")
+    private static StackTraceElement namedModuleStackTraceElement() {
+        try {
+            new Socket("0.0.0.0", -1);
+        } catch (final Exception error) {
+            final StackTraceElement[] stackTraceElements = error.getStackTrace();
+            final String socketClassName = Socket.class.getCanonicalName();
+            for (final StackTraceElement stackTraceElement : stackTraceElements) {
+                if (stackTraceElement.getClassName().equals(socketClassName)) {
+                    if (Constants.JAVA_MAJOR_VERSION > 8) {
+                        final String stackTraceElementString = stackTraceElement.toString();
+                        assertThat(stackTraceElementString).startsWith("java.base/");
+                    }
+                    return stackTraceElement;
+                }
+            }
+        }
+        throw new IllegalStateException("should not have reached here");
+    }
+
+    private static final String[] EXCLUDED_CLASS_NAME_PREFIXES = {
+        "java.lang", "jdk.internal", "org.junit", "sun.reflect"
+    };
+
+    public static final TestFriendlyException INSTANCE = create("r", 0, 2, new boolean[] {false}, new boolean[] {true});
+
+    private static TestFriendlyException create(
+            final String name,
+            final int depth,
+            final int maxDepth,
+            final boolean[] circular,
+            final boolean[] namedModuleAllowed) {
+        final TestFriendlyException error = new TestFriendlyException(name, namedModuleAllowed);
+        if (depth < maxDepth) {
+            final TestFriendlyException cause = create(name + "_c", depth + 1, maxDepth, circular, namedModuleAllowed);
+            error.initCause(cause);
+            final TestFriendlyException suppressed =
+                    create(name + "_s", depth + 1, maxDepth, circular, namedModuleAllowed);
+            error.addSuppressed(suppressed);
+            final boolean circularAllowed = depth + 1 == maxDepth && !circular[0];
+            if (circularAllowed) {
+                cause.initCause(error);
+                suppressed.initCause(error);
+                circular[0] = true;
+            }
+        }
+        return error;
+    }
+
+    private TestFriendlyException(final String message, final boolean[] namedModuleAllowed) {
+        super(message);
+        removeExcludedStackTraceElements(namedModuleAllowed);
+    }
+
+    private void removeExcludedStackTraceElements(final boolean[] namedModuleAllowed) {
+        final StackTraceElement[] oldStackTrace = getStackTrace();
+        final boolean[] seenExcludedStackTraceElement = {false};
+        final StackTraceElement[] newStackTrace = Arrays.stream(oldStackTrace)
+                .flatMap(stackTraceElement ->
+                        mapStackTraceElement(stackTraceElement, namedModuleAllowed, seenExcludedStackTraceElement))
+                .toArray(StackTraceElement[]::new);
+        setStackTrace(newStackTrace);
+    }
+
+    private static Stream<StackTraceElement> mapStackTraceElement(
+            final StackTraceElement stackTraceElement,
+            final boolean[] namedModuleAllowed,
+            final boolean[] seenExcludedStackTraceElement) {
+        final Stream<StackTraceElement> filteredStackTraceElement =
+                filterStackTraceElement(stackTraceElement, seenExcludedStackTraceElement);
+        final Stream<StackTraceElement> javaBaseIncludedStackTraceElement =
+                namedModuleIncludedStackTraceElement(namedModuleAllowed);
+        return Stream.concat(javaBaseIncludedStackTraceElement, filteredStackTraceElement);
+    }
+
+    private static Stream<StackTraceElement> filterStackTraceElement(
+            final StackTraceElement stackTraceElement, final boolean[] seenExcludedStackTraceElement) {
+        if (seenExcludedStackTraceElement[0]) {
+            return Stream.empty();
+        }
+        final String className = stackTraceElement.getClassName();
+        for (final String excludedClassNamePrefix : EXCLUDED_CLASS_NAME_PREFIXES) {
+            if (className.startsWith(excludedClassNamePrefix)) {
+                seenExcludedStackTraceElement[0] = true;
+                return Stream.empty();
+            }
+        }
+        return Stream.of(stackTraceElement);
+    }
+
+    private static Stream<StackTraceElement> namedModuleIncludedStackTraceElement(final boolean[] namedModuleAllowed) {
+        if (!namedModuleAllowed[0]) {
+            return Stream.of();
+        }
+        namedModuleAllowed[0] = false;
+        return Stream.of(NAMED_MODULE_STACK_TRACE_ELEMENT);
+    }
+
+    @Override
+    public String getLocalizedMessage() {
+        return getMessage() + " [localized]";
+    }
+}

Diff for: log4j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java

@@ -18,6 +18,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -63,15 +64,20 @@ public void testParametersAreNotLeaked() throws Exception {
         final String line1 = reader.readLine();
         final String line2 = reader.readLine();
         final String line3 = reader.readLine();
+        // line4 is empty line because of the line separator after throwable pattern
         final String line4 = reader.readLine();
         final String line5 = reader.readLine();
+        final String line6 = reader.readLine();
+        final String line7 = reader.readLine();
         reader.close();
         file.delete();
         assertThat(line1, containsString("Message with parameter paramValue"));
         assertThat(line2, containsString("paramValue"));
         assertThat(line3, containsString("paramValue"));
-        assertThat(line4, containsString("paramValue"));
-        assertNull(line5, "Expected only three lines");
+        assertThat(line4, is(""));
+        assertThat(line5, containsString("paramValue"));
+        assertThat(line6, is(""));
+        assertNull(line7, "Expected only six lines");
         final GarbageCollectionHelper gcHelper = new GarbageCollectionHelper();
         gcHelper.run();
         try {

Diff for: log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderOnStartupTest.java

@@ -51,7 +51,7 @@ public class RollingAppenderOnStartupTest {
     @BeforeAll
     public static void setup() throws Exception {
         final Path target = loggingPath.resolve(FILENAME);
-        Files.copy(Paths.get(SOURCE, FILENAME), target, StandardCopyOption.COPY_ATTRIBUTES);
+        Files.copy(Paths.get(SOURCE, FILENAME), target, StandardCopyOption.REPLACE_EXISTING);
         final FileTime newTime = FileTime.from(Instant.now().minus(1, ChronoUnit.DAYS));
         final BasicFileAttributeView attrs = Files.getFileAttributeView(target, BasicFileAttributeView.class);
         attrs.setTimes(newTime, newTime, newTime);

Diff for: log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/NestedLoggingFromThrowableMessageTest.java

@@ -80,7 +80,7 @@ public void testNestedLoggingInLastArgument() throws Exception {
         final Set<String> lines2 = readUniqueLines(file2);
 
         assertEquals("Expected the same data from both appenders", lines1, lines2);
-        assertEquals(2, lines1.size());
+        assertEquals(3, lines1.size());
         assertTrue(lines1.contains("INFO NestedLoggingFromThrowableMessageTest Logging in getMessage "));
         assertTrue(lines1.contains("ERROR NestedLoggingFromThrowableMessageTest Test message"));
     }