Skip to content

Commit 1450533

Browse files
jhl221123jhl221123
authored
Enforce explicit names for global scripts and refine script id handling (#3691)
* Refactor script identification and enforce explicit names in global Scripts container This commit introduces a clearer distinction between user-defined script names and internal script identifiers, and enforces explicit naming for global scripts. - AbstractScript now provides 'getName()' for user-set names and 'getId()' for an internal Id. - ScriptsPlugin throws a 'ConfigurationException' if global scripts lack a non-blank name via 'getName()'. - ScriptManager and its users were updated to utilize 'script.getId()' for internal referencing. - Added unit tests for 'AbstractScript' and 'ScriptsPlugin'. - Updated 'org.apache.logging.log4j.core.script' package-info to version 2.25.0. - Added a changelog entry for issue #3176. * Refine script handling and improve runtime safety - Improve null-safety in ScriptsPlugin by throwing an exception for null input. - Replace unsafe toString() call in AbstractScript constructor with a safer internal ID generation. - Adjust test cases to align with code changes and project conventions. - Update OSGi @Version annotation to 2.26.0 for the next release cycle.
1 parent 422c385 commit 1450533

File tree

16 files changed

+230
-42
lines changed

16 files changed

+230
-42
lines changed

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.config;
18+
19+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertSame;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
24+
import org.apache.logging.log4j.core.script.AbstractScript;
25+
import org.junit.jupiter.api.Test;
26+
27+
class ScriptsPluginTest {
28+
29+
@Test
30+
void testCreateScriptsNullInput() {
31+
assertThrows(NullPointerException.class, () -> ScriptsPlugin.createScripts(null));
32+
}
33+
34+
@Test
35+
void testCreateScriptsEmptyInput() {
36+
AbstractScript[] emptyArray = new AbstractScript[0];
37+
assertSame(emptyArray, ScriptsPlugin.createScripts(emptyArray), "Should return empty array");
38+
}
39+
40+
@Test
41+
void testCreateScriptsAllExplicitNames() {
42+
AbstractScript script1 = new MockScript("script1", "JavaScript", "text");
43+
AbstractScript script2 = new MockScript("script2", "Groovy", "text");
44+
AbstractScript[] input = {script1, script2};
45+
AbstractScript[] result = ScriptsPlugin.createScripts(input);
46+
assertEquals(2, result.length, "Should return 2 scripts");
47+
assertArrayEquals(input, result, "Should contain all valid scripts");
48+
}
49+
50+
@Test
51+
void testCreateScriptsImplicitName() {
52+
AbstractScript script = new MockScript(null, "JavaScript", "text");
53+
AbstractScript[] input = {script};
54+
assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input));
55+
}
56+
57+
@Test
58+
void testCreateScriptsBlankName() {
59+
AbstractScript script = new MockScript(" ", "JavaScript", "text");
60+
AbstractScript[] input = {script};
61+
assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input));
62+
}
63+
64+
@Test
65+
void testCreateScriptsMixedExplicitAndImplicitNames() {
66+
AbstractScript explicitScript = new MockScript("script", "Python", "text");
67+
AbstractScript implicitScript = new MockScript(null, "JavaScript", "text");
68+
AbstractScript[] input = {explicitScript, implicitScript};
69+
assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input));
70+
}
71+
72+
private class MockScript extends AbstractScript {
73+
74+
public MockScript(String name, String language, String scriptText) {
75+
super(name, language, scriptText);
76+
}
77+
}
78+
}

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/script/AbstractScriptTest.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.script;
18+
19+
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertNotNull;
21+
import static org.junit.jupiter.api.Assertions.assertNull;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
class AbstractScriptTest {
26+
27+
@Test
28+
void testConstructorAndGettersWithExplicitName() {
29+
final String lang = "JavaScript";
30+
final String text = "text";
31+
final String name = "script";
32+
final AbstractScript script = new MockScript(name, lang, text);
33+
34+
assertEquals(lang, script.getLanguage(), "Language should match");
35+
assertEquals(text, script.getScriptText(), "Script text should match");
36+
assertEquals(name, script.getName(), "Name should match the provided name");
37+
assertEquals(name, script.getId(), "Id should match the provided name");
38+
}
39+
40+
@Test
41+
void testConstructorAndGettersWithImplicitName() {
42+
final String lang = "JavaScript";
43+
final String text = "text";
44+
final AbstractScript script = new MockScript(null, lang, text);
45+
46+
assertEquals(lang, script.getLanguage(), "Language should match");
47+
assertEquals(text, script.getScriptText(), "Script text should match");
48+
assertNull(script.getName(), "Name should be null");
49+
assertNotNull(script.getId(), "Id should not be null");
50+
}
51+
52+
@Test
53+
void testConstructorAndGettersWithBlankName() {
54+
final String lang = "JavaScript";
55+
final String text = "text";
56+
final String name = " ";
57+
final AbstractScript script = new MockScript(name, lang, text);
58+
59+
assertEquals(lang, script.getLanguage(), "Language should match");
60+
assertEquals(text, script.getScriptText(), "Script text should match");
61+
assertEquals(name, script.getName(), "Name should be blank");
62+
assertNotNull(script.getId(), "Id should not be null");
63+
}
64+
65+
private class MockScript extends AbstractScript {
66+
67+
public MockScript(String name, String language, String scriptText) {
68+
super(name, language, scriptText);
69+
}
70+
}
71+
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/appender/ScriptAppenderSelector.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ public Appender build() {
9494
"ScriptAppenderSelector '{}' executing {} '{}': {}",
9595
name,
9696
script.getLanguage(),
97-
script.getName(),
97+
script.getId(),
9898
script.getScriptText());
99-
final Object object = scriptManager.execute(script.getName(), bindings);
99+
final Object object = scriptManager.execute(script.getId(), bindings);
100100
final String actualAppenderName = Objects.toString(object, null);
101101
LOGGER.debug("ScriptAppenderSelector '{}' selected '{}'", name, actualAppenderName);
102102
return appenderSet.createAppender(actualAppenderName, name);

‎log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/ScriptCondition.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public List<PathWithAttributes> selectFilesToDelete(
7373
bindings.put("configuration", configuration);
7474
bindings.put("substitutor", configuration.getStrSubstitutor());
7575
bindings.put("statusLogger", LOGGER);
76-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
76+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
7777
return (List<PathWithAttributes>) object;
7878
}
7979

@@ -110,8 +110,8 @@ public static ScriptCondition createCondition(
110110
return null;
111111
}
112112
if (script instanceof ScriptRef) {
113-
if (configuration.getScriptManager().getScript(script.getName()) == null) {
114-
LOGGER.error("ScriptCondition: No script with name {} has been declared.", script.getName());
113+
if (configuration.getScriptManager().getScript(script.getId()) == null) {
114+
LOGGER.error("ScriptCondition: No script with name {} has been declared.", script.getId());
115115
return null;
116116
}
117117
} else {

‎log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/Routes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public String getPattern(final LogEvent event, final ConcurrentMap<Object, Objec
180180
final Bindings bindings = scriptManager.createBindings(patternScript);
181181
bindings.put(STATIC_VARIABLES_KEY, scriptStaticVariables);
182182
bindings.put(LOG_EVENT_KEY, event);
183-
final Object object = scriptManager.execute(patternScript.getName(), bindings);
183+
final Object object = scriptManager.execute(patternScript.getId(), bindings);
184184
bindings.remove(LOG_EVENT_KEY);
185185
return Objects.toString(object, null);
186186
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public void start() {
202202
final ScriptManager scriptManager = configuration.getScriptManager();
203203
final Bindings bindings = scriptManager.createBindings(defaultRouteScript);
204204
bindings.put(STATIC_VARIABLES_KEY, scriptStaticVariables);
205-
final Object object = scriptManager.execute(defaultRouteScript.getName(), bindings);
205+
final Object object = scriptManager.execute(defaultRouteScript.getId(), bindings);
206206
final Route route = routes.getRoute(Objects.toString(object, null));
207207
if (route != null) {
208208
defaultRoute = route;

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,21 @@
1616
*/
1717
package org.apache.logging.log4j.core.config;
1818

19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.Objects;
1922
import org.apache.logging.log4j.core.Core;
2023
import org.apache.logging.log4j.core.config.plugins.Plugin;
2124
import org.apache.logging.log4j.core.config.plugins.PluginElement;
2225
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
2326
import org.apache.logging.log4j.core.script.AbstractScript;
27+
import org.apache.logging.log4j.util.Strings;
28+
import org.jspecify.annotations.NullMarked;
2429

2530
/**
2631
* A container of Scripts.
2732
*/
33+
@NullMarked
2834
@Plugin(name = "Scripts", category = Core.CATEGORY_NAME)
2935
public final class ScriptsPlugin {
3036

@@ -37,7 +43,19 @@ private ScriptsPlugin() {}
3743
*/
3844
@PluginFactory
3945
public static AbstractScript[] createScripts(@PluginElement("Scripts") final AbstractScript[] scripts) {
46+
Objects.requireNonNull(scripts, "Scripts array cannot be null");
47+
if (scripts.length == 0) {
48+
return scripts;
49+
}
4050

41-
return scripts;
51+
final List<AbstractScript> validScripts = new ArrayList<>(scripts.length);
52+
for (final AbstractScript script : scripts) {
53+
if (Strings.isBlank(script.getName())) {
54+
throw new ConfigurationException("A script defined in <Scripts> lacks an explicit 'name' attribute");
55+
} else {
56+
validScripts.add(script);
57+
}
58+
}
59+
return validScripts.toArray(new AbstractScript[0]);
4260
}
4361
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/config/arbiters/ScriptArbiter.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public boolean isCondition() {
5757
final SimpleBindings bindings = new SimpleBindings();
5858
bindings.putAll(configuration.getProperties());
5959
bindings.put("substitutor", configuration.getStrSubstitutor());
60-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
60+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
6161
return Boolean.parseBoolean(object.toString());
6262
}
6363

@@ -114,8 +114,8 @@ public ScriptArbiter build() {
114114
return null;
115115
}
116116
if (script instanceof ScriptRef) {
117-
if (configuration.getScriptManager().getScript(script.getName()) == null) {
118-
LOGGER.error("No script with name {} has been declared.", script.getName());
117+
if (configuration.getScriptManager().getScript(script.getId()) == null) {
118+
LOGGER.error("No script with name {} has been declared.", script.getId());
119119
return null;
120120
}
121121
} else {

‎log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public Result filter(
6969
bindings.put("throwable", null);
7070
bindings.putAll(configuration.getProperties());
7171
bindings.put("substitutor", configuration.getStrSubstitutor());
72-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
72+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
7373
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
7474
}
7575

@@ -85,7 +85,7 @@ public Result filter(
8585
bindings.put("throwable", t);
8686
bindings.putAll(configuration.getProperties());
8787
bindings.put("substitutor", configuration.getStrSubstitutor());
88-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
88+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
8989
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
9090
}
9191

@@ -101,7 +101,7 @@ public Result filter(
101101
bindings.put("throwable", t);
102102
bindings.putAll(configuration.getProperties());
103103
bindings.put("substitutor", configuration.getStrSubstitutor());
104-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
104+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
105105
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
106106
}
107107

@@ -111,13 +111,13 @@ public Result filter(final LogEvent event) {
111111
bindings.put("logEvent", event);
112112
bindings.putAll(configuration.getProperties());
113113
bindings.put("substitutor", configuration.getStrSubstitutor());
114-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
114+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
115115
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
116116
}
117117

118118
@Override
119119
public String toString() {
120-
return script.getName();
120+
return script.getId();
121121
}
122122

123123
/**
@@ -146,8 +146,8 @@ public static ScriptFilter createFilter(
146146
return null;
147147
}
148148
if (script instanceof ScriptRef) {
149-
if (configuration.getScriptManager().getScript(script.getName()) == null) {
150-
logger.error("No script with name {} has been declared.", script.getName());
149+
if (configuration.getScriptManager().getScript(script.getId()) == null) {
150+
logger.error("No script with name {} has been declared.", script.getId());
151151
return null;
152152
}
153153
} else {

‎log4j-core/src/main/java/org/apache/logging/log4j/core/layout/ScriptPatternSelector.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ public ScriptPatternSelector build() {
8989
return null;
9090
}
9191
if (script instanceof ScriptRef) {
92-
if (configuration.getScriptManager().getScript(script.getName()) == null) {
93-
LOGGER.error("No script with name {} has been declared.", script.getName());
92+
if (configuration.getScriptManager().getScript(script.getId()) == null) {
93+
LOGGER.error("No script with name {} has been declared.", script.getId());
9494
return null;
9595
}
9696
} else {
@@ -262,7 +262,7 @@ public PatternFormatter[] getFormatters(final LogEvent event) {
262262
bindings.putAll(configuration.getProperties());
263263
bindings.put("substitutor", configuration.getStrSubstitutor());
264264
bindings.put("logEvent", event);
265-
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
265+
final Object object = configuration.getScriptManager().execute(script.getId(), bindings);
266266
if (object == null) {
267267
return defaultFormatters;
268268
}

0 commit comments

Comments
 (0)