diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/AbstractConfigurationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/AbstractConfigurationTest.java index 91d4405edf2..64ce9ac3fc6 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/AbstractConfigurationTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/AbstractConfigurationTest.java @@ -24,14 +24,19 @@ import java.util.Map; import java.util.Map.Entry; import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.filter.CompositeFilter; +import org.apache.logging.log4j.core.filter.ScriptFilter; import org.apache.logging.log4j.core.lookup.Interpolator; import org.apache.logging.log4j.core.lookup.InterpolatorTest; import org.apache.logging.log4j.core.lookup.StrSubstitutor; +import org.apache.logging.log4j.core.test.junit.LoggerContextSource; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.junitpioneer.jupiter.Issue; +@SetTestProperty(key = "log4j2.scriptEnableLanguages", value = "groovy") class AbstractConfigurationTest { @Test @@ -60,6 +65,26 @@ void substitutorHasConfigurationAndLoggerContext(final boolean hasProperties) { } } + @Test + @LoggerContextSource("log4j2-script-order-test.xml") + void scriptRefShouldBeResolvedWhenScriptsElementIsLast(final Configuration config) { + assertThat(config.getFilter()) + .as("Top-level filter should be a CompositeFilter") + .isInstanceOf(CompositeFilter.class); + final CompositeFilter compositeFilter = (CompositeFilter) config.getFilter(); + + assertThat(compositeFilter.getFilters()) + .as("CompositeFilter should contain one filter") + .hasSize(1); + final ScriptFilter scriptFilter = + (ScriptFilter) compositeFilter.getFilters().get(0); + + assertThat(scriptFilter).isNotNull(); + assertThat(scriptFilter.toString()) + .as("Script name should match the one in the config") + .isEqualTo("GLOBAL_FILTER"); + } + private static class TestConfiguration extends AbstractConfiguration { private final Map map; diff --git a/log4j-core-test/src/test/resources/log4j2-script-order-test.xml b/log4j-core-test/src/test/resources/log4j2-script-order-test.xml new file mode 100644 index 00000000000..810b160973b --- /dev/null +++ b/log4j-core-test/src/test/resources/log4j2-script-order-test.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + 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 4c3dde91790..402dfe6d7fd 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 @@ -682,13 +682,14 @@ protected void doConfigure() { preConfigure(rootNode); configurationScheduler.start(); // Find the "Properties" node first + final List children = rootNode.getChildren(); boolean hasProperties = false; - for (final Node node : rootNode.getChildren()) { - if ("Properties".equalsIgnoreCase(node.getName())) { + for (final Node child : children) { + if ("Properties".equalsIgnoreCase(child.getName())) { hasProperties = true; - createConfiguration(node, null); - if (node.getObject() != null) { - final StrLookup lookup = node.getObject(); + createConfiguration(child, null); + if (child.getObject() != null) { + final StrLookup lookup = child.getObject(); runtimeStrSubstitutor.setVariableResolver(lookup); configurationStrSubstitutor.setVariableResolver(lookup); } @@ -705,10 +706,28 @@ protected void doConfigure() { configurationStrSubstitutor.setVariableResolver(interpolator); } + for (final Node child : children) { + if ("Scripts".equalsIgnoreCase(child.getName())) { + createConfiguration(child, null); + if (child.getObject() != null) { + for (final AbstractScript script : child.getObject(AbstractScript[].class)) { + if (script instanceof ScriptRef) { + LOGGER.error( + "Script reference to {} not added. Scripts definition cannot contain script references", + script.getName()); + } else if (scriptManager != null) { + scriptManager.addScript(script); + } + } + } + break; + } + } + boolean setLoggers = false; boolean setRoot = false; - for (final Node child : rootNode.getChildren()) { - if ("Properties".equalsIgnoreCase(child.getName())) { + for (final Node child : children) { + if ("Properties".equalsIgnoreCase(child.getName()) || "Scripts".equalsIgnoreCase(child.getName())) { // We already used this node continue; } @@ -716,17 +735,7 @@ protected void doConfigure() { if (child.getObject() == null) { continue; } - if ("Scripts".equalsIgnoreCase(child.getName())) { - for (final AbstractScript script : child.getObject(AbstractScript[].class)) { - if (script instanceof ScriptRef) { - LOGGER.error( - "Script reference to {} not added. Scripts definition cannot contain script references", - script.getName()); - } else if (scriptManager != null) { - scriptManager.addScript(script); - } - } - } else if ("Appenders".equalsIgnoreCase(child.getName())) { + if ("Appenders".equalsIgnoreCase(child.getName())) { appenders = child.getObject(); } else if (child.isInstanceOf(Filter.class)) { addFilter(child.getObject(Filter.class)); diff --git a/src/changelog/.2.x.x/3336_script_resolution_order_fix.xml b/src/changelog/.2.x.x/3336_script_resolution_order_fix.xml new file mode 100644 index 00000000000..f9adb48e5b2 --- /dev/null +++ b/src/changelog/.2.x.x/3336_script_resolution_order_fix.xml @@ -0,0 +1,12 @@ + + + + + Fix script resolution failure when the `Scripts` element is placed after a `ScriptRef` in the configuration. + +