Skip to content

Commit 8ac8aa2

Browse files
committed
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 a44c320 commit 8ac8aa2

File tree

5 files changed

+21
-16
lines changed

5 files changed

+21
-16
lines changed

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
2020
import static org.junit.jupiter.api.Assertions.assertEquals;
21-
import static org.junit.jupiter.api.Assertions.assertNull;
2221
import static org.junit.jupiter.api.Assertions.assertSame;
2322
import static org.junit.jupiter.api.Assertions.assertThrows;
2423

@@ -28,18 +27,18 @@
2827
class ScriptsPluginTest {
2928

3029
@Test
31-
public void testCreateScriptsNullInput() {
32-
assertNull(ScriptsPlugin.createScripts(null), "Should return null");
30+
void testCreateScriptsNullInput() {
31+
assertThrows(NullPointerException.class, () -> ScriptsPlugin.createScripts(null));
3332
}
3433

3534
@Test
36-
public void testCreateScriptsEmptyInput() {
35+
void testCreateScriptsEmptyInput() {
3736
AbstractScript[] emptyArray = new AbstractScript[0];
3837
assertSame(emptyArray, ScriptsPlugin.createScripts(emptyArray), "Should return empty array");
3938
}
4039

4140
@Test
42-
public void testCreateScriptsAllExplicitNames() {
41+
void testCreateScriptsAllExplicitNames() {
4342
AbstractScript script1 = new MockScript("script1", "JavaScript", "text");
4443
AbstractScript script2 = new MockScript("script2", "Groovy", "text");
4544
AbstractScript[] input = {script1, script2};
@@ -49,21 +48,21 @@ public void testCreateScriptsAllExplicitNames() {
4948
}
5049

5150
@Test
52-
public void testCreateScriptsImplicitName() {
51+
void testCreateScriptsImplicitName() {
5352
AbstractScript script = new MockScript(null, "JavaScript", "text");
5453
AbstractScript[] input = {script};
5554
assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input));
5655
}
5756

5857
@Test
59-
public void testCreateScriptsBlankName() {
58+
void testCreateScriptsBlankName() {
6059
AbstractScript script = new MockScript(" ", "JavaScript", "text");
6160
AbstractScript[] input = {script};
6261
assertThrows(ConfigurationException.class, () -> ScriptsPlugin.createScripts(input));
6362
}
6463

6564
@Test
66-
public void testCreateScriptsMixedExplicitAndImplicitNames() {
65+
void testCreateScriptsMixedExplicitAndImplicitNames() {
6766
AbstractScript explicitScript = new MockScript("script", "Python", "text");
6867
AbstractScript implicitScript = new MockScript(null, "JavaScript", "text");
6968
AbstractScript[] input = {explicitScript, implicitScript};

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@
1717
package org.apache.logging.log4j.core.script;
1818

1919
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertNotNull;
2021
import static org.junit.jupiter.api.Assertions.assertNull;
2122

2223
import org.junit.jupiter.api.Test;
2324

2425
class AbstractScriptTest {
2526

2627
@Test
27-
public void testConstructorAndGettersWithExplicitName() {
28+
void testConstructorAndGettersWithExplicitName() {
2829
final String lang = "JavaScript";
2930
final String text = "text";
3031
final String name = "script";
@@ -37,19 +38,19 @@ public void testConstructorAndGettersWithExplicitName() {
3738
}
3839

3940
@Test
40-
public void testConstructorAndGettersWithImplicitName() {
41+
void testConstructorAndGettersWithImplicitName() {
4142
final String lang = "JavaScript";
4243
final String text = "text";
4344
final AbstractScript script = new MockScript(null, lang, text);
4445

4546
assertEquals(lang, script.getLanguage(), "Language should match");
4647
assertEquals(text, script.getScriptText(), "Script text should match");
4748
assertNull(script.getName(), "Name should be null");
48-
assertEquals(script.toString(), script.getId(), "Id should match its toString()");
49+
assertNotNull(script.getId(), "Id should not be null");
4950
}
5051

5152
@Test
52-
public void testConstructorAndGettersWithBlankName() {
53+
void testConstructorAndGettersWithBlankName() {
5354
final String lang = "JavaScript";
5455
final String text = "text";
5556
final String name = " ";
@@ -58,7 +59,7 @@ public void testConstructorAndGettersWithBlankName() {
5859
assertEquals(lang, script.getLanguage(), "Language should match");
5960
assertEquals(text, script.getScriptText(), "Script text should match");
6061
assertEquals(name, script.getName(), "Name should be blank");
61-
assertEquals(script.toString(), script.getId(), "Id should match its toString()");
62+
assertNotNull(script.getId(), "Id should not be null");
6263
}
6364

6465
private class MockScript extends AbstractScript {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,19 @@
1818

1919
import java.util.ArrayList;
2020
import java.util.List;
21+
import java.util.Objects;
2122
import org.apache.logging.log4j.core.Core;
2223
import org.apache.logging.log4j.core.config.plugins.Plugin;
2324
import org.apache.logging.log4j.core.config.plugins.PluginElement;
2425
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
2526
import org.apache.logging.log4j.core.script.AbstractScript;
2627
import org.apache.logging.log4j.util.Strings;
28+
import org.jspecify.annotations.NullMarked;
2729

2830
/**
2931
* A container of Scripts.
3032
*/
33+
@NullMarked
3134
@Plugin(name = "Scripts", category = Core.CATEGORY_NAME)
3235
public final class ScriptsPlugin {
3336

@@ -40,7 +43,8 @@ private ScriptsPlugin() {}
4043
*/
4144
@PluginFactory
4245
public static AbstractScript[] createScripts(@PluginElement("Scripts") final AbstractScript[] scripts) {
43-
if (scripts == null || scripts.length == 0) {
46+
Objects.requireNonNull(scripts, "Scripts array cannot be null");
47+
if (scripts.length == 0) {
4448
return scripts;
4549
}
4650

log4j-core/src/main/java/org/apache/logging/log4j/core/script/AbstractScript.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package org.apache.logging.log4j.core.script;
1818

19+
import java.util.Objects;
1920
import org.apache.logging.log4j.Logger;
2021
import org.apache.logging.log4j.status.StatusLogger;
2122
import org.apache.logging.log4j.util.Strings;
@@ -37,7 +38,7 @@ public AbstractScript(final String name, final String language, final String scr
3738
this.language = language;
3839
this.scriptText = scriptText;
3940
this.name = name;
40-
this.id = Strings.isBlank(name) ? this.toString() : name;
41+
this.id = Strings.isBlank(name) ? Integer.toHexString(Objects.hashCode(this)) : name;
4142
}
4243

4344
public String getLanguage() {

log4j-core/src/main/java/org/apache/logging/log4j/core/script/package-info.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* Log4j 2 Script support.
1919
*/
2020
@Export
21-
@Version("2.25.0")
21+
@Version("2.26.0")
2222
package org.apache.logging.log4j.core.script;
2323

2424
import org.osgi.annotation.bundle.Export;

0 commit comments

Comments
 (0)