apache / apache/logging-log4j2

DefaultMergeStrategy - problem merging filters

Open
#3,173 6 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
3.6k
Forks
1.7k
Avg merge
21h 30m
Merged PRs (30d)
27

Description

DefaultMergeStrategy (Log4j 2.24.1)

In `DefaultMergeStrategy#updateFilterNode` I *think* there is a problem when merging two configurations with filters and the target does not *yet* contain a `CompositeFilter`. (in the code below this is the `else`case).
```
private void updateFilterNode(
final Node target,
final Node targetChildNode,
final Node sourceChildNode,
final PluginManager pluginManager) {
if (CompositeFilter.class.isAssignableFrom(targetChildNode.getType().getPluginClass())) {
final Node node = new Node(targetChildNode, sourceChildNode.getName(), sourceChildNode.getType());
node.getChildren().addAll(sourceChildNode.getChildren());
node.getAttributes().putAll(sourceChildNode.getAttributes());
targetChildNode.getChildren().add(node);
} else {
final PluginType pluginType = pluginManager.getPluginType(FILTERS);
final Node filtersNode = new Node(targetChildNode, FILTERS, pluginType);
final Node node = new Node(filtersNode, sourceChildNode.getName(), sourceChildNode.getType());
node.getAttributes().putAll(sourceChildNode.getAttributes());
final List children = filtersNode.getChildren();
children.add(targetChildNode);
children.add(node);
final List nodes = target.getChildren();
nodes.remove(targetChildNode);
nodes.add(filtersNode);
}
}
```

If I am not mistaken, there is a step missing to add the children of the 'sourceChildNode' to the new `node`.

I think it should be (see line commented with "`<==== ADDED`").

```
else {
final PluginType pluginType = pluginManager.getPluginType(FILTERS);
final Node filtersNode = new Node(targetChildNode, FILTERS, pluginType);
final Node node = new Node(filtersNode, sourceChildNode.getName(), sourceChildNode.getType());
node.getChildren().addAll(sourceChildeNode.getChildren()); // <==== ADDED
node.getAttributes().putAll(sourceChildNode.getAttributes());
final List children = filtersNode.getChildren();
children.add(targetChildNode);
children.add(node);
final List nodes = target.getChildren();
nodes.remove(targetChildNode);
nodes.add(filtersNode);
}
}
```

For example if merging a secondary configuration (source) containing the following filter to a base configuration (target) when the target contains a filter (but *not* a `CompositeFilter`):

```




```

I *believe* all the `KeyValuePair` children would be dropped during merge with the current code.

In addition, the current code does not account for the possibilty of the `sourceChildNode` being a `CompositeFilter` which would result in nested composite-filters. (this applies to both the `if`and `else`branches of the original code.

```







```
Would result in:
```








```
But should probably be:
```






```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.