diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationPropertiesOrderTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationPropertiesOrderTest.java new file mode 100644 index 00000000000..a725283da93 --- /dev/null +++ b/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); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java index d95202be2a0..10a2bcfab47 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java +++ b/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 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,11 +691,11 @@ 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; @@ -702,7 +703,7 @@ protected void doConfigure() { 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 copy = new ArrayList<>(customLevels); diff --git a/src/changelog/.2.x.x/allow_arbitrary_properties_order.xml b/src/changelog/.2.x.x/allow_arbitrary_properties_order.xml new file mode 100644 index 00000000000..825b147cc3c --- /dev/null +++ b/src/changelog/.2.x.x/allow_arbitrary_properties_order.xml @@ -0,0 +1,9 @@ + + + + Allow the <Properties> node to appear in any position in the configuration element. + +