mirror of https://github.com/qos-ch/slf4j
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>
This commit is contained in:
parent
55e67102a8
commit
8068f198ee
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue