Skip to content

Commit 684dfca

Browse files
cekiceki
committed
Fix SLF4J-421 and SLF4J-287
1 parent 05486b9 commit 684dfca

File tree

5 files changed

+70
-69
lines changed

5 files changed

+70
-69
lines changed

‎slf4j-api/src/main/java/org/slf4j/spi/LocationAwareLogger.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@
3939
*/
4040
public interface LocationAwareLogger extends Logger {
4141

42-
// these constants should be in EventContants. However, in order to preserve binary backward compatibility
43-
// we keep these constants here
42+
// these constants should be in EventConstants. However, in order to preserve binary backward compatibility
43+
// we keep these constants here. {@link EventConstants} redefines these constants using the values below.
4444
final public int TRACE_INT = 00;
4545
final public int DEBUG_INT = 10;
4646
final public int INFO_INT = 20;

‎slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,15 @@ public void testNull() {
4949
assertEquals(null, result);
5050
}
5151

52+
@Test
53+
public void testParamaterContainingAnAnchor() {
54+
result = MessageFormatter.format("Value is {}.", "[{}]").getMessage();
55+
assertEquals("Value is [{}].", result);
56+
57+
result = MessageFormatter.format("Values are {} and {}.", i1, "[{}]").getMessage();
58+
assertEquals("Values are 1 and [{}].", result);
59+
}
60+
5261
@Test
5362
public void nullParametersShouldBeHandledWithoutBarfing() {
5463
result = MessageFormatter.format("Value is {}.", null).getMessage();

‎slf4j-ext/src/main/java/org/slf4j/ext/LoggerWrapper.java

Lines changed: 33 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626

2727
import org.slf4j.Logger;
2828
import org.slf4j.Marker;
29-
import org.slf4j.helpers.FormattingTuple;
30-
import org.slf4j.helpers.MessageFormatter;
29+
//import org.slf4j.helpers.FormattingTuple;
30+
//import org.slf4j.helpers.MessageFormatter;
3131
import org.slf4j.spi.LocationAwareLogger;
3232

3333
/**
@@ -40,8 +40,7 @@
4040
public class LoggerWrapper implements Logger {
4141

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

10099
if (instanceofLAL) {
101-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
102-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg }, null);
100+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg }, null);
103101
} else {
104102
logger.trace(format, arg);
105103
}
@@ -113,8 +111,7 @@ public void trace(String format, Object arg1, Object arg2) {
113111
return;
114112

115113
if (instanceofLAL) {
116-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
117-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
114+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg1, arg2 }, null);
118115
} else {
119116
logger.trace(format, arg1, arg2);
120117
}
@@ -128,8 +125,7 @@ public void trace(String format, Object... args) {
128125
return;
129126

130127
if (instanceofLAL) {
131-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
132-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, args, null);
128+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.TRACE_INT, format, args, null);
133129
} else {
134130
logger.trace(format, args);
135131
}
@@ -169,8 +165,7 @@ public void trace(Marker marker, String format, Object arg) {
169165
if (!logger.isTraceEnabled(marker))
170166
return;
171167
if (instanceofLAL) {
172-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
173-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg }, null);
168+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg }, null);
174169
} else {
175170
logger.trace(marker, format, arg);
176171
}
@@ -183,8 +178,7 @@ public void trace(Marker marker, String format, Object arg1, Object arg2) {
183178
if (!logger.isTraceEnabled(marker))
184179
return;
185180
if (instanceofLAL) {
186-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
187-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
181+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, format, new Object[] { arg1, arg2 }, null);
188182
} else {
189183
logger.trace(marker, format, arg1, arg2);
190184
}
@@ -197,8 +191,7 @@ public void trace(Marker marker, String format, Object... args) {
197191
if (!logger.isTraceEnabled(marker))
198192
return;
199193
if (instanceofLAL) {
200-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
201-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, formattedMessage, args, null);
194+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.TRACE_INT, format, args, null);
202195
} else {
203196
logger.trace(marker, format, args);
204197
}
@@ -253,8 +246,7 @@ public void debug(String format, Object arg) {
253246
return;
254247

255248
if (instanceofLAL) {
256-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
257-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, formattedMessage, new Object[] { arg }, null);
249+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg }, null);
258250
} else {
259251
logger.debug(format, arg);
260252
}
@@ -268,8 +260,7 @@ public void debug(String format, Object arg1, Object arg2) {
268260
return;
269261

270262
if (instanceofLAL) {
271-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
272-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
263+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg1, arg2 }, null);
273264
} else {
274265
logger.debug(format, arg1, arg2);
275266
}
@@ -283,8 +274,7 @@ public void debug(String format, Object... argArray) {
283274
return;
284275

285276
if (instanceofLAL) {
286-
FormattingTuple ft = MessageFormatter.arrayFormat(format, argArray);
287-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, ft.getMessage(), ft.getArgArray(), ft.getThrowable());
277+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.DEBUG_INT, format, argArray, null);
288278
} else {
289279
logger.debug(format, argArray);
290280
}
@@ -324,8 +314,7 @@ public void debug(Marker marker, String format, Object arg) {
324314
if (!logger.isDebugEnabled(marker))
325315
return;
326316
if (instanceofLAL) {
327-
FormattingTuple ft = MessageFormatter.format(format, arg);
328-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, ft.getMessage(), ft.getArgArray(), ft.getThrowable());
317+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg }, null);
329318
} else {
330319
logger.debug(marker, format, arg);
331320
}
@@ -338,8 +327,7 @@ public void debug(Marker marker, String format, Object arg1, Object arg2) {
338327
if (!logger.isDebugEnabled(marker))
339328
return;
340329
if (instanceofLAL) {
341-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
342-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
330+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, format, new Object[] { arg1, arg2 }, null);
343331
} else {
344332
logger.debug(marker, format, arg1, arg2);
345333
}
@@ -353,8 +341,7 @@ public void debug(Marker marker, String format, Object... argArray) {
353341
return;
354342
if (instanceofLAL) {
355343

356-
FormattingTuple ft = MessageFormatter.arrayFormat(format, argArray);
357-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, ft.getMessage(), argArray, ft.getThrowable());
344+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.DEBUG_INT, format, argArray, null);
358345
} else {
359346
logger.debug(marker, format, argArray);
360347
}
@@ -409,8 +396,7 @@ public void info(String format, Object arg) {
409396
return;
410397

411398
if (instanceofLAL) {
412-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
413-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg }, null);
399+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg }, null);
414400
} else {
415401
logger.info(format, arg);
416402
}
@@ -424,8 +410,7 @@ public void info(String format, Object arg1, Object arg2) {
424410
return;
425411

426412
if (instanceofLAL) {
427-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
428-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
413+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg1, arg2 }, null);
429414
} else {
430415
logger.info(format, arg1, arg2);
431416
}
@@ -439,8 +424,7 @@ public void info(String format, Object... args) {
439424
return;
440425

441426
if (instanceofLAL) {
442-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
443-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, args, null);
427+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.INFO_INT, format, args, null);
444428
} else {
445429
logger.info(format, args);
446430
}
@@ -480,8 +464,7 @@ public void info(Marker marker, String format, Object arg) {
480464
if (!logger.isInfoEnabled(marker))
481465
return;
482466
if (instanceofLAL) {
483-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
484-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg }, null);
467+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg }, null);
485468
} else {
486469
logger.info(marker, format, arg);
487470
}
@@ -494,8 +477,7 @@ public void info(Marker marker, String format, Object arg1, Object arg2) {
494477
if (!logger.isInfoEnabled(marker))
495478
return;
496479
if (instanceofLAL) {
497-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
498-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
480+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, format, new Object[] { arg1, arg2 }, null);
499481
} else {
500482
logger.info(marker, format, arg1, arg2);
501483
}
@@ -508,8 +490,7 @@ public void info(Marker marker, String format, Object... args) {
508490
if (!logger.isInfoEnabled(marker))
509491
return;
510492
if (instanceofLAL) {
511-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
512-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, formattedMessage, args, null);
493+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.INFO_INT, format, args, null);
513494
} else {
514495
logger.info(marker, format, args);
515496
}
@@ -561,8 +542,7 @@ public void warn(String format, Object arg) {
561542
return;
562543

563544
if (instanceofLAL) {
564-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
565-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg }, null);
545+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg }, null);
566546
} else {
567547
logger.warn(format, arg);
568548
}
@@ -576,8 +556,7 @@ public void warn(String format, Object arg1, Object arg2) {
576556
return;
577557

578558
if (instanceofLAL) {
579-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
580-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
559+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg1, arg2 }, null);
581560
} else {
582561
logger.warn(format, arg1, arg2);
583562
}
@@ -591,8 +570,7 @@ public void warn(String format, Object... args) {
591570
return;
592571

593572
if (instanceofLAL) {
594-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
595-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, args, null);
573+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.WARN_INT, format, args, null);
596574
} else {
597575
logger.warn(format, args);
598576
}
@@ -632,8 +610,7 @@ public void warn(Marker marker, String format, Object arg) {
632610
if (!logger.isWarnEnabled(marker))
633611
return;
634612
if (instanceofLAL) {
635-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
636-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg }, null);
613+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg }, null);
637614
} else {
638615
logger.warn(marker, format, arg);
639616
}
@@ -646,8 +623,7 @@ public void warn(Marker marker, String format, Object arg1, Object arg2) {
646623
if (!logger.isWarnEnabled(marker))
647624
return;
648625
if (instanceofLAL) {
649-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
650-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
626+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, format, new Object[] { arg1, arg2 }, null);
651627
} else {
652628
logger.warn(marker, format, arg1, arg2);
653629
}
@@ -660,8 +636,7 @@ public void warn(Marker marker, String format, Object... args) {
660636
if (!logger.isWarnEnabled(marker))
661637
return;
662638
if (instanceofLAL) {
663-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
664-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, formattedMessage, args, null);
639+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.WARN_INT, format, args, null);
665640
} else {
666641
logger.warn(marker, format, args);
667642
}
@@ -716,8 +691,7 @@ public void error(String format, Object arg) {
716691
return;
717692

718693
if (instanceofLAL) {
719-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
720-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg }, null);
694+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg }, null);
721695
} else {
722696
logger.error(format, arg);
723697
}
@@ -731,8 +705,7 @@ public void error(String format, Object arg1, Object arg2) {
731705
return;
732706

733707
if (instanceofLAL) {
734-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
735-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
708+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg1, arg2 }, null);
736709
} else {
737710
logger.error(format, arg1, arg2);
738711
}
@@ -746,8 +719,7 @@ public void error(String format, Object... args) {
746719
return;
747720

748721
if (instanceofLAL) {
749-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
750-
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, args, null);
722+
((LocationAwareLogger) logger).log(null, fqcn, LocationAwareLogger.ERROR_INT, format, args, null);
751723
} else {
752724
logger.error(format, args);
753725
}
@@ -787,8 +759,7 @@ public void error(Marker marker, String format, Object arg) {
787759
if (!logger.isErrorEnabled(marker))
788760
return;
789761
if (instanceofLAL) {
790-
String formattedMessage = MessageFormatter.format(format, arg).getMessage();
791-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg }, null);
762+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg }, null);
792763
} else {
793764
logger.error(marker, format, arg);
794765
}
@@ -801,8 +772,7 @@ public void error(Marker marker, String format, Object arg1, Object arg2) {
801772
if (!logger.isErrorEnabled(marker))
802773
return;
803774
if (instanceofLAL) {
804-
String formattedMessage = MessageFormatter.format(format, arg1, arg2).getMessage();
805-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, new Object[] { arg1, arg2 }, null);
775+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, format, new Object[] { arg1, arg2 }, null);
806776
} else {
807777
logger.error(marker, format, arg1, arg2);
808778
}
@@ -815,8 +785,7 @@ public void error(Marker marker, String format, Object... args) {
815785
if (!logger.isErrorEnabled(marker))
816786
return;
817787
if (instanceofLAL) {
818-
String formattedMessage = MessageFormatter.arrayFormat(format, args).getMessage();
819-
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, formattedMessage, args, null);
788+
((LocationAwareLogger) logger).log(marker, fqcn, LocationAwareLogger.ERROR_INT, format, args, null);
820789
} else {
821790
logger.error(marker, format, args);
822791
}

‎slf4j-ext/src/test/java/org/slf4j/dummyExt/XLoggerTest.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,17 @@ public void testLocationExtraction_Bug114() {
150150
assertEquals(this.getClass().getName(), li.getClassName());
151151
assertEquals("" + (line + 1), li.getLineNumber());
152152
}
153-
154153
}
154+
155+
@Test
156+
public void testNoDoubleSubstitution_Bug421() {
157+
XLogger logger = XLoggerFactory.getXLogger("UnitTest");
158+
logger.error("{},{}", "foo", "[{}]");
159+
verify((LoggingEvent) listAppender.list.get(0), "foo,[{}]");
160+
161+
logger.error("{},{}", "[{}]", "foo");
162+
verify((LoggingEvent) listAppender.list.get(1), "[{}],foo");
163+
}
164+
165+
155166
}

0 commit comments

Comments
 (0)