LOG4J2-3211 - Remove Messge Lookups (#623)

* Remove Messge Lookups

* Log a message that the option is no longer supported

* LOG4J2-3211 - Log a message. Update doc

* Add changes.xml entry. Don't limit visibility of LOGGER

Author: rgoers

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

@@ -20,6 +20,7 @@
 import java.util.List;
 import java.util.Locale;
 
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
@@ -38,25 +39,14 @@
 @ConverterKeys({ "m", "msg", "message" })
 @PerformanceSensitive("allocation")
 public class MessagePatternConverter extends LogEventPatternConverter {
-
+    
     private static final String LOOKUPS = "lookups";
     private static final String NOLOOKUPS = "nolookups";
 
     private MessagePatternConverter() {
         super("Message", "message");
     }
 
-    private static boolean loadLookups(final String[] options) {
-        if (options != null) {
-            for (final String option : options) {
-                if (LOOKUPS.equalsIgnoreCase(option)) {
-                    return true;
-                }
-            }
-        }
-        return false;
-    }
-
     private static TextRenderer loadMessageRenderer(final String[] options) {
         if (options != null) {
             for (final String option : options) {
@@ -86,15 +76,11 @@ private static TextRenderer loadMessageRenderer(final String[] options) {
      * @return instance of pattern converter.
      */
     public static MessagePatternConverter newInstance(final Configuration config, final String[] 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);
         }
@@ -107,7 +93,9 @@ private static String[] withoutLookupOptions(final String[] options) {
         }
         List<String> results = new ArrayList<>(options.length);
         for (String option : options) {
-            if (!LOOKUPS.equalsIgnoreCase(option) && !NOLOOKUPS.equalsIgnoreCase(option)) {
+            if (LOOKUPS.equalsIgnoreCase(option) || NOLOOKUPS.equalsIgnoreCase(option)) {
+                LOGGER.info("The {} option will be ignored. Message Lookups are no longer supported.", option);
+            } else {
                 results.add(option);
             }
         }
@@ -164,30 +152,6 @@ public void format(final LogEvent event, final StringBuilder toAppendTo) {
         }
     }
 
-    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;

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

@@ -22,7 +22,7 @@
 import org.apache.logging.log4j.test.appender.ListAppender;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * See (LOG4J2-905) Ability to disable (date) lookup completely, compatibility issues with other libraries like camel.
@@ -38,7 +38,7 @@ public void testDateLookupInMessage(final LoggerContext context, @Named("List")
         final String template = "${date:YYYY-MM-dd}";
         context.getLogger(PatternLayoutLookupDateTest.class.getName()).info(template);
         final String string = listAppender.getMessages().get(0);
-        assertFalse(string.contains(template), string);
+        assertTrue(string.contains(template), string);
     }
 
 }

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

@@ -121,7 +121,7 @@ public void testLookup() {
                 .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

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

@@ -67,7 +67,7 @@ public void testMessageReplacement() {
         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));
+        assertEquals("LoggerTest This is a test for ${ctx:MyKey}" + Strings.LINE_SEPARATOR, msgs.get(0));
     }
 
     @Test

src/changes/changes.xml

@@ -29,10 +29,13 @@
          - "update" - Change
          - "remove" - Removed
     -->
-    <release version="2.15.1" date="2021-12-11" description="GA Release 2.15.1">
+    <release version="2.16.0" date="2021-12-13" description="GA Release 2.16.0">
       <action issue="LOG4J2-3208" dev="rgoers" type="fix">
         Disable JNDI by default. Require log4j2.enableJndi to be set to true to allow JNDI.
       </action>
+      <action issue="LOG4J2-3211" dev="rgoers" type="fix">
+        Completely remove support for Message Lookups.
+      </action>
     </release>
     <release version="2.15.0" date="2021-12-06" description="GA Release 2.15.0">
       <!-- ADDS -->

src/site/xdoc/manual/layouts.xml.vm

@@ -1460,14 +1460,19 @@ WARN  [main]: Message 2</pre>
             <tr>
               <td align="center">
                 <a name="PatternMessage"/>
-                <b>m</b>{lookups}{ansi}<br />
-                <b>msg</b>{lookups}{ansi}<br />
-                <b>message</b>{lookups}{ansi}
+                <b>m</b>{ansi}<br />
+                <b>msg</b>{ansi}<br />
+                <b>message</b>{ansi}
               </td>
               <td>
                 <p>
                   Outputs the application supplied message associated with the logging event.
                 </p>
+                <p>
+                  From Log4j 2.16.0, support for lookups in log messages has been removed for security reasons.
+                  Both the<code>{lookups}</code> and the <code>{nolookups}</code> options on the %m, %msg and %message
+                  pattern are now ignored. If either is specified a message will be logged.
+                </p>
                 <!-- Copied and tweaked from Javadoc for org.apache.logging.log4j.core.pattern.JAnsiMessageRenderer -->
                 <p>
                   Add <code>{ansi}</code> to render messages with ANSI escape codes (requires JAnsi,
@@ -1497,18 +1502,6 @@ WARN  [main]: Message 2</pre>
                   The call site can look like this:
                 </p>
                 <pre class="prettyprint linenums">logger.info("@|KeyStyle {}|@ = @|ValueStyle {}|@", entry.getKey(), entry.getValue());</pre>
-                <p>
-                  Use <code>{lookups}</code> to log messages like <code>logger.info("Try ${esc.d}{date:YYYY-MM-dd}")</code>
-                  using lookups, this will replace the date template <code>${esc.d}{date:YYYY-MM-dd}</code>
-                  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.
-                </p>
-                <p>
-                  <b>Note: </b>Users are <b>STRONGLY</b> discouraged from using the lookups option. Doing so may allow uncontrolled user input
-                  containing lookups to take unintended actions. In almost all cases the software developer can accomplish the same tasks
-                  lookups perform directly in the application code.
-                </p>
               </td>
             </tr>
             <tr>