Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Fix: Script Resolution fix #3394

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from
Open

Bug Fix: Script Resolution fix #3394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f6fde1
Script resolution
Suvrat1629 Jan 14, 2025
e38fb5c
script res
Suvrat1629 Jan 14, 2025
b71bff8
Merge branch 'apache:2.x' into 2.x
Suvrat1629 Jan 14, 2025
92f8cbe
minor fix
Suvrat1629 Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion log4j-core-test/src/test/resources/log4j-script-filters.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@
<Configuration status="ERROR">
<Appenders>
<List name="List">
<PatternLayout pattern="[%-5level] %c{1.} %msg%n"/>
<PatternLayout pattern="[%-5level] %c{1.} %msg%n"/>[suvrat@suvrat logging-log4j2]$ git push
Username for 'https://github.com': suvrat1629
Password for 'https://[email protected]':
remote: Support for password authentication was removed on August 13, 2021.
remote: Please see https://docs.github.com/get-started/getting-started-with-git/about-remote-repositories#cloning-with-https-urls for information on currently recommended modes of authentication.
fatal: Authentication failed for 'https://github.com/Suvrat1629/logging-log4j2.git/'

Comment on lines +21 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did this end up here? 😉

[In the mean time I am checking if I didn't leave any :x or :wq in the code 😛 ]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well i just shifted to arch and was trying to push to code from intellij and this log ended up here after i pushed the code and i realised it just after :D

</List>
</Appenders>
<Loggers>
Expand Down
85 changes: 42 additions & 43 deletions log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@
@Plugin(name = "ScriptFilter", category = Node.CATEGORY, elementType = Filter.ELEMENT_TYPE, printObject = true)
public final class ScriptFilter extends AbstractFilter {

private static org.apache.logging.log4j.Logger logger = StatusLogger.getLogger();
private static final org.apache.logging.log4j.Logger LOGGER = StatusLogger.getLogger();

private final AbstractScript script;
private AbstractScript script;
private final Configuration configuration;

private ScriptFilter(
Expand All @@ -57,58 +57,67 @@ private ScriptFilter(
this.configuration = configuration;
}

@Override
public void start() {
super.start();
if (script instanceof ScriptRef) {
AbstractScript resolvedScript = configuration.getScriptManager().getScript(script.getName());
if (resolvedScript == null) {
LOGGER.error("No script with name {} has been declared.", script.getName());
// Optionally: mark the filter as unusable or handle failure gracefully
return;
}
this.script = resolvedScript; // Update to resolved script
} else {
if (!configuration.getScriptManager().addScript(script)) {
LOGGER.error("Failed to add script {} to the ScriptManager.", script.getName());
}
}
}
Comment on lines +61 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can not use the same code as in the createFilter factory method: if the script variable is null it will create problems further down in the call hierarchy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so should i just revert back the changes i made to the ScriptFilter.java file and and make changes to the processRootNode method in the AbstractConfiguration and update the logic?
The logic would be:
1.Process first, as it currently does.
2.Process script-related components (ScriptPatternSelector, Routes, ScriptFilter, etc.) after to ensure their dependencies are resolved.
3.Process all remaining components afterward.

is this how i should keep my approach?

Also thanks for your patience and guidance :)


@Override
public Result filter(
final Logger logger, final Level level, final Marker marker, final String msg, final Object... params) {
final SimpleBindings bindings = new SimpleBindings();
bindings.put("logger", logger);
bindings.put("level", level);
bindings.put("marker", marker);
bindings.put("message", new SimpleMessage(msg));
bindings.put("parameters", params);
bindings.put("throwable", null);
bindings.putAll(configuration.getProperties());
bindings.put("substitutor", configuration.getStrSubstitutor());
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
return executeScript(logger, level, marker, new SimpleMessage(msg), params, null);
}

@Override
public Result filter(
final Logger logger, final Level level, final Marker marker, final Object msg, final Throwable t) {
final SimpleBindings bindings = new SimpleBindings();
bindings.put("logger", logger);
bindings.put("level", level);
bindings.put("marker", marker);
bindings.put("message", msg instanceof String ? new SimpleMessage((String) msg) : new ObjectMessage(msg));
bindings.put("parameters", null);
bindings.put("throwable", t);
bindings.putAll(configuration.getProperties());
bindings.put("substitutor", configuration.getStrSubstitutor());
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
Message message = msg instanceof String ? new SimpleMessage((String) msg) : new ObjectMessage(msg);
return executeScript(logger, level, marker, message, null, t);
}

@Override
public Result filter(
final Logger logger, final Level level, final Marker marker, final Message msg, final Throwable t) {
return executeScript(logger, level, marker, msg, null, t);
}

@Override
public Result filter(final LogEvent event) {
final SimpleBindings bindings = new SimpleBindings();
bindings.put("logger", logger);
bindings.put("level", level);
bindings.put("marker", marker);
bindings.put("message", msg);
bindings.put("parameters", null);
bindings.put("throwable", t);
bindings.put("logEvent", event);
bindings.putAll(configuration.getProperties());
bindings.put("substitutor", configuration.getStrSubstitutor());
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
return object == null || !Boolean.TRUE.equals(object) ? onMismatch : onMatch;
}

@Override
public Result filter(final LogEvent event) {
private Result executeScript(
final Logger logger,
final Level level,
final Marker marker,
final Message msg,
final Object[] params,
final Throwable t) {
final SimpleBindings bindings = new SimpleBindings();
bindings.put("logEvent", event);
bindings.put("logger", logger);
bindings.put("level", level);
bindings.put("marker", marker);
bindings.put("message", msg);
bindings.put("parameters", params);
bindings.put("throwable", t);
bindings.putAll(configuration.getProperties());
bindings.put("substitutor", configuration.getStrSubstitutor());
final Object object = configuration.getScriptManager().execute(script.getName(), bindings);
Expand Down Expand Up @@ -145,16 +154,6 @@ public static ScriptFilter createFilter(
LOGGER.error("Script support is not enabled");
return null;
}
if (script instanceof ScriptRef) {
if (configuration.getScriptManager().getScript(script.getName()) == null) {
logger.error("No script with name {} has been declared.", script.getName());
return null;
}
} else {
if (!configuration.getScriptManager().addScript(script)) {
return null;
}
}

return new ScriptFilter(script, configuration, match, mismatch);
}
Expand Down