apache / apache/logging-log4j2
DefaultMergeStrategy - problem merging filters
- 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.