Apache Log4j Core: log injection in `Rfc5424Layout` due to silent configuration incompatibility

@@ -29,8 +29,11 @@
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
+import java.util.stream.Stream;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.MarkerManager;
 import org.apache.logging.log4j.ThreadContext;
@@ -57,6 +60,8 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
  
 @UsingAnyThreadContext
@@ -89,6 +94,10 @@ class Rfc5424LayoutTest {
             "[RequestContext@3692 ipAddress=\"192.168.0.120\" loginId=\"JohnDoe\"]";
     private static final String collectionEndOfLine = "Transfer Complete";
  
+    private static final String NEW_LINE_ESCAPE = "\\n";
+    private static final String INCLUDED_KEYS = "key1, key2, locale";
+    private static final String EXCLUDED_KEYS = "key3, key4";
+ 
     static ConfigurationFactory cf = new BasicConfigurationFactory();
  
     @BeforeAll
@@ -792,4 +801,123 @@ void testFQDN() throws UnknownHostException {
         final Rfc5424Layout layout = Rfc5424Layout.newBuilder().build();
         assertThat(layout.getLocalHostName()).isEqualTo(fqdn);
     }
+ 
+    private static Map<String, String> attributeMap(String... keyValuePairs) {
+        Map<String, String> result = new HashMap<>();
+        for (int i = 0; i < keyValuePairs.length; i += 2) {
+            result.put(keyValuePairs[i], keyValuePairs[i + 1]);
+        }
+        return result;
+    }
+ 
+    private static Rfc5424Layout buildRfc5424Layout(Map<String, String> attributes) {
+        PluginManager manager = new PluginManager(Node.CATEGORY);
+        manager.collectPlugins();
+ 
+        Node node = new Node();
+        node.getAttributes().putAll(attributes);
+ 
+        Object object = new PluginBuilder(manager.getPluginType("Rfc5424Layout"))
+                .withConfigurationNode(node)
+                .withConfiguration(new DefaultConfiguration())
+                .build();
+ 
+        assertThat(object).isInstanceOf(Rfc5424Layout.class);
+        return (Rfc5424Layout) object;
+    }
+ 
+    private static Stream<Arguments> testAcceptsDocumentedAttributesAndCompatibilityAliases() {
+        return Stream.of(
+                Arguments.of(
+                        "documented attributes",
+                        attributeMap(
+                                "newLine",
+                                "true",
+                                "newLineEscape",
+                                NEW_LINE_ESCAPE,
+                                "useTlsMessageFormat",
+                                "true",
+                                "mdcRequired",
+                                INCLUDED_KEYS)),
+                Arguments.of(
+                        "compatibility aliases",
+                        attributeMap(
+                                "includeNL",
+                                "true",
+                                "escapeNL",
+                                NEW_LINE_ESCAPE,
+                                "useTLSMessageFormat",
+                                "true",
+                                "required",
+                                INCLUDED_KEYS)));
+    }
+ 
+    @ParameterizedTest
+    @MethodSource
+    void testAcceptsDocumentedAttributesAndCompatibilityAliases(
+            String ignoredDisplayName, Map<String, String> attributes) {
+ 
+        Rfc5424Layout layout = buildRfc5424Layout(attributes);
+ 
+        assertThat(layout.isIncludeNewLine()).isTrue();
+        // The field contains Matcher.quote() escaped value, so we expect the backslash to be escaped.
+        assertThat(layout.getEscapeNewLine()).isEqualTo("\\\\n");
+        assertThat(layout.isUseTlsMessageFormat()).isTrue();
+        assertThat(layout.getMdcRequired()).containsExactly("key1", "key2", "locale");
+    }
+ 
+    private static Stream<Arguments> testAcceptsIncludeAttributesAndCompatibilityAliases() {
+        return Stream.of(
+                Arguments.of("documented attributes", attributeMap("mdcIncludes", INCLUDED_KEYS)),
+                Arguments.of("compatibility aliases", attributeMap("includes", INCLUDED_KEYS)));
+    }
+ 
+    @ParameterizedTest
+    @MethodSource
+    void testAcceptsIncludeAttributesAndCompatibilityAliases(
+            String ignoredDisplayName, Map<String, String> attributes) {
+ 
+        Rfc5424Layout layout = buildRfc5424Layout(attributes);
+ 
+        assertThat(layout.getMdcIncludes()).containsExactly("key1", "key2", "locale");
+        assertThat(layout.getMdcExcludes()).isNullOrEmpty();
+    }
+ 
+    private static Stream<Arguments> testAcceptsExcludeAttributesAndCompatibilityAliases() {
+        return Stream.of(
+                Arguments.of("documented attributes", attributeMap("mdcExcludes", EXCLUDED_KEYS)),
+                Arguments.of("compatibility aliases", attributeMap("excludes", EXCLUDED_KEYS)));
+    }
+ 
+    @ParameterizedTest
+    @MethodSource
+    void testAcceptsExcludeAttributesAndCompatibilityAliases(
+            String ignoredDisplayName, Map<String, String> attributes) {
+ 
+        Rfc5424Layout layout = buildRfc5424Layout(attributes);
+ 
+        assertThat(layout.getMdcExcludes()).containsExactly("key3", "key4");
+        assertThat(layout.getMdcIncludes()).isNullOrEmpty();
+    }
+ 
+    private static Stream<Arguments> testRejectsIncludesAndExcludesTogether() {
+        return Stream.of(
+                Arguments.of(
+                        "documented attributes",
+                        attributeMap("mdcIncludes", INCLUDED_KEYS, "mdcExcludes", EXCLUDED_KEYS)),
+                Arguments.of(
+                        "compatibility aliases", attributeMap("includes", INCLUDED_KEYS, "excludes", EXCLUDED_KEYS)));
+    }
+ 
+    @ParameterizedTest
+    @MethodSource
+    void testRejectsIncludesAndExcludesTogether(String ignoredDisplayName, Map<String, String> attributes) {
+ 
+        Rfc5424Layout layout = buildRfc5424Layout(attributes);
+ 
+        // If both includes and excludes are specified, the layout will ignore the includes and log an error about the
+        // invalid configuration.
+        assertThat(layout.getMdcExcludes()).containsExactly("key3", "key4");
+        assertThat(layout.getMdcIncludes()).isNullOrEmpty();
+    }
 }

