Skip to content

Commit

Permalink
Fix SLF4J-421 and SLF4J-287
Browse files Browse the repository at this point in the history
  • Loading branch information
@ceki
ceki committed Aug 10, 2021
1 parent 3e0016d commit 65eacb3
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 69 deletions.
4 changes: 2 additions & 2 deletions slf4j-api/src/main/java/org/slf4j/spi/LocationAwareLogger.java
View file
Expand Up @@ -39,8 +39,8 @@
*/
public interface LocationAwareLogger extends Logger {

// these constants should be in EventContants. However, in order to preserve binary backward compatibility
// we keep these constants here
// these constants should be in EventConstants. However, in order to preserve binary backward compatibility
// we keep these constants here. {@link EventConstants} redefines these constants using the values below.
final public int TRACE_INT = 00;
final public int DEBUG_INT = 10;
final public int INFO_INT = 20;
Expand Down
9 changes: 9 additions & 0 deletions slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java
View file
Expand Up @@ -49,6 +49,15 @@ public void testNull() {
assertEquals(null, result);
}

@Test
public void testParamaterContainingAnAnchor() {
result = MessageFormatter.format("Value is {}.", "[{}]").getMessage();
assertEquals("Value is [{}].", result);

result = MessageFormatter.format("Values are {} and {}.", i1, "[{}]").getMessage();
assertEquals("Values are 1 and [{}].", result);
}

@Test
public void nullParametersShouldBeHandledWithoutBarfing() {
result = MessageFormatter.format("Value is {}.", null).getMessage();
Expand Down
97 changes: 33 additions & 64 deletions slf4j-ext/src/main/java/org/slf4j/ext/LoggerWrapper.java
View file
Expand Up @@ -26,8 +26,8 @@

import org.slf4j.Logger;
import org.slf4j.Marker;
import org.slf4j.helpers.FormattingTuple;
import org.slf4j.helpers.MessageFormatter;
//import org.slf4j.helpers.FormattingTuple;
//import org.slf4j.helpers.MessageFormatter;
import org.slf4j.spi.LocationAwareLogger;

/**
Expand All @@ -40,8 +40,7 @@
public class LoggerWrapper implements Logger {

// To ensure consistency between two instances sharing the same name
// (homonyms)
// a LoggerWrapper should not contain any state beyond
// (homonyms) a LoggerWrapper should not contain any state beyond
// the Logger instance it wraps.
// Note that 'instanceofLAL' directly depends on Logger.
// fqcn depend on the caller, but its value would not be different
Expand Down Expand Up @@ -98,8 +97,7 @@ public void trace(String format, Object arg) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg }, null);
} else {
logger.trace(format, arg);
}
Expand All @@ -113,8 +111,7 @@ public void trace(String format, Object arg1, Object arg2) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.trace(format, arg1, arg2);
}
Expand All @@ -128,8 +125,7 @@ public void trace(String format, Object... args) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, format, args, null);
} else {
logger.trace(format, args);
}
Expand Down Expand Up @@ -169,8 +165,7 @@ public void trace(Marker marker, String format, Object arg) {
if (!logger.isTraceEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg }, null);
} else {
logger.trace(marker, format, arg);
}
Expand All @@ -183,8 +178,7 @@ public void trace(Marker marker, String format, Object arg1, Object arg2) {
if (!logger.isTraceEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.trace(marker, format, arg1, arg2);
}
Expand All @@ -197,8 +191,7 @@ public void trace(Marker marker, String format, Object... args) {
if (!logger.isTraceEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, format, args, null);
} else {
logger.trace(marker, format, args);
}
Expand Down Expand Up @@ -253,8 +246,7 @@ public void debug(String format, Object arg) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg }, null);
} else {
logger.debug(format, arg);
}
Expand All @@ -268,8 +260,7 @@ public void debug(String format, Object arg1, Object arg2) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.debug(format, arg1, arg2);
}
Expand All @@ -283,8 +274,7 @@ public void debug(String format, Object... argArray) {
return;

if (instanceofLAL) {
FormattingTuple ft = MessageFormatter.arrayFormat(format, argArray);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, ft.getMessage(), ft.getArgArray(), ft.getThrowable());
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, format, argArray, null);
} else {
logger.debug(format, argArray);
}
Expand Down Expand Up @@ -324,8 +314,7 @@ public void debug(Marker marker, String format, Object arg) {
if (!logger.isDebugEnabled(marker))
return;
if (instanceofLAL) {
FormattingTuple ft = MessageFormatter.format(format, arg);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, ft.getMessage(), ft.getArgArray(), ft.getThrowable());
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg }, null);
} else {
logger.debug(marker, format, arg);
}
Expand All @@ -338,8 +327,7 @@ public void debug(Marker marker, String format, Object arg1, Object arg2) {
if (!logger.isDebugEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.debug(marker, format, arg1, arg2);
}
Expand All @@ -353,8 +341,7 @@ public void debug(Marker marker, String format, Object... argArray) {
return;
if (instanceofLAL) {

FormattingTuple ft = MessageFormatter.arrayFormat(format, argArray);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, ft.getMessage(), argArray, ft.getThrowable());
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, format, argArray, null);
} else {
logger.debug(marker, format, argArray);
}
Expand Down Expand Up @@ -409,8 +396,7 @@ public void info(String format, Object arg) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg }, null);
} else {
logger.info(format, arg);
}
Expand All @@ -424,8 +410,7 @@ public void info(String format, Object arg1, Object arg2) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.info(format, arg1, arg2);
}
Expand All @@ -439,8 +424,7 @@ public void info(String format, Object... args) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, format, args, null);
} else {
logger.info(format, args);
}
Expand Down Expand Up @@ -480,8 +464,7 @@ public void info(Marker marker, String format, Object arg) {
if (!logger.isInfoEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg }, null);
} else {
logger.info(marker, format, arg);
}
Expand All @@ -494,8 +477,7 @@ public void info(Marker marker, String format, Object arg1, Object arg2) {
if (!logger.isInfoEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.info(marker, format, arg1, arg2);
}
Expand All @@ -508,8 +490,7 @@ public void info(Marker marker, String format, Object... args) {
if (!logger.isInfoEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, format, args, null);
} else {
logger.info(marker, format, args);
}
Expand Down Expand Up @@ -561,8 +542,7 @@ public void warn(String format, Object arg) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg }, null);
} else {
logger.warn(format, arg);
}
Expand All @@ -576,8 +556,7 @@ public void warn(String format, Object arg1, Object arg2) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.warn(format, arg1, arg2);
}
Expand All @@ -591,8 +570,7 @@ public void warn(String format, Object... args) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, format, args, null);
} else {
logger.warn(format, args);
}
Expand Down Expand Up @@ -632,8 +610,7 @@ public void warn(Marker marker, String format, Object arg) {
if (!logger.isWarnEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg }, null);
} else {
logger.warn(marker, format, arg);
}
Expand All @@ -646,8 +623,7 @@ public void warn(Marker marker, String format, Object arg1, Object arg2) {
if (!logger.isWarnEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.warn(marker, format, arg1, arg2);
}
Expand All @@ -660,8 +636,7 @@ public void warn(Marker marker, String format, Object... args) {
if (!logger.isWarnEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, format, args, null);
} else {
logger.warn(marker, format, args);
}
Expand Down Expand Up @@ -716,8 +691,7 @@ public void error(String format, Object arg) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg }, null);
} else {
logger.error(format, arg);
}
Expand All @@ -731,8 +705,7 @@ public void error(String format, Object arg1, Object arg2) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.error(format, arg1, arg2);
}
Expand All @@ -746,8 +719,7 @@ public void error(String format, Object... args) {
return;

if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, format, args, null);
} else {
logger.error(format, args);
}
Expand Down Expand Up @@ -787,8 +759,7 @@ public void error(Marker marker, String format, Object arg) {
if (!logger.isErrorEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg }, null);
} else {
logger.error(marker, format, arg);
}
Expand All @@ -801,8 +772,7 @@ public void error(Marker marker, String format, Object arg1, Object arg2) {
if (!logger.isErrorEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg1, arg2 }, null);
} else {
logger.error(marker, format, arg1, arg2);
}
Expand All @@ -815,8 +785,7 @@ public void error(Marker marker, String format, Object... args) {
if (!logger.isErrorEnabled(marker))
return;
if (instanceofLAL) {
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, args, null);
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, format, args, null);
} else {
logger.error(marker, format, args);
}
Expand Down
13 changes: 12 additions & 1 deletion slf4j-ext/src/test/java/org/slf4j/dummyExt/XLoggerTest.java
View file
Expand Up @@ -150,6 +150,17 @@ public void testLocationExtraction_Bug114() {
assertEquals(this.getClass().getName(), li.getClassName());
assertEquals("" + (line + 1), li.getLineNumber());
}

}

@Test
public void testNoDoubleSubstitution_Bug421() {
XLogger logger = XLoggerFactory.getXLogger("UnitTest");
logger.error("{},{}", "foo", "[{}]");
verify((LoggingEvent) listAppender.list.get(0), "foo,[{}]");

logger.error("{},{}", "[{}]", "foo");
verify((LoggingEvent) listAppender.list.get(1), "[{}],foo");
}


}

0 comments on commit 65eacb3

Please sign in to comment.