This is an automated email from the ASF dual-hosted git repository.
rgoers pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push:
new 04637dd LOG4J2-3198 - Log4j2 no longer formats lookups in messages by
default
04637dd is described below
commit 04637dd9102175f765cfad349de0c2a63c279ac3
Author: Ralph Goers <[email protected]>
AuthorDate: Sun Dec 5 00:20:56 2021 -0700
LOG4J2-3198 - Log4j2 no longer formats lookups in messages by default
---
.../core/pattern/MessagePatternConverter.java | 178 ++++++++++++++-------
.../apache/logging/log4j/core/util/Constants.java | 10 +-
.../core/layout/PatternLayoutLookupDateTest.java | 2 +-
.../core/layout/PatternLayoutNoLookupDateTest.java | 2 +-
.../core/pattern/MessagePatternConverterTest.java | 26 ++-
.../log4j/core/pattern/RegexReplacementTest.java | 7 +-
.../src/test/resources/log4j-list-lookups.xml | 29 ++++
log4j-core/src/test/resources/log4j-replace.xml | 10 +-
src/changes/changes.xml | 6 +
src/site/asciidoc/manual/configuration.adoc | 11 +-
src/site/asciidoc/manual/layouts.adoc | 16 +-
11 files changed, 205 insertions(+), 92 deletions(-)
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
index 4372afe..0e625af 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
@@ -16,6 +16,8 @@
*/
package org.apache.logging.log4j.core.pattern;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Locale;
import org.apache.logging.log4j.core.LogEvent;
@@ -37,43 +39,27 @@ import
org.apache.logging.log4j.util.StringBuilderFormattable;
@Plugin(name = "MessagePatternConverter", category = PatternConverter.CATEGORY)
@ConverterKeys({ "m", "msg", "message" })
@PerformanceSensitive("allocation")
-public final class MessagePatternConverter extends LogEventPatternConverter {
+public class MessagePatternConverter extends LogEventPatternConverter {
+ private static final String LOOKUPS = "lookups";
private static final String NOLOOKUPS = "nolookups";
- private final String[] formats;
- private final Configuration config;
- private final TextRenderer textRenderer;
- private final boolean noLookups;
-
- /**
- * Private constructor.
- *
- * @param options
- * options, may be null.
- */
- private MessagePatternConverter(final Configuration config, final String[]
options) {
+ private MessagePatternConverter() {
super("Message", "message");
- this.formats = options;
- this.config = config;
- final int noLookupsIdx = loadNoLookups(options);
- this.noLookups = Constants.FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS ||
noLookupsIdx >= 0;
- this.textRenderer = loadMessageRenderer(noLookupsIdx >= 0 ?
ArrayUtils.remove(options, noLookupsIdx) : options);
}
- private int loadNoLookups(final String[] options) {
+ private static boolean loadLookups(final String[] options) {
if (options != null) {
- for (int i = 0; i < options.length; i++) {
- final String option = options[i];
- if (NOLOOKUPS.equalsIgnoreCase(option)) {
- return i;
+ for (final String option : options) {
+ if (LOOKUPS.equalsIgnoreCase(option)) {
+ return true;
}
}
}
- return -1;
+ return false;
}
- private TextRenderer loadMessageRenderer(final String[] options) {
+ private static TextRenderer loadMessageRenderer(final String[] options) {
if (options != null) {
for (final String option : options) {
switch (option.toUpperCase(Locale.ROOT)) {
@@ -102,7 +88,32 @@ public final class MessagePatternConverter extends
LogEventPatternConverter {
* @return instance of pattern converter.
*/
public static MessagePatternConverter newInstance(final Configuration
config, final String[] options) {
- return new MessagePatternConverter(config, options);
+ boolean lookups = loadLookups(options);
+ String[] formats = withoutLookupOptions(options);
+ TextRenderer textRenderer = loadMessageRenderer(formats);
+ MessagePatternConverter result = formats == null || formats.length == 0
+ ? SimpleMessagePatternConverter.INSTANCE
+ : new FormattedMessagePatternConverter(formats);
+ if (lookups && config != null) {
+ result = new LookupMessagePatternConverter(result, config);
+ }
+ if (textRenderer != null) {
+ result = new RenderingPatternConverter(result, textRenderer);
+ }
+ return result;
+ }
+
+ private static String[] withoutLookupOptions(final String[] options) {
+ if (options == null || options.length == 0) {
+ return options;
+ }
+ List<String> results = new ArrayList<>(options.length);
+ for (String option : options) {
+ if (!LOOKUPS.equalsIgnoreCase(option) &&
!NOLOOKUPS.equalsIgnoreCase(option)) {
+ results.add(option);
+ }
+ }
+ return results.toArray(new String[0]);
}
/**
@@ -110,47 +121,96 @@ public final class MessagePatternConverter extends
LogEventPatternConverter {
*/
@Override
public void format(final LogEvent event, final StringBuilder toAppendTo) {
- final Message msg = event.getMessage();
- if (msg instanceof StringBuilderFormattable) {
+ throw new UnsupportedOperationException();
+ }
- final boolean doRender = textRenderer != null;
- final StringBuilder workingBuilder = doRender ? new
StringBuilder(80) : toAppendTo;
+ private static final class SimpleMessagePatternConverter extends
MessagePatternConverter {
+ private static final MessagePatternConverter INSTANCE = new
SimpleMessagePatternConverter();
- final int offset = workingBuilder.length();
- if (msg instanceof MultiFormatStringBuilderFormattable) {
- ((MultiFormatStringBuilderFormattable) msg).formatTo(formats,
workingBuilder);
- } else {
- ((StringBuilderFormattable) msg).formatTo(workingBuilder);
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ Message msg = event.getMessage();
+ if (msg instanceof StringBuilderFormattable) {
+ ((StringBuilderFormattable) msg).formatTo(toAppendTo);
+ } else if (msg != null) {
+ toAppendTo.append(msg.getFormattedMessage());
}
+ }
+ }
- // TODO can we optimize this?
- if (config != null && !noLookups) {
- for (int i = offset; i < workingBuilder.length() - 1; i++) {
- if (workingBuilder.charAt(i) == '$' &&
workingBuilder.charAt(i + 1) == '{') {
- final String value = workingBuilder.substring(offset,
workingBuilder.length());
- workingBuilder.setLength(offset);
-
workingBuilder.append(config.getStrSubstitutor().replace(event, value));
- }
+ private static final class FormattedMessagePatternConverter extends
MessagePatternConverter {
+
+ private final String[] formats;
+
+ FormattedMessagePatternConverter(final String[] formats) {
+ this.formats = formats;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ Message msg = event.getMessage();
+ if (msg instanceof StringBuilderFormattable) {
+ if (msg instanceof MultiFormatStringBuilderFormattable) {
+ ((MultiFormatStringBuilderFormattable)
msg).formatTo(formats, toAppendTo);
+ } else {
+ ((StringBuilderFormattable) msg).formatTo(toAppendTo);
}
+ } else if (msg != null) {
+ toAppendTo.append(msg instanceof MultiformatMessage
+ ? ((MultiformatMessage)
msg).getFormattedMessage(formats)
+ : msg.getFormattedMessage());
}
- if (doRender) {
- textRenderer.render(workingBuilder, toAppendTo);
- }
- return;
}
- if (msg != null) {
- final String result;
- if (msg instanceof MultiformatMessage) {
- result = ((MultiformatMessage)
msg).getFormattedMessage(formats);
- } else {
- result = msg.getFormattedMessage();
- }
- if (result != null) {
- toAppendTo.append(config != null && result.contains("${")
- ? config.getStrSubstitutor().replace(event, result) :
result);
- } else {
- toAppendTo.append("null");
+ }
+
+ private static final class LookupMessagePatternConverter extends
MessagePatternConverter {
+ private final MessagePatternConverter delegate;
+ private final Configuration config;
+
+ LookupMessagePatternConverter(final MessagePatternConverter delegate,
final Configuration config) {
+ this.delegate = delegate;
+ this.config = config;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ int start = toAppendTo.length();
+ delegate.format(event, toAppendTo);
+ int indexOfSubstitution = toAppendTo.indexOf("${", start);
+ if (indexOfSubstitution >= 0) {
+ config.getStrSubstitutor()
+ .replaceIn(event, toAppendTo, indexOfSubstitution,
toAppendTo.length() - indexOfSubstitution);
}
}
}
+
+ private static final class RenderingPatternConverter extends
MessagePatternConverter {
+
+ private final MessagePatternConverter delegate;
+ private final TextRenderer textRenderer;
+
+ RenderingPatternConverter(final MessagePatternConverter delegate,
final TextRenderer textRenderer) {
+ this.delegate = delegate;
+ this.textRenderer = textRenderer;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ StringBuilder workingBuilder = new StringBuilder(80);
+ delegate.format(event, workingBuilder);
+ textRenderer.render(workingBuilder, toAppendTo);
+ }
+ }
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Constants.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Constants.java
index fa01c13..9609160 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Constants.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Constants.java
@@ -55,13 +55,17 @@ public final class Constants {
"log4j.format.msg.async", false);
/**
- * LOG4J2-2109 if {@code true}, MessagePatternConverter will always
operate as though
- * <pre>%m{nolookups}</pre> is configured.
+ * LOG4J2-3198 property which used to globally opt out of lookups in
pattern layout message text, however
+ * this is the default and this property is no longer read.
+ *
+ * Deprecated in 2.15.
*
* @since 2.10
+ * @deprecated no longer used, lookups are only used when {@code
%m{lookups}} is specified
*/
+ @Deprecated
public static final boolean FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS =
PropertiesUtil.getProperties().getBooleanProperty(
- "log4j2.formatMsgNoLookups", false);
+ "log4j2.formatMsgNoLookups", true);
/**
* {@code true} if we think we are running in a web container, based on
the boolean value of system property
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutLookupDateTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutLookupDateTest.java
index c1ea78f..9b2d616 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutLookupDateTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutLookupDateTest.java
@@ -29,7 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
*
* This shows the behavior this user wants to disable.
*/
-@LoggerContextSource("log4j-list.xml")
+@LoggerContextSource("log4j-list-lookups.xml")
public class PatternLayoutLookupDateTest {
@Test
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutNoLookupDateTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutNoLookupDateTest.java
index 5949cc8..e544f6c 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutNoLookupDateTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutNoLookupDateTest.java
@@ -26,7 +26,7 @@ import org.junit.jupiter.api.Test;
/**
* See (LOG4J2-905) Ability to disable (date) lookup completely, compatibility
issues with other libraries like camel.
*/
-@LoggerContextSource("log4j-list-nolookups.xml")
+@LoggerContextSource("log4j-list.xml")
public class PatternLayoutNoLookupDateTest {
@Test
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MessagePatternConverterTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MessagePatternConverterTest.java
index fe8cdfe..e16baa9 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MessagePatternConverterTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MessagePatternConverterTest.java
@@ -76,12 +76,7 @@ public class MessagePatternConverterTest {
}
@Test
- public void testLookupEnabledByDefault() {
- assertFalse(Constants.FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS,
"Expected lookups to be enabled");
- }
-
- @Test
- public void testLookup() {
+ public void testDefaultDisabledLookup() {
final Configuration config = new DefaultConfigurationBuilder()
.addProperty("foo", "bar")
.build(true);
@@ -93,7 +88,7 @@ public class MessagePatternConverterTest {
.setMessage(msg).build();
final StringBuilder sb = new StringBuilder();
converter.format(event, sb);
- assertEquals("bar", sb.toString(), "Unexpected result");
+ assertEquals("${foo}", sb.toString(), "Unexpected result");
}
@Test
@@ -114,6 +109,23 @@ public class MessagePatternConverterTest {
}
@Test
+ public void testLookup() {
+ final Configuration config = new DefaultConfigurationBuilder()
+ .addProperty("foo", "bar")
+ .build(true);
+ final MessagePatternConverter converter =
+ MessagePatternConverter.newInstance(config, new String[]
{"lookups"});
+ final Message msg = new ParameterizedMessage("${foo}");
+ final LogEvent event = Log4jLogEvent.newBuilder() //
+ .setLoggerName("MyLogger") //
+ .setLevel(Level.DEBUG) //
+ .setMessage(msg).build();
+ final StringBuilder sb = new StringBuilder();
+ converter.format(event, sb);
+ assertEquals("bar", sb.toString(), "Unexpected result");
+ }
+
+ @Test
public void testPatternWithConfiguration() {
final Configuration config = new DefaultConfiguration();
final MessagePatternConverter converter =
MessagePatternConverter.newInstance(config, null);
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/RegexReplacementTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/RegexReplacementTest.java
index 9a2273a..20d6904 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/RegexReplacementTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/RegexReplacementTest.java
@@ -55,10 +55,13 @@ public class RegexReplacementTest {
assertEquals(1, msgs.size(), "Incorrect number of messages. Should be
1 is " + msgs.size());
assertTrue(
msgs.get(0).endsWith(EXPECTED), "Replacement failed - expected
ending " + EXPECTED + " Actual " + msgs.get(0));
- app.clear();
+ }
+
+ @Test
+ public void testMessageReplacement() {
ThreadContext.put("MyKey", "Apache");
logger.error("This is a test for ${ctx:MyKey}");
- msgs = app.getMessages();
+ List<String> msgs = app.getMessages();
assertNotNull(msgs);
assertEquals(1, msgs.size(), "Incorrect number of messages. Should be
1 is " + msgs.size());
assertEquals("LoggerTest This is a test for Apache" +
Strings.LINE_SEPARATOR, msgs.get(0));
diff --git a/log4j-core/src/test/resources/log4j-list-lookups.xml
b/log4j-core/src/test/resources/log4j-list-lookups.xml
new file mode 100644
index 0000000..f8df499
--- /dev/null
+++ b/log4j-core/src/test/resources/log4j-list-lookups.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+<Configuration status="WARN">
+ <Appenders>
+ <List name="List">
+ <PatternLayout pattern="[%-5level] %c{1.} %msg{ansi}{lookups}%n" />
+ </List>
+ </Appenders>
+ <Loggers>
+ <Root level="debug">
+ <AppenderRef ref="List" />
+ </Root>
+ </Loggers>
+</Configuration>
\ No newline at end of file
diff --git a/log4j-core/src/test/resources/log4j-replace.xml
b/log4j-core/src/test/resources/log4j-replace.xml
index 09ff3b1..684cd7e 100644
--- a/log4j-core/src/test/resources/log4j-replace.xml
+++ b/log4j-core/src/test/resources/log4j-replace.xml
@@ -19,14 +19,14 @@
<Configuration status="OFF" name="RegexReplacementTest">
<Appenders>
<List name="List">
- <PatternLayout>
- <replace regex="\." replacement="/"/>
- <Pattern>%logger %msg%n</Pattern>
+ <PatternLayout>
+ <replace regex="\." replacement="/"/>
+ <Pattern>%logger %msg{lookups}%n</Pattern>
</PatternLayout>
</List>
<List name="List2">
- <PatternLayout>
- <Pattern>%replace{%logger %C{1.} %msg%n}{\.}{/}</Pattern>
+ <PatternLayout>
+ <Pattern>%replace{%logger %C{1.} %msg{lookups}%n}{\.}{/}</Pattern>
</PatternLayout>
</List>
</Appenders>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 4090173..fc740b8 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -170,6 +170,12 @@
</release>
<release version="2.15.0" date="2021-MM-DD" description="GA Release
2.15.0">
<!-- ADDS -->
+ <action issue="LOG4J2-3198" dev="ckozak" type="add">
+ Pattern layout no longer enables lookups within message text by
default for cleaner API boundaries and reduced
+ formatting overhead. The old 'log4j2.formatMsgNoLookups' which enabled
this behavior has been removed as well
+ as the 'nolookups' message pattern converter option. The old behavior
can be enabled on a per-pattern basis
+ using '%m{lookups}'.
+ </action>
<action issue="LOG4J2-3194" dev="rgoers" type="add" due-to="markuss">
Allow fractional attributes for size attribute of
SizeBsaedTriggeringPolicy.
</action>
diff --git a/src/site/asciidoc/manual/configuration.adoc
b/src/site/asciidoc/manual/configuration.adoc
index df17351..a03ba97 100644
--- a/src/site/asciidoc/manual/configuration.adoc
+++ b/src/site/asciidoc/manual/configuration.adoc
@@ -1312,12 +1312,11 @@ included, such as ``${main:\--file:-app.properties}`.
This would use the
`MainMapLookup` for a key named `--file`. If the key is not found then
<code>app.properties</code> would be used as the default value.
-[#DisablingMessagePatternLookups]
-== Disables Message Pattern Lookups
-A message is processed (by default) by lookups, for example if you defined
-`<Property name="foo.bar">FOO_BAR</Property>`, then
`logger.info("${foo.bar}")` will output `FOO_BAR` instead of `${foo.bar}`.
-You could disable message pattern lookups globally by setting system property
`log4j2.formatMsgNoLookups` to true,
-or defining message pattern using %m{nolookups}.
+[#EnablingMessagePatternLookups]
+== Enabling Message Pattern Lookups
+A message is processed (by default) without using lookups, for example if you
defined
+`<Property name="foo.bar">FOO_BAR</Property>`, then
`logger.info("${foo.bar}")` will output `${foo.bar}` instead of `FOO_BAR`.
+You could enable message pattern lookups by defining the message pattern using
%m{lookups}.
[#RuntimeLookup]
== Lookup Variables with Multiple Leading '$' Characters
diff --git a/src/site/asciidoc/manual/layouts.adoc
b/src/site/asciidoc/manual/layouts.adoc
index 1f12eca..f986c23 100644
--- a/src/site/asciidoc/manual/layouts.adoc
+++ b/src/site/asciidoc/manual/layouts.adoc
@@ -1234,9 +1234,9 @@ Generating line number information
(link:#LocationInformation[location
information]) is an expensive operation and may impact performance. Use
with caution.
-|[[PatternMessage]] *m*{nolookups}{ansi} +
-*msg*{nolookups}{ansi} +
-*message*{nolookups}{ansi}
+|[[PatternMessage]] *m*{lookups}{ansi} +
+*msg*{lookups}{ansi} +
+*message{lookups}{ansi}
|Outputs the application supplied message associated with the logging
event.
@@ -1280,11 +1280,11 @@ The call site can look like this:
logger.info("@\|KeyStyle {}\|@ = @\|ValueStyle {}\|@", entry.getKey(),
entry.getValue());
....
-Use `{nolookups}` to log messages like `"${date:YYYY-MM-dd}"` without
-using any lookups. Normally calling
-`logger.info("Try ${date:YYYY-MM-dd}")` would replace the date template
-`${date:YYYY-MM-dd}` with an actual date. Using `nolookups` disables
-this feature and logs the message string untouched.
+Use `{lookups}` to log messages like `"${date:YYYY-MM-dd}"` using lookups.
+using any lookups. This will replace the date template `{date:YYYY-MM-dd}`
+with an actual date. This can be confusing in many cases, and it's often both
easier and
+more obvious to handle the lookup in code.
+This feature is disabled by default and the message string is logged untouched.
|[[PatternMethod]] *M* +
*method*
