[1223] Change FormattedMessage pattern heuristic

We change the order in which `FormattedMessage` checks the format of the
provided pattern: we first check for the presence of `{}` placeholders
and only then for `java.util.Format` specifiers.

This eliminates the need for a potentially exponential regular
expression evalutation, which was reported by Spotbugs (#1849).

The Javadoc and documentation were improved to clarify the heuristic
used by `FormattedMessage`.

Closes #1223.

Remark: since `FormattedMessage` used the **same** regular expression as
`java.util.Format`, if a message uses `java.util.Format` specifiers, it
is still vulnerable to a ReDOS.

Author: ppkarwasz

log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java

@@ -23,7 +23,6 @@
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.Locale;
-import java.util.regex.Pattern;
 
 /**
  * Handles messages that contain a format String. Dynamically determines if the format conforms to
@@ -33,8 +32,6 @@ public class FormattedMessage implements Message {
 
     private static final long serialVersionUID = -665975803997290697L;
     private static final int HASHVAL = 31;
-    private static final String FORMAT_SPECIFIER = "%(\\d+\\$)?([-#+ 0,(\\<]*)?(\\d+)?(\\.\\d+)?([tT])?([a-zA-Z%])";
-    private static final Pattern MSG_PATTERN = Pattern.compile(FORMAT_SPECIFIER);
 
     private String messagePattern;
     private transient Object[] argArray;
@@ -180,24 +177,44 @@ public String getFormattedMessage() {
         return formattedMessage;
     }
 
+    /**
+     * Gets the message implementation to which formatting is delegated.
+     *
+     * <ul>
+     *     <li>if {@code msgPattern} contains {@link MessageFormat} format specifiers a {@link MessageFormatMessage}
+     * is returned,</li>
+     *     <li>if {@code msgPattern} contains {@code {}} placeholders a {@link ParameterizedMessage} is returned,</li>
+     *     <li>if {@code msgPattern} contains {@link Format} specifiers a {@link StringFormattedMessage} is returned
+     *    .</li>
+     * </ul>
+     * <p>
+     *     Mixing specifiers from multiple types is not supported.
+     * </p>
+     *
+     * @param msgPattern The message pattern.
+     * @param args       The parameters.
+     * @param aThrowable The throwable
+     * @return The message that performs formatting.
+     */
     protected Message getMessage(final String msgPattern, final Object[] args, final Throwable aThrowable) {
+        // Check for valid `{ ArgumentIndex [, FormatType [, FormatStyle]] }` format specifiers
         try {
             final MessageFormat format = new MessageFormat(msgPattern);
             final Format[] formats = format.getFormats();
-            if (formats != null && formats.length > 0) {
+            if (formats.length > 0) {
                 return new MessageFormatMessage(locale, msgPattern, args);
             }
         } catch (final Exception ignored) {
             // Obviously, the message is not a proper pattern for MessageFormat.
         }
-        try {
-            if (MSG_PATTERN.matcher(msgPattern).find()) {
-                return new StringFormattedMessage(locale, msgPattern, args);
-            }
-        } catch (final Exception ignored) {
-            // Also not properly formatted.
+        // Check for non-escaped `{}` format specifiers
+        // This case also includes patterns without any `java.util.Formatter` specifiers
+        if (ParameterFormatter.analyzePattern(msgPattern, 1).placeholderCount > 0
+                || msgPattern.indexOf('%') == -1) {
+            return new ParameterizedMessage(msgPattern, args, aThrowable);
         }
-        return new ParameterizedMessage(msgPattern, args, aThrowable);
+        // Interpret as `java.util.Formatter` format
+        return new StringFormattedMessage(locale, msgPattern, args);
     }
 
     /**

log4j-api/src/main/java/org/apache/logging/log4j/message/package-info.java

@@ -19,7 +19,10 @@
  * Public Message Types used for Log4j 2. Users may implement their own Messages.
  */
 @Export
-@Version("2.20.1")
+/**
+ * Bumped to 2.21.0, since FormattedMessage behavior changed.
+ */
+@Version("2.21.0")
 package org.apache.logging.log4j.message;
 
 import org.osgi.annotation.bundle.Export;

src/changelog/.2.x.x/1223_change_formatted_message_heuristic.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="http://logging.apache.org/log4j/changelog"
+       xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
+       type="changed">
+  <issue id="1223" link="https://github.com/apache/logging-log4j2/issues/1223"/>
+  <description format="asciidoc">
+    Change the order of evaluation of `FormattedMessage` formatters.
+    Messages are evaluated using `java.util.Format` only if they don't comply to the `java.text.MessageFormat` or `ParameterizedMessage` format.
+  </description>
+</entry>

src/site/xdoc/manual/messages.xml

@@ -168,12 +168,31 @@ public class MyApp {
       <h4>FormattedMessage</h4>
         <a name="FormattedMessage"/>
         <p>
-          The message pattern passed to a
-          <a class="javadoc" href="../log4j-api/apidocs/org/apache/logging/log4j/message/FormattedMessage.html">FormattedMessage</a>
-          is first checked to see if it is a valid java.text.MessageFormat pattern. If it is, a MessageFormatMessage is
-          used to format it. If not it is next checked to see if it contains any tokens that are valid format
-          specifiers for String.format(). If so, a StringFormattedMessage is used to format it. Finally, if the
-          pattern doesn't match either of those then a ParameterizedMessage is used to format it.
+          <a class="javadoc" href="../log4j-api/apidocs/org/apache/logging/log4j/message/FormattedMessage">FormattedMessage</a>
+          is a message that supports multiple pattern formatters:
+          <ul>
+            <li>
+              if the pattern is a valid
+              <a class="javadoc" href="https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/text/MessageFormat.html">java.text.MessageFormat</a>
+              pattern, then
+              <a class="javadoc" href="https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/message/MessageFormatMessage.html">MessageFormatMessage</a>
+              is used to format it,
+            </li>
+            <li>
+              if the pattern contains valid unescaped <code>{}</code> specifiers, then
+              <a class="javadoc" href="https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/message/ParameterizedMessage">ParameterizedMessage</a>
+              is used to format it,
+            </li>
+            <li>
+              if the pattern is a valid
+              <a class="javadoc" href="https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Formatter.html">java.uti.Formatter</a>
+              pattern, then
+              <a class="javadoc" href="https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/message/StringFormattedMessage">StringFormattedMessage</a>
+              is used to format it.
+            </li>
+          </ul>
+          Mixing specifiers from multiple formatters is not supported.
+          A pattern without any specifiers will use any of the above formatters.
         </p>
       <h4>LocalizedMessage</h4>
         <a name="LocalizedMessage"/>