Skip to content

Commit 04637dd

Browse files
rgoersrgoers
committed
LOG4J2-3198 - Log4j2 no longer formats lookups in messages by default
1 parent d82b47c commit 04637dd

File tree

11 files changed

+205
-92
lines changed

11 files changed

+205
-92
lines changed

‎log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java‎

Lines changed: 119 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
package org.apache.logging.log4j.core.pattern;
1818

19+
import java.util.ArrayList;
20+
import java.util.List;
1921
import java.util.Locale;
2022

2123
import org.apache.logging.log4j.core.LogEvent;
@@ -37,43 +39,27 @@
3739
@Plugin(name = "MessagePatternConverter", category = PatternConverter.CATEGORY)
3840
@ConverterKeys({ "m", "msg", "message" })
3941
@PerformanceSensitive("allocation")
40-
public final class MessagePatternConverter extends LogEventPatternConverter {
42+
public class MessagePatternConverter extends LogEventPatternConverter {
4143

44+
private static final String LOOKUPS = "lookups";
4245
private static final String NOLOOKUPS = "nolookups";
4346

44-
private final String[] formats;
45-
private final Configuration config;
46-
private final TextRenderer textRenderer;
47-
private final boolean noLookups;
48-
49-
/**
50-
* Private constructor.
51-
*
52-
* @param options
53-
* options, may be null.
54-
*/
55-
private MessagePatternConverter(final Configuration config, final String[] options) {
47+
private MessagePatternConverter() {
5648
super("Message", "message");
57-
this.formats = options;
58-
this.config = config;
59-
final int noLookupsIdx = loadNoLookups(options);
60-
this.noLookups = Constants.FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS || noLookupsIdx >= 0;
61-
this.textRenderer = loadMessageRenderer(noLookupsIdx >= 0 ? ArrayUtils.remove(options, noLookupsIdx) : options);
6249
}
6350

64-
private int loadNoLookups(final String[] options) {
51+
private static boolean loadLookups(final String[] options) {
6552
if (options != null) {
66-
for (int i = 0; i < options.length; i++) {
67-
final String option = options[i];
68-
if (NOLOOKUPS.equalsIgnoreCase(option)) {
69-
return i;
53+
for (final String option : options) {
54+
if (LOOKUPS.equalsIgnoreCase(option)) {
55+
return true;
7056
}
7157
}
7258
}
73-
return -1;
59+
return false;
7460
}
7561

76-
private TextRenderer loadMessageRenderer(final String[] options) {
62+
private static TextRenderer loadMessageRenderer(final String[] options) {
7763
if (options != null) {
7864
for (final String option : options) {
7965
switch (option.toUpperCase(Locale.ROOT)) {
@@ -102,55 +88,129 @@ private TextRenderer loadMessageRenderer(final String[] options) {
10288
* @return instance of pattern converter.
10389
*/
10490
public static MessagePatternConverter newInstance(final Configuration config, final String[] options) {
105-
return new MessagePatternConverter(config, options);
91+
boolean lookups = loadLookups(options);
92+
String[] formats = withoutLookupOptions(options);
93+
TextRenderer textRenderer = loadMessageRenderer(formats);
94+
MessagePatternConverter result = formats == null || formats.length == 0
95+
? SimpleMessagePatternConverter.INSTANCE
96+
: new FormattedMessagePatternConverter(formats);
97+
if (lookups && config != null) {
98+
result = new LookupMessagePatternConverter(result, config);
99+
}
100+
if (textRenderer != null) {
101+
result = new RenderingPatternConverter(result, textRenderer);
102+
}
103+
return result;
104+
}
105+
106+
private static String[] withoutLookupOptions(final String[] options) {
107+
if (options == null || options.length == 0) {
108+
return options;
109+
}
110+
List<String> results = new ArrayList<>(options.length);
111+
for (String option : options) {
112+
if (!LOOKUPS.equalsIgnoreCase(option) && !NOLOOKUPS.equalsIgnoreCase(option)) {
113+
results.add(option);
114+
}
115+
}
116+
return results.toArray(new String[0]);
106117
}
107118

108119
/**
109120
* {@inheritDoc}
110121
*/
111122
@Override
112123
public void format(final LogEvent event, final StringBuilder toAppendTo) {
113-
final Message msg = event.getMessage();
114-
if (msg instanceof StringBuilderFormattable) {
124+
throw new UnsupportedOperationException();
125+
}
115126

116-
final boolean doRender = textRenderer != null;
117-
final StringBuilder workingBuilder = doRender ? new StringBuilder(80) : toAppendTo;
127+
private static final class SimpleMessagePatternConverter extends MessagePatternConverter {
128+
private static final MessagePatternConverter INSTANCE = new SimpleMessagePatternConverter();
118129

119-
final int offset = workingBuilder.length();
120-
if (msg instanceof MultiFormatStringBuilderFormattable) {
121-
((MultiFormatStringBuilderFormattable) msg).formatTo(formats, workingBuilder);
122-
} else {
123-
((StringBuilderFormattable) msg).formatTo(workingBuilder);
130+
/**
131+
* {@inheritDoc}
132+
*/
133+
@Override
134+
public void format(final LogEvent event, final StringBuilder toAppendTo) {
135+
Message msg = event.getMessage();
136+
if (msg instanceof StringBuilderFormattable) {
137+
((StringBuilderFormattable) msg).formatTo(toAppendTo);
138+
} else if (msg != null) {
139+
toAppendTo.append(msg.getFormattedMessage());
124140
}
141+
}
142+
}
125143

126-
// TODO can we optimize this?
127-
if (config != null && !noLookups) {
128-
for (int i = offset; i < workingBuilder.length() - 1; i++) {
129-
if (workingBuilder.charAt(i) == '$' && workingBuilder.charAt(i + 1) == '{') {
130-
final String value = workingBuilder.substring(offset, workingBuilder.length());
131-
workingBuilder.setLength(offset);
132-
workingBuilder.append(config.getStrSubstitutor().replace(event, value));
133-
}
144+
private static final class FormattedMessagePatternConverter extends MessagePatternConverter {
145+
146+
private final String[] formats;
147+
148+
FormattedMessagePatternConverter(final String[] formats) {
149+
this.formats = formats;
150+
}
151+
152+
/**
153+
* {@inheritDoc}
154+
*/
155+
@Override
156+
public void format(final LogEvent event, final StringBuilder toAppendTo) {
157+
Message msg = event.getMessage();
158+
if (msg instanceof StringBuilderFormattable) {
159+
if (msg instanceof MultiFormatStringBuilderFormattable) {
160+
((MultiFormatStringBuilderFormattable) msg).formatTo(formats, toAppendTo);
161+
} else {
162+
((StringBuilderFormattable) msg).formatTo(toAppendTo);
134163
}
164+
} else if (msg != null) {
165+
toAppendTo.append(msg instanceof MultiformatMessage
166+
? ((MultiformatMessage) msg).getFormattedMessage(formats)
167+
: msg.getFormattedMessage());
135168
}
136-
if (doRender) {
137-
textRenderer.render(workingBuilder, toAppendTo);
138-
}
139-
return;
140169
}
141-
if (msg != null) {
142-
final String result;
143-
if (msg instanceof MultiformatMessage) {
144-
result = ((MultiformatMessage) msg).getFormattedMessage(formats);
145-
} else {
146-
result = msg.getFormattedMessage();
147-
}
148-
if (result != null) {
149-
toAppendTo.append(config != null && result.contains("${")
150-
? config.getStrSubstitutor().replace(event, result) : result);
151-
} else {
152-
toAppendTo.append("null");
170+
}
171+
172+
private static final class LookupMessagePatternConverter extends MessagePatternConverter {
173+
private final MessagePatternConverter delegate;
174+
private final Configuration config;
175+
176+
LookupMessagePatternConverter(final MessagePatternConverter delegate, final Configuration config) {
177+
this.delegate = delegate;
178+
this.config = config;
179+
}
180+
181+
/**
182+
* {@inheritDoc}
183+
*/
184+
@Override
185+
public void format(final LogEvent event, final StringBuilder toAppendTo) {
186+
int start = toAppendTo.length();
187+
delegate.format(event, toAppendTo);
188+
int indexOfSubstitution = toAppendTo.indexOf("${", start);
189+
if (indexOfSubstitution >= 0) {
190+
config.getStrSubstitutor()
191+
.replaceIn(event, toAppendTo, indexOfSubstitution, toAppendTo.length() - indexOfSubstitution);
153192
}
154193
}
155194
}
195+
196+
private static final class RenderingPatternConverter extends MessagePatternConverter {
197+
198+
private final MessagePatternConverter delegate;
199+
private final TextRenderer textRenderer;
200+
201+
RenderingPatternConverter(final MessagePatternConverter delegate, final TextRenderer textRenderer) {
202+
this.delegate = delegate;
203+
this.textRenderer = textRenderer;
204+
}
205+
206+
/**
207+
* {@inheritDoc}
208+
*/
209+
@Override
210+
public void format(final LogEvent event, final StringBuilder toAppendTo) {
211+
StringBuilder workingBuilder = new StringBuilder(80);
212+
delegate.format(event, workingBuilder);
213+
textRenderer.render(workingBuilder, toAppendTo);
214+
}
215+
}
156216
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/util/Constants.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,17 @@ public final class Constants {
5555
"log4j.format.msg.async", false);
5656

5757
/**
58-
* LOG4J2-2109 if {@code true}, MessagePatternConverter will always operate as though
59-
* <pre>%m{nolookups}</pre> is configured.
58+
* LOG4J2-3198 property which used to globally opt out of lookups in pattern layout message text, however
59+
* this is the default and this property is no longer read.
60+
*
61+
* Deprecated in 2.15.
6062
*
6163
* @since 2.10
64+
* @deprecated no longer used, lookups are only used when {@code %m{lookups}} is specified
6265
*/
66+
@Deprecated
6367
public static final boolean FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS = PropertiesUtil.getProperties().getBooleanProperty(
64-
"log4j2.formatMsgNoLookups", false);
68+
"log4j2.formatMsgNoLookups", true);
6569

6670
/**
6771
* {@code true} if we think we are running in a web container, based on the boolean value of system property

‎log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutLookupDateTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
*
3030
* This shows the behavior this user wants to disable.
3131
*/
32-
@LoggerContextSource("log4j-list.xml")
32+
@LoggerContextSource("log4j-list-lookups.xml")
3333
public class PatternLayoutLookupDateTest {
3434

3535
@Test

‎log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutNoLookupDateTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
/**
2727
* See (LOG4J2-905) Ability to disable (date) lookup completely, compatibility issues with other libraries like camel.
2828
*/
29-
@LoggerContextSource("log4j-list-nolookups.xml")
29+
@LoggerContextSource("log4j-list.xml")
3030
public class PatternLayoutNoLookupDateTest {
3131

3232
@Test

‎log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MessagePatternConverterTest.java‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,7 @@ public void testPatternAndParameterizedMessageDateLookup() {
7676
}
7777

7878
@Test
79-
public void testLookupEnabledByDefault() {
80-
assertFalse(Constants.FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS, "Expected lookups to be enabled");
81-
}
82-
83-
@Test
84-
public void testLookup() {
79+
public void testDefaultDisabledLookup() {
8580
final Configuration config = new DefaultConfigurationBuilder()
8681
.addProperty("foo", "bar")
8782
.build(true);
@@ -93,7 +88,7 @@ public void testLookup() {
9388
.setMessage(msg).build();
9489
final StringBuilder sb = new StringBuilder();
9590
converter.format(event, sb);
96-
assertEquals("bar", sb.toString(), "Unexpected result");
91+
assertEquals("${foo}", sb.toString(), "Unexpected result");
9792
}
9893

9994
@Test
@@ -113,6 +108,23 @@ public void testDisabledLookup() {
113108
assertEquals("${foo}", sb.toString(), "Expected the raw pattern string without lookup");
114109
}
115110

111+
@Test
112+
public void testLookup() {
113+
final Configuration config = new DefaultConfigurationBuilder()
114+
.addProperty("foo", "bar")
115+
.build(true);
116+
final MessagePatternConverter converter =
117+
MessagePatternConverter.newInstance(config, new String[] {"lookups"});
118+
final Message msg = new ParameterizedMessage("${foo}");
119+
final LogEvent event = Log4jLogEvent.newBuilder() //
120+
.setLoggerName("MyLogger") //
121+
.setLevel(Level.DEBUG) //
122+
.setMessage(msg).build();
123+
final StringBuilder sb = new StringBuilder();
124+
converter.format(event, sb);
125+
assertEquals("bar", sb.toString(), "Unexpected result");
126+
}
127+
116128
@Test
117129
public void testPatternWithConfiguration() {
118130
final Configuration config = new DefaultConfiguration();

‎log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/RegexReplacementTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,13 @@ public void testReplacement() {
5555
assertEquals(1, msgs.size(), "Incorrect number of messages. Should be 1 is " + msgs.size());
5656
assertTrue(
5757
msgs.get(0).endsWith(EXPECTED), "Replacement failed - expected ending " + EXPECTED + " Actual " + msgs.get(0));
58-
app.clear();
58+
}
59+
60+
@Test
61+
public void testMessageReplacement() {
5962
ThreadContext.put("MyKey", "Apache");
6063
logger.error("This is a test for ${ctx:MyKey}");
61-
msgs = app.getMessages();
64+
List<String> msgs = app.getMessages();
6265
assertNotNull(msgs);
6366
assertEquals(1, msgs.size(), "Incorrect number of messages. Should be 1 is " + msgs.size());
6467
assertEquals("LoggerTest This is a test for Apache" + Strings.LINE_SEPARATOR, msgs.get(0));

‎log4j-core/src/test/resources/log4j-list-lookups.xml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to You under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration status="WARN">
19+
<Appenders>
20+
<List name="List">
21+
<PatternLayout pattern="[%-5level] %c{1.} %msg{ansi}{lookups}%n" />
22+
</List>
23+
</Appenders>
24+
<Loggers>
25+
<Root level="debug">
26+
<AppenderRef ref="List" />
27+
</Root>
28+
</Loggers>
29+
</Configuration>

‎log4j-core/src/test/resources/log4j-replace.xml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
<Configuration status="OFF" name="RegexReplacementTest">
2020
<Appenders>
2121
<List name="List">
22-
<PatternLayout>
23-
<replace regex="\." replacement="/"/>
24-
<Pattern>%logger %msg%n</Pattern>
22+
<PatternLayout>
23+
<replace regex="\." replacement="/"/>
24+
<Pattern>%logger %msg{lookups}%n</Pattern>
2525
</PatternLayout>
2626
</List>
2727
<List name="List2">
28-
<PatternLayout>
29-
<Pattern>%replace{%logger %C{1.} %msg%n}{\.}{/}</Pattern>
28+
<PatternLayout>
29+
<Pattern>%replace{%logger %C{1.} %msg{lookups}%n}{\.}{/}</Pattern>
3030
</PatternLayout>
3131
</List>
3232
</Appenders>

‎src/changes/changes.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,12 @@
170170
</release>
171171
<release version="2.15.0" date="2021-MM-DD" description="GA Release 2.15.0">
172172
<!-- ADDS -->
173+
<action issue="LOG4J2-3198" dev="ckozak" type="add">
174+
Pattern layout no longer enables lookups within message text by default for cleaner API boundaries and reduced
175+
formatting overhead. The old 'log4j2.formatMsgNoLookups' which enabled this behavior has been removed as well
176+
as the 'nolookups' message pattern converter option. The old behavior can be enabled on a per-pattern basis
177+
using '%m{lookups}'.
178+
</action>
173179
<action issue="LOG4J2-3194" dev="rgoers" type="add" due-to="markuss">
174180
Allow fractional attributes for size attribute of SizeBsaedTriggeringPolicy.
175181
</action>

0 commit comments

Comments
 (0)