Don't create circular throwables and don't throw StackOverflowError i…

…f one gets logged (#11793) #11792 detect loops in throwables to avoid StackOverflowError Signed-off-by: Ludovic Orban <[email protected]>
jetty · May 17, 2024 · c97c995 · c97c995
1 parent 66b13d5
commit c97c995
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 12 deletions.
diff --git a/jetty-core/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/StdErrAppender.java b/jetty-core/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/StdErrAppender.java
@@ -14,7 +14,10 @@
 package org.eclipse.jetty.logging;
 
 import java.io.PrintStream;
+import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.Objects;
+import java.util.Set;
 import java.util.TimeZone;
 
 import org.slf4j.event.Level;
@@ -175,7 +178,7 @@ private void format(StringBuilder builder, JettyLogger logger, Level level, long
             }
             else
             {
-                appendCause(builder, cause, "");
+                appendCause(builder, cause, "", Collections.newSetFromMap(new IdentityHashMap<>()));
             }
         }
     }
@@ -199,9 +202,18 @@ private String renderedLevel(Level level)
         }
     }
 
-    private void appendCause(StringBuilder builder, Throwable cause, String indent)
+    private void appendCause(StringBuilder builder, Throwable cause, String indent, Set<Throwable> visited)
     {
         builder.append(EOL).append(indent);
+        if (visited.contains(cause))
+        {
+            builder.append("[CIRCULAR REFERENCE: ");
+            appendEscaped(builder, cause.toString());
+            builder.append("]");
+            return;
+        }
+        visited.add(cause);
+
         appendEscaped(builder, cause.toString());
         StackTraceElement[] elements = cause.getStackTrace();
         for (int i = 0; elements != null && i < elements.length; i++)
@@ -213,14 +225,14 @@ private void appendCause(StringBuilder builder, Throwable cause, String indent)
         for (Throwable suppressed : cause.getSuppressed())
         {
             builder.append(EOL).append(indent).append("Suppressed: ");
-            appendCause(builder, suppressed, "\t|" + indent);
+            appendCause(builder, suppressed, "\t|" + indent, visited);
         }
 
         Throwable by = cause.getCause();
         if (by != null && by != cause)
         {
             builder.append(EOL).append(indent).append("Caused by: ");
-            appendCause(builder, by, indent);
+            appendCause(builder, by, indent, visited);
         }
     }
 

diff --git a/jetty-core/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/StdErrAppenderTest.java b/jetty-core/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/StdErrAppenderTest.java
@@ -20,6 +20,27 @@
 
 public class StdErrAppenderTest
 {
+    @Test
+    public void testCircularThrowable()
+    {
+        JettyLoggerConfiguration config = new JettyLoggerConfiguration();
+        JettyLoggerFactory factory = new JettyLoggerFactory(config);
+        CapturedStream output = new CapturedStream();
+        StdErrAppender appender = (StdErrAppender)factory.getRootLogger().getAppender();
+        appender.setStream(output);
+        JettyLogger logger = factory.getJettyLogger("org.eclipse.jetty.logging.LogTest");
+
+        // Build an exception with circular refs.
+        IllegalArgumentException commonCause = new IllegalArgumentException();
+        Throwable thrown = new Throwable(commonCause);
+        RuntimeException suppressed = new RuntimeException(thrown);
+        thrown.addSuppressed(suppressed);
+
+        appender.emit(logger, Level.INFO, System.currentTimeMillis(), "tname", thrown, "the message");
+
+        output.assertContains("CIRCULAR REFERENCE");
+    }
+
     @Test
     public void testStdErrLogFormat()
     {

diff --git a/...10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextEvent.java b/...10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextEvent.java
@@ -20,6 +20,7 @@
 import jakarta.servlet.ServletResponse;
 import jakarta.servlet.http.HttpServletRequest;
 import org.eclipse.jetty.http.HttpURI;
+import org.eclipse.jetty.util.ExceptionUtil;
 import org.eclipse.jetty.util.thread.Scheduler;
 
 public class AsyncContextEvent extends AsyncEvent implements Runnable
@@ -149,9 +150,6 @@ public void run()
 
     public void addThrowable(Throwable e)
     {
-        if (_throwable == null)
-            _throwable = e;
-        else if (e != _throwable)
-            _throwable.addSuppressed(e);
+        _throwable = ExceptionUtil.combine(_throwable, e);
     }
 }
diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContextEvent.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContextEvent.java
@@ -19,6 +19,7 @@
 import jakarta.servlet.ServletRequest;
 import jakarta.servlet.ServletResponse;
 import org.eclipse.jetty.http.HttpURI;
+import org.eclipse.jetty.util.ExceptionUtil;
 import org.eclipse.jetty.util.thread.Scheduler;
 
 public class AsyncContextEvent extends AsyncEvent implements Runnable
@@ -149,9 +150,6 @@ public void run()
 
     public void addThrowable(Throwable e)
     {
-        if (_throwable == null)
-            _throwable = e;
-        else if (e != _throwable)
-            _throwable.addSuppressed(e);
+        _throwable = ExceptionUtil.combine(_throwable, e);
     }
 }