@@ -24,6 +24,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.regex.Matcher;
@@ -467,6 +468,26 @@ protected List<String> getMdcIncludes() {
         return mdcIncludes;
     }
  
+    // Test-only
+    List<String> getMdcRequired() {
+        return mdcRequired;
+    }
+ 
+    // Test-only
+    boolean isIncludeNewLine() {
+        return includeNewLine;
+    }
+ 
+    // Test-only
+    String getEscapeNewLine() {
+        return escapeNewLine;
+    }
+ 
+    // Test-only
+    boolean isUseTlsMessageFormat() {
+        return useTlsMessageFormat;
+    }
+ 
     private String computeTimeStampString(final long now) {
         long last;
         synchronized (this) {
@@ -639,7 +660,7 @@ public String toString() {
      * @param loggerFields Container for the KeyValuePairs containing the patterns
      * @param config The Configuration. Some Converters require access to the Interpolator.
      * @return An Rfc5424Layout.
-     * @deprecated Use {@link Rfc5424LayoutBuilder instead}
+     * @deprecated Since 2.21.0 use {@link Rfc5424LayoutBuilder instead}
      */
     @Deprecated
     public static Rfc5424Layout createLayout(
@@ -689,65 +710,208 @@ public static Rfc5424Layout createLayout(
                 .build();
     }
  
+    /**
+     * @since 2.21.0
+     */
     @PluginBuilderFactory
     public static Rfc5424LayoutBuilder newBuilder() {
         return new Rfc5424LayoutBuilder();
     }
  
+    /**
+     * @since 2.21.0
+     */
     public static class Rfc5424LayoutBuilder extends AbstractStringLayout.Builder<Rfc5424LayoutBuilder>
             implements org.apache.logging.log4j.core.util.Builder<Rfc5424Layout> {
  
+        /**
+         * The name of the {@link Facility} as described in RFC 5424
+         *
+         * <p>The matching is case-insensitive. Defaults to {@code LOCAL0}.</p>
+         */
         @PluginBuilderAttribute
         private Facility facility = Facility.LOCAL0;
  
+        /**
+         * The default {@code SD-ID} as described in RFC 5424.
+         */
         @PluginBuilderAttribute
         private String id;
  
+        /**
+         * The enterprise number to include in {@code SD-ID} identifiers.
+         *
+         * <p>Can contain multiple integers separated by a dot, for example {@code 32473.1}</p>
+         *
+         * <p>Defaults to {@value #DEFAULT_ENTERPRISE_NUMBER}.</p>
+         */
         @PluginBuilderAttribute
         private String ein = String.valueOf(DEFAULT_ENTERPRISE_NUMBER);
  
+        /**
+         * The enterprise number to include in {@code SD-ID} identifiers.
+         *
+         * <p>Limited to a single integer.</p>
+         *
+         * <p>Defaults to {@value #DEFAULT_ENTERPRISE_NUMBER}.</p>
+         */
         @PluginBuilderAttribute
         private Integer enterpriseNumber;
  
+        /**
+         * Indicates whether data from the context map will be included as RFC 5424 {@code SD-ELEMENT}.
+         *
+         * <p>Defaults to {@code true}.</p>
+         */
         @PluginBuilderAttribute
         private boolean includeMDC = true;
  
+        /**
+         * If {@code true}, a newline will be appended to the end of the syslog record.
+         *
+         * <p>Default is {@code false}.</p>
+         */
+        @SuppressWarnings("log4j.public.setter")
+        @PluginBuilderAttribute
+        private boolean newLine;
+ 
+        /**
+         * Same as {@code newLine}.
+         *
+         * <p>Erroneously introduced in version 2.21.0, but kept for compatibility.</p>
+         */
         @PluginBuilderAttribute
         private boolean includeNL;
  
+        /**
+         * If set, this string will be used to replace new lines within the message text.
+         *
+         * <p>By default, new lines are not escaped.</p>
+         */
+        @SuppressWarnings("log4j.public.setter")
+        @PluginBuilderAttribute
+        private String newLineEscape;
+ 
+        /**
+         * Same as {@code newLineEscape}.
+         *
+         * <p>Erroneously introduced in version 2.21.0, but kept for compatibility.</p>
+         */
         @PluginBuilderAttribute
         private String escapeNL;
  
+        /**
+         * The id to use for the MDC Structured Data Element.
+         *
+         * <p>Defaults to {@value #DEFAULT_MDCID}.</p>
+         */
         @PluginBuilderAttribute
         private String mdcId = DEFAULT_MDCID;
  
+        /**
+         * A prefix to add to MDC key names when formatting them as structured data parameters.
+         */
         @PluginBuilderAttribute
         private String mdcPrefix;
  
+        /**
+         * A prefix to add to event key names when formatting {@link StructuredDataMessage} fields.
+         */
         @PluginBuilderAttribute
         private String eventPrefix;
  
+        /**
+         * The value to use as the {@code APP-NAME} in the RFC 5424 syslog record.
+         */
         @PluginBuilderAttribute
         private String appName;
  
+        /**
+         * The default value to be used in the {@code MSGID} field of RFC 5424 syslog records.
+         *
+         * <p>If the log event contains a {@link StructuredDataMessage}, the id from that message will be used
+         * instead.</p>
+         */
         @PluginBuilderAttribute
         private String messageId;
  
+        /**
+         * A comma separated list of MDC keys that should be excluded from the LogEvent.
+         *
+         * <p>Mutually exclusive with {@link #mdcIncludes}.</p>
+         */
+        @SuppressWarnings("log4j.public.setter")
+        @PluginBuilderAttribute
+        private String mdcExcludes;
+ 
+        /**
+         * Same as {@code mdcExcludes}.
+         *
+         * <p>Erroneously introduced in version 2.21.0, but kept for compatibility.</p>
+         */
         @PluginBuilderAttribute
         private String excludes;
  
+        /**
+         * A comma separated list of MDC keys that should be included in the LogEvent.
+         *
+         * <p>Mutually exclusive with {@link #mdcExcludes}.</p>
+         */
+        @SuppressWarnings("log4j.public.setter")
+        @PluginBuilderAttribute
+        private String mdcIncludes;
+ 
+        /**
+         * Same as {@code mdcIncludes}.
+         *
+         * <p>Erroneously introduced in version 2.21.0, but kept for compatibility.</p>
+         */
         @PluginBuilderAttribute
         private String includes;
  
+        /**
+         * A comma separated list of MDC keys that must be present in the MDC.
+         */
+        @SuppressWarnings("log4j.public.setter")
+        @PluginBuilderAttribute
+        private String mdcRequired;
+ 
+        /**
+         * Same as {@code mdcRequired}.
+         *
+         * <p>Erroneously introduced in version 2.21.0, but kept for compatibility.</p>
+         */
         @PluginBuilderAttribute
         private String required;
  
+        /**
+         * The pattern used to format exceptions appended to the syslog message.
+         */
         @PluginBuilderAttribute
         private String exceptionPattern;
  
+        /**
+         * If true the message will be formatted according to RFC 5425.
+         *
+         * <p>Default is {@code false}.</p>
+         */
+        @SuppressWarnings("log4j.public.setter")
+        @PluginBuilderAttribute
+        private boolean useTlsMessageFormat;
+ 
+        /**
+         * Same as {@code useTlsMessageFormat}.
+         *
+         * <p>Erroneously introduced in version 2.21.0, but kept for compatibility.</p>
+         */
         @PluginBuilderAttribute
         private boolean useTLSMessageFormat;
  
+        /**
+         * Optional additional {@code SD-ELEMENT}s.
+         *
+         * <p>Each {@link LoggerFields} entry contains a set of key/value patterns to produce structured data parameters.</p>
+         */
         @PluginElement(value = "loggerFields")
         private LoggerFields[] loggerFields;
  
@@ -860,9 +1024,12 @@ public Rfc5424LayoutBuilder setEnterpriseNumber(Integer enterpriseNumber) {
  
         @Override
         public Rfc5424Layout build() {
-            if (includes != null && excludes != null) {
+            String effectiveIncludes = Objects.toString(mdcIncludes, includes);
+            String effectiveExcludes = Objects.toString(mdcExcludes, excludes);
+ 
+            if (effectiveIncludes != null && effectiveExcludes != null) {
                 LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored");
-                includes = null;
+                effectiveIncludes = null;
             }
  
             if (enterpriseNumber != null) {
@@ -880,19 +1047,19 @@ public Rfc5424Layout build() {
                     id,
                     ein,
                     includeMDC,
-                    includeNL,
-                    escapeNL,
+                    newLine || includeNL,
+                    Objects.toString(newLineEscape, escapeNL),
                     mdcId,
                     mdcPrefix,
                     eventPrefix,
                     appName,
                     messageId,
-                    excludes,
-                    includes,
-                    required,
+                    effectiveExcludes,
+                    effectiveIncludes,
+                    Objects.toString(mdcRequired, required),
                     charset != null ? charset : StandardCharsets.UTF_8,
                     exceptionPattern,
-                    useTLSMessageFormat,
+                    useTlsMessageFormat || useTLSMessageFormat,
                     loggerFields);
         }
     }
 src/changelog/.2.x.x/4022_rfc5424-param-names.xml | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 src/changelog/.2.x.x/4022_rfc5424-param-names.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="
+           https://logging.apache.org/xml/ns
+           https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+    <issue id="4022" link="https://github.com/apache/logging-log4j2/issues/4022"/>
+    <issue id="4074" link="https://github.com/apache/logging-log4j2/pull/4074"/>
+    <description format="asciidoc">
+        Restore support for documented `Rfc5424Layout` parameter names.
+    </description>
+</entry>
 .../org/apache/logging/log4j/core/layout/Rfc5424Layout.java     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -1028,7 +1028,7 @@ public Rfc5424Layout build() {
             String effectiveExcludes = Objects.toString(mdcExcludes, excludes);
  
             if (effectiveIncludes != null && effectiveExcludes != null) {
-                LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored");
+                LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes will be ignored");
                 effectiveIncludes = null;
             }
  
 .../log4j/core/layout/Rfc5424LayoutTest.java     | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

@@ -100,16 +100,23 @@ class Rfc5424LayoutTest {
  
     static ConfigurationFactory cf = new BasicConfigurationFactory();
  
+    private static PluginManager pluginManager;
+ 
     @BeforeAll
     static void setupClass() {
         StatusLogger.getLogger().setLevel(Level.OFF);
         ConfigurationFactory.setConfigurationFactory(cf);
         final LoggerContext ctx = LoggerContext.getContext();
         ctx.reconfigure();
+ 
+        pluginManager = new PluginManager(Node.CATEGORY);
+        pluginManager.collectPlugins();
     }
  
     @AfterAll
     static void cleanupClass() {
+        pluginManager = null;
+ 
         ConfigurationFactory.removeConfigurationFactory(cf);
     }
  
@@ -758,9 +765,7 @@ void testLayoutBuilderDefaultValues() {
         final Rfc5424Layout layout = new Rfc5424Layout.Rfc5424LayoutBuilder().build();
         checkDefaultValues(layout);
  
-        final PluginManager manager = new PluginManager(Node.CATEGORY);
-        manager.collectPlugins();
-        final Object obj = new PluginBuilder(manager.getPluginType("Rfc5424Layout"))
+        final Object obj = new PluginBuilder(pluginManager.getPluginType("Rfc5424Layout"))
                 .withConfigurationNode(new Node())
                 .withConfiguration(new DefaultConfiguration())
                 .build();
@@ -811,13 +816,10 @@ private static Map<String, String> attributeMap(String... keyValuePairs) {
     }
  
     private static Rfc5424Layout buildRfc5424Layout(Map<String, String> attributes) {
-        PluginManager manager = new PluginManager(Node.CATEGORY);
-        manager.collectPlugins();
- 
         Node node = new Node();
         node.getAttributes().putAll(attributes);
  
-        Object object = new PluginBuilder(manager.getPluginType("Rfc5424Layout"))
+        Object object = new PluginBuilder(pluginManager.getPluginType("Rfc5424Layout"))
                 .withConfigurationNode(node)
                 .withConfiguration(new DefaultConfiguration())
                 .build();