Allow arbitrary position of <Properties> element

log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationPropertiesOrderTest.java

@@ -0,0 +1,67 @@
+/*
+ * 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.
+ */
+package org.apache.logging.log4j.core.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.appender.ConsoleAppender;
+import org.junit.jupiter.api.Test;
+
+public class ConfigurationPropertiesOrderTest {
+
+    @Test
+    void propertiesCanComeLast() {
+        final Configuration config = new AbstractConfiguration(null, ConfigurationSource.NULL_SOURCE) {
+            @Override
+            public void setup() {
+                // Nodes
+                final Node appenders = newNode(rootNode, "Appenders");
+                rootNode.getChildren().add(appenders);
+
+                final Node console = newNode(appenders, "Console");
+                console.getAttributes().put("name", "${console.name}");
+                appenders.getChildren().add(console);
+
+                final Node loggers = newNode(rootNode, "Loggers");
+                rootNode.getChildren().add(loggers);
+
+                final Node rootLogger = newNode(loggers, "Root");
+                rootLogger.getAttributes().put("level", "INFO");
+                loggers.getChildren().add(rootLogger);
+
+                final Node properties = newNode(rootNode, "Properties");
+                rootNode.getChildren().add(properties);
+
+                final Node property = newNode(properties, "Property");
+                property.getAttributes().put("name", "console.name");
+                property.getAttributes().put("value", "CONSOLE");
+                properties.getChildren().add(property);
+            }
+
+            private Node newNode(final Node parent, final String name) {
+                return new Node(rootNode, name, pluginManager.getPluginType(name));
+            }
+        };
+        config.initialize();
+
+        final Appender noInterpolation = config.getAppender("${console.name}");
+        assertThat(noInterpolation).as("No interpolation for '${console.name}'").isNull();
+        final Appender console = config.getAppender("CONSOLE");
+        assertThat(console).isInstanceOf(ConsoleAppender.class);
+    }
+}

log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java

@@ -647,18 +647,21 @@ protected void doConfigure() {
         processConditionals(rootNode);
         preConfigure(rootNode);
         configurationScheduler.start();
-        if (rootNode.hasChildren() && rootNode.getChildren().get(0).getName().equalsIgnoreCase("Properties")) {
-            final Node first = rootNode.getChildren().get(0);
-            createConfiguration(first, null);
-            if (first.getObject() != null) {
-                final StrLookup lookup = (StrLookup) first.getObject();
-                if (lookup instanceof LoggerContextAware) {
-                    ((LoggerContextAware) lookup).setLoggerContext(loggerContext.get());
+        // Find the "Properties" node first
+        boolean hasProperties = false;
+        for (final Node node : rootNode.getChildren()) {
+            if ("Properties".equalsIgnoreCase(node.getName())) {
+                hasProperties = true;
+                createConfiguration(node, null);
+                if (node.getObject() != null) {
+                    final StrLookup lookup = node.getObject();
+                    runtimeStrSubstitutor.setVariableResolver(lookup);
+                    configurationStrSubstitutor.setVariableResolver(lookup);
                 }
-                runtimeStrSubstitutor.setVariableResolver(lookup);
-                configurationStrSubstitutor.setVariableResolver(lookup);
+                break;
             }
-        } else {
+        }
+        if (!hasProperties) {
             final Map<String, String> map = this.getComponent(CONTEXT_PROPERTIES);
             final StrLookup lookup = map == null ? null : new PropertiesLookup(map);
             final Interpolator interpolator = new Interpolator(lookup, pluginPackages);
@@ -670,17 +673,15 @@ protected void doConfigure() {
         boolean setLoggers = false;
         boolean setRoot = false;
         for (final Node child : rootNode.getChildren()) {
-            if (child.getName().equalsIgnoreCase("Properties")) {
-                if (tempLookup == runtimeStrSubstitutor.getVariableResolver()) {
-                    LOGGER.error("Properties declaration must be the first element in the configuration");
-                }
+            if ("Properties".equalsIgnoreCase(child.getName())) {
+                // We already used this node
                 continue;
             }
             createConfiguration(child, null);
             if (child.getObject() == null) {
                 continue;
             }
-            if (child.getName().equalsIgnoreCase("Scripts")) {
+            if ("Scripts".equalsIgnoreCase(child.getName())) {
                 for (final AbstractScript script : child.getObject(AbstractScript[].class)) {
                     if (script instanceof ScriptRef) {
                         LOGGER.error(
@@ -690,19 +691,19 @@ protected void doConfigure() {
                         scriptManager.addScript(script);
                     }
                 }
-            } else if (child.getName().equalsIgnoreCase("Appenders")) {
+            } else if ("Appenders".equalsIgnoreCase(child.getName())) {
                 appenders = child.getObject();
             } else if (child.isInstanceOf(Filter.class)) {
                 addFilter(child.getObject(Filter.class));
-            } else if (child.getName().equalsIgnoreCase("Loggers")) {
+            } else if (child.isInstanceOf(Loggers.class)) {
                 final Loggers l = child.getObject();
                 loggerConfigs = l.getMap();
                 setLoggers = true;
                 if (l.getRoot() != null) {
                     root = l.getRoot();
                     setRoot = true;
                 }
-            } else if (child.getName().equalsIgnoreCase("CustomLevels")) {
+            } else if (child.isInstanceOf(CustomLevels.class)) {
                 customLevels = child.getObject(CustomLevels.class).getCustomLevels();
             } else if (child.isInstanceOf(CustomLevelConfig.class)) {
                 final List<CustomLevelConfig> copy = new ArrayList<>(customLevels);

src/changelog/.2.x.x/allow_arbitrary_properties_order.xml

@@ -0,0 +1,9 @@
+<?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.3.xsd"
+       type="fixed">
+  <description format="asciidoc">
+    Allow the &lt;Properties&gt; node to appear in any position in the configuration element.
+  </description>
+</entry>