From 8068f198ee6095a4e26e3487b42f89d21ab9f89b Mon Sep 17 00:00:00 2001
From: arimu1 <19286898+arimu1@users.noreply.github.com>
Date: Thu, 6 Aug 2026 08:54:45 +0700
Subject: [PATCH] fix(api): guard addKeyValue value toString from fatal errors

When a key-value value's toString() throws (including StackOverflowError),
mergeKeyValuePairs previously aborted logging. MessageFormatter already
substitutes [FAILED toString()] for message arguments; apply the same
protection when formatting key-value pairs into the merged message.

Fixes #448

Signed-off-by: arimu1 <19286898+arimu1@users.noreply.github.com>
---
 .../slf4j/spi/DefaultLoggingEventBuilder.java | 24 ++++++++++++++-
 .../slf4j/jul/FluentApiInvocationTest.java    | 29 +++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java b/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java
index de0d42fe..337a3bae 100755
--- a/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java
+++ b/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java
@@ -32,6 +32,7 @@ import org.slf4j.event.DefaultLoggingEvent;
 import org.slf4j.event.KeyValuePair;
 import org.slf4j.event.Level;
 import org.slf4j.event.LoggingEvent;
+import org.slf4j.helpers.Reporter;
 
 /**
  * Default implementation of {@link LoggingEventBuilder}.
@@ -259,12 +260,33 @@ public class DefaultLoggingEventBuilder implements LoggingEventBuilder, CallerBo
         for(KeyValuePair kvp : keyValuePairList) {
             sb.append(kvp.key);
             sb.append('=');
-            sb.append(kvp.value);
+            // Same protection as MessageFormatter.safeObjectAppend: a failing
+            // toString() (including StackOverflowError) must not abort logging.
+            // See https://github.com/qos-ch/slf4j/issues/448
+            safeObjectAppend(sb, kvp.value);
             sb.append(' ');
         }
         return sb;
     }
 
+    /**
+     * Append {@code o} to {@code sb}, catching any {@link Throwable} thrown by
+     * {@link Object#toString()} and substituting {@code [FAILED toString()]}.
+     * Mirrors {@code MessageFormatter.safeObjectAppend}.
+     */
+    private static void safeObjectAppend(StringBuilder sb, Object o) {
+        if (o == null) {
+            sb.append("null");
+            return;
+        }
+        try {
+            sb.append(o.toString());
+        } catch (Throwable t) {
+            Reporter.error("Failed toString() invocation on an object of type [" + o.getClass().getName() + "]", t);
+            sb.append("[FAILED toString()]");
+        }
+    }
+
     private String mergeMessage(String msg, StringBuilder sb) {
         if(sb != null) {
             sb.append(msg);
diff --git a/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java b/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java
index dc37ac21..08b3c925 100755
--- a/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java
+++ b/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java
@@ -130,6 +130,35 @@ public class FluentApiInvocationTest {
 
     }
 
+    /**
+     * Regression for https://github.com/qos-ch/slf4j/issues/448:
+     * a toString() that throws (including StackOverflowError) on an
+     * addKeyValue value must not abort logging; same as message args.
+     */
+    @Test
+    public void keyValuePairWithFailingToString() {
+        Object bad = new Object() {
+            @Override
+            public String toString() {
+                throw new IllegalStateException("boom");
+            }
+        };
+        logger.atDebug().addKeyValue("key", bad).log("msg with key/value");
+        assertLogMessage("key=[FAILED toString()] msg with key/value", 0);
+    }
+
+    @Test
+    public void keyValuePairWithStackOverflowInToString() {
+        Object overflow = new Object() {
+            @Override
+            public String toString() {
+                return super.toString() + this.toString();
+            }
+        };
+        logger.atDebug().addKeyValue("key", overflow).log("msg with key/value");
+        assertLogMessage("key=[FAILED toString()] msg with key/value", 0);
+    }
+
     private void assertLogMessage(String expected, int index) {
         LogRecord logRecord = listHandler.recordList.get(index);
         Assert.assertNotNull(logRecord);