diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ScriptsPluginTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ScriptsPluginTest.java new file mode 100644 index 00000000000..0ea1acd6ef7 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ScriptsPluginTest.java @@ -0,0 +1,78 @@ +/* + * 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.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.apache.logging.log4j.core.script.AbstractScript; +import org.junit.jupiter.api.Test; + +class ScriptsPluginTest { + + @Test + void testCreateScriptsNullInput() { + assertThrows(NullPointerException.class, () -> ScriptsPlugin.createScripts(null)); + } + + @Test + void testCreateScriptsEmptyInput() { + AbstractScript[] emptyArray = new AbstractScript[0]; + assertSame(emptyArray, ScriptsPlugin.createScripts(emptyArray), "Should return empty array"); + } + + @Test + void testCreateScriptsAllExplicitNames() { + AbstractScript script1 = new MockScript("script1", "JavaScript", "text"); + AbstractScript script2 = new MockScript("script2", "Groovy", "text"); + AbstractScript[] input = {script1, script2}; + AbstractScript[] result = ScriptsPlugin.createScripts(input); + assertEquals(2, result.length, "Should return 2 scripts"); + assertArrayEquals(input, result, "Should contain all valid scripts"); + } + + @Test + void testCreateScriptsImplicitName() { + AbstractScript script = new MockScript(null, "JavaScript", "text"); + AbstractScript[] input = {script}; + assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input)); + } + + @Test + void testCreateScriptsBlankName() { + AbstractScript script = new MockScript(" ", "JavaScript", "text"); + AbstractScript[] input = {script}; + assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input)); + } + + @Test + void testCreateScriptsMixedExplicitAndImplicitNames() { + AbstractScript explicitScript = new MockScript("script", "Python", "text"); + AbstractScript implicitScript = new MockScript(null, "JavaScript", "text"); + AbstractScript[] input = {explicitScript, implicitScript}; + assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input)); + } + + private class MockScript extends AbstractScript { + + public MockScript(String name, String language, String scriptText) { + super(name, language, scriptText); + } + } +} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/script/AbstractScriptTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/script/AbstractScriptTest.java new file mode 100644 index 00000000000..d4cbef56064 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/script/AbstractScriptTest.java @@ -0,0 +1,71 @@ +/* + * 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.script; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.Test; + +class AbstractScriptTest { + + @Test + void testConstructorAndGettersWithExplicitName() { + final String lang = "JavaScript"; + final String text = "text"; + final String name = "script"; + final AbstractScript script = new MockScript(name, lang, text); + + assertEquals(lang, script.getLanguage(), "Language should match"); + assertEquals(text, script.getScriptText(), "Script text should match"); + assertEquals(name, script.getName(), "Name should match the provided name"); + assertEquals(name, script.getId(), "Id should match the provided name"); + } + + @Test + void testConstructorAndGettersWithImplicitName() { + final String lang = "JavaScript"; + final String text = "text"; + final AbstractScript script = new MockScript(null, lang, text); + + assertEquals(lang, script.getLanguage(), "Language should match"); + assertEquals(text, script.getScriptText(), "Script text should match"); + assertNull(script.getName(), "Name should be null"); + assertNotNull(script.getId(), "Id should not be null"); + } + + @Test + void testConstructorAndGettersWithBlankName() { + final String lang = "JavaScript"; + final String text = "text"; + final String name = " "; + final AbstractScript script = new MockScript(name, lang, text); + + assertEquals(lang, script.getLanguage(), "Language should match"); + assertEquals(text, script.getScriptText(), "Script text should match"); + assertEquals(name, script.getName(), "Name should be blank"); + assertNotNull(script.getId(), "Id should not be null"); + } + + private class MockScript extends AbstractScript { + + public MockScript(String name, String language, String scriptText) { + super(name, language, scriptText); + } + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/ScriptAppenderSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/ScriptAppenderSelector.java index 200fa47ec7c..efbd93e433d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/ScriptAppenderSelector.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/ScriptAppenderSelector.java @@ -94,9 +94,9 @@ public Appender build() { "ScriptAppenderSelector '{}' executing {} '{}': {}", name, script.getLanguage(), - script.getName(), + script.getId(), script.getScriptText()); - final Object object = scriptManager.execute(script.getName(), bindings); + final Object object = scriptManager.execute(script.getId(), bindings); final String actualAppenderName = Objects.toString(object, null); LOGGER.debug("ScriptAppenderSelector '{}' selected '{}'", name, actualAppenderName); return appenderSet.createAppender(actualAppenderName, name); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ScriptCondition.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ScriptCondition.java index 3519980c2ad..b87eff7ccc3 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ScriptCondition.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ScriptCondition.java @@ -73,7 +73,7 @@ public List selectFilesToDelete( bindings.put("configuration", configuration); bindings.put("substitutor", configuration.getStrSubstitutor()); bindings.put("statusLogger", LOGGER); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); return (List) object; } @@ -110,8 +110,8 @@ public static ScriptCondition createCondition( return null; } if (script instanceof ScriptRef) { - if (configuration.getScriptManager().getScript(script.getName()) == null) { - LOGGER.error("ScriptCondition: No script with name {} has been declared.", script.getName()); + if (configuration.getScriptManager().getScript(script.getId()) == null) { + LOGGER.error("ScriptCondition: No script with name {} has been declared.", script.getId()); return null; } } else { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/Routes.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/Routes.java index cb5a452ef27..9ca37c15cb4 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/Routes.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/Routes.java @@ -180,7 +180,7 @@ public String getPattern(final LogEvent event, final ConcurrentMap validScripts = new ArrayList<>(scripts.length); + for (final AbstractScript script : scripts) { + if (Strings.isBlank(script.getName())) { + throw new ConfigurationException("A script defined in lacks an explicit 'name' attribute"); + } else { + validScripts.add(script); + } + } + return validScripts.toArray(new AbstractScript[0]); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/arbiters/ScriptArbiter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/arbiters/ScriptArbiter.java index d4c96da171b..042b36a8146 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/arbiters/ScriptArbiter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/arbiters/ScriptArbiter.java @@ -57,7 +57,7 @@ public boolean isCondition() { final SimpleBindings bindings = new SimpleBindings(); bindings.putAll(configuration.getProperties()); bindings.put("substitutor", configuration.getStrSubstitutor()); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); return Boolean.parseBoolean(object.toString()); } @@ -114,8 +114,8 @@ public ScriptArbiter build() { return null; } if (script instanceof ScriptRef) { - if (configuration.getScriptManager().getScript(script.getName()) == null) { - LOGGER.error("No script with name {} has been declared.", script.getName()); + if (configuration.getScriptManager().getScript(script.getId()) == null) { + LOGGER.error("No script with name {} has been declared.", script.getId()); return null; } } else { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java index 066dc7aa137..cec83951e39 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java @@ -69,7 +69,7 @@ public Result filter( bindings.put("throwable", null); bindings.putAll(configuration.getProperties()); bindings.put("substitutor", configuration.getStrSubstitutor()); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch; } @@ -85,7 +85,7 @@ public Result filter( bindings.put("throwable", t); bindings.putAll(configuration.getProperties()); bindings.put("substitutor", configuration.getStrSubstitutor()); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch; } @@ -101,7 +101,7 @@ public Result filter( bindings.put("throwable", t); bindings.putAll(configuration.getProperties()); bindings.put("substitutor", configuration.getStrSubstitutor()); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch; } @@ -111,13 +111,13 @@ public Result filter(final LogEvent event) { bindings.put("logEvent", event); bindings.putAll(configuration.getProperties()); bindings.put("substitutor", configuration.getStrSubstitutor()); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch; } @Override public String toString() { - return script.getName(); + return script.getId(); } /** @@ -146,8 +146,8 @@ public static ScriptFilter createFilter( return null; } if (script instanceof ScriptRef) { - if (configuration.getScriptManager().getScript(script.getName()) == null) { - logger.error("No script with name {} has been declared.", script.getName()); + if (configuration.getScriptManager().getScript(script.getId()) == null) { + logger.error("No script with name {} has been declared.", script.getId()); return null; } } else { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/ScriptPatternSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/ScriptPatternSelector.java index 92648e24d4b..f1e4d240f31 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/ScriptPatternSelector.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/ScriptPatternSelector.java @@ -89,8 +89,8 @@ public ScriptPatternSelector build() { return null; } if (script instanceof ScriptRef) { - if (configuration.getScriptManager().getScript(script.getName()) == null) { - LOGGER.error("No script with name {} has been declared.", script.getName()); + if (configuration.getScriptManager().getScript(script.getId()) == null) { + LOGGER.error("No script with name {} has been declared.", script.getId()); return null; } } else { @@ -262,7 +262,7 @@ public PatternFormatter[] getFormatters(final LogEvent event) { bindings.putAll(configuration.getProperties()); bindings.put("substitutor", configuration.getStrSubstitutor()); bindings.put("logEvent", event); - final Object object = configuration.getScriptManager().execute(script.getName(), bindings); + final Object object = configuration.getScriptManager().execute(script.getId(), bindings); if (object == null) { return defaultFormatters; } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/AbstractScript.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/AbstractScript.java index d43ab2a452a..1242d59ac3d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/AbstractScript.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/AbstractScript.java @@ -16,8 +16,10 @@ */ package org.apache.logging.log4j.core.script; +import java.util.Objects; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.status.StatusLogger; +import org.apache.logging.log4j.util.Strings; /** * Container for the language and body of a script. @@ -30,11 +32,13 @@ public abstract class AbstractScript { private final String language; private final String scriptText; private final String name; + private final String id; public AbstractScript(final String name, final String language, final String scriptText) { this.language = language; this.scriptText = scriptText; - this.name = name == null ? this.toString() : name; + this.name = name; + this.id = Strings.isBlank(name) ? Integer.toHexString(Objects.hashCode(this)) : name; } public String getLanguage() { @@ -48,4 +52,8 @@ public String getScriptText() { public String getName() { return this.name; } + + public String getId() { + return this.id; + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptFile.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptFile.java index 5ff0b27d34e..8f55ab48cba 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptFile.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptFile.java @@ -123,8 +123,8 @@ public static ScriptFile createScript( @Override public String toString() { final StringBuilder sb = new StringBuilder(); - if (!(getName().equals(filePath.toString()))) { - sb.append("name=").append(getName()).append(", "); + if (!(getId().equals(filePath.toString()))) { + sb.append("name=").append(getId()).append(", "); } sb.append("path=").append(filePath); if (getLanguage() != null) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptManager.java index 5d87f925ae7..1409800bea0 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptManager.java @@ -147,9 +147,9 @@ public boolean addScript(final AbstractScript script) { return false; } if (engine.getFactory().getParameter(KEY_THREADING) == null) { - scriptRunners.put(script.getName(), new ThreadLocalScriptRunner(script)); + scriptRunners.put(script.getId(), new ThreadLocalScriptRunner(script)); } else { - scriptRunners.put(script.getName(), new MainScriptRunner(engine, script)); + scriptRunners.put(script.getId(), new MainScriptRunner(engine, script)); } if (script instanceof ScriptFile) { @@ -162,7 +162,7 @@ public boolean addScript(final AbstractScript script) { } else { logger.error( "Unable to add script {}, {} has not been configured as an allowed language", - script.getName(), + script.getId(), script.getLanguage()); return false; } @@ -173,8 +173,8 @@ public Bindings createBindings(final AbstractScript script) { return getScriptRunner(script).createBindings(); } - public AbstractScript getScript(final String name) { - final ScriptRunner runner = scriptRunners.get(name); + public AbstractScript getScript(final String id) { + final ScriptRunner runner = scriptRunners.get(id); return runner != null ? runner.getScript() : null; } @@ -188,16 +188,16 @@ public void fileModified(final File file) { final ScriptEngine engine = runner.getScriptEngine(); final AbstractScript script = runner.getScript(); if (engine.getFactory().getParameter(KEY_THREADING) == null) { - scriptRunners.put(script.getName(), new ThreadLocalScriptRunner(script)); + scriptRunners.put(script.getId(), new ThreadLocalScriptRunner(script)); } else { - scriptRunners.put(script.getName(), new MainScriptRunner(engine, script)); + scriptRunners.put(script.getId(), new MainScriptRunner(engine, script)); } } - public Object execute(final String name, final Bindings bindings) { - final ScriptRunner scriptRunner = scriptRunners.get(name); + public Object execute(final String id, final Bindings bindings) { + final ScriptRunner scriptRunner = scriptRunners.get(id); if (scriptRunner == null) { - logger.warn("No script named {} could be found", name); + logger.warn("No script named {} could be found", id); return null; } return AccessController.doPrivileged((PrivilegedAction) () -> scriptRunner.execute(bindings)); @@ -224,7 +224,7 @@ public MainScriptRunner(final ScriptEngine scriptEngine, final AbstractScript sc this.scriptEngine = scriptEngine; CompiledScript compiled = null; if (scriptEngine instanceof Compilable) { - logger.debug("Script {} is compilable", script.getName()); + logger.debug("Script {} is compilable", script.getId()); compiled = AccessController.doPrivileged((PrivilegedAction) () -> { try { return ((Compilable) scriptEngine).compile(script.getScriptText()); @@ -252,14 +252,14 @@ public Object execute(final Bindings bindings) { try { return compiledScript.eval(bindings); } catch (final ScriptException ex) { - logger.error("Error running script " + script.getName(), ex); + logger.error("Error running script " + script.getId(), ex); return null; } } try { return scriptEngine.eval(script.getScriptText(), bindings); } catch (final ScriptException ex) { - logger.error("Error running script " + script.getName(), ex); + logger.error("Error running script " + script.getId(), ex); return null; } } @@ -302,6 +302,6 @@ public ScriptEngine getScriptEngine() { } private ScriptRunner getScriptRunner(final AbstractScript script) { - return scriptRunners.get(script.getName()); + return scriptRunners.get(script.getId()); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptRef.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptRef.java index a809f25e9d7..d9bb3df31ae 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptRef.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/ScriptRef.java @@ -38,13 +38,13 @@ public ScriptRef(final String name, final ScriptManager scriptManager) { @Override public String getLanguage() { - final AbstractScript script = this.scriptManager.getScript(getName()); + final AbstractScript script = this.scriptManager.getScript(getId()); return script != null ? script.getLanguage() : null; } @Override public String getScriptText() { - final AbstractScript script = this.scriptManager.getScript(getName()); + final AbstractScript script = this.scriptManager.getScript(getId()); return script != null ? script.getScriptText() : null; } @@ -62,6 +62,6 @@ public static ScriptRef createReference( @Override public String toString() { - return "ref=" + getName(); + return "ref=" + getId(); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/package-info.java index d973435b64c..1374fe6673b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/script/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/script/package-info.java @@ -18,7 +18,7 @@ * Log4j 2 Script support. */ @Export -@Version("2.20.2") +@Version("2.26.0") package org.apache.logging.log4j.core.script; import org.osgi.annotation.bundle.Export; diff --git a/src/changelog/.2.x.x/3176_validate_scripts_in_ScriptsPlugin.xml b/src/changelog/.2.x.x/3176_validate_scripts_in_ScriptsPlugin.xml new file mode 100644 index 00000000000..0246396937d --- /dev/null +++ b/src/changelog/.2.x.x/3176_validate_scripts_in_ScriptsPlugin.xml @@ -0,0 +1,13 @@ + + + + + Ensured scripts in global `Scripts container` have explicit names by throwing a `ConfigurationException` for unnamed ones. + Refined script identification with `AbstractScript.getId()` and clarified `AbstractScript.getName()`. + +