apache / apache/logging-log4j2

DefaultMergeStrategy - not accounting for AsyncWaitStrategyFactory when merging configurations

Open
#3,172 4 comments 0 reactions 0 assignees View on GitHub
waiting-for-maintainer
Dominant language
Java
Stars
3.6k
Forks
1.7k
Avg merge
21h 30m
Merged PRs (30d)
27

Description

DefaultMergeStrategy (Log4j 2.24.1)

I *think* I may have encountered a potential error if using a CompositeConfiguration.

In the `DefaultMergeStrategy#mergeConfigurations`, it does a double loop over the source and target node's children - merging from the source into the target.

In these loops it provides special handling for PROPERTIES, SCRIPTS, APPENDERS, FILTERS and LOGGERS.

If the element tag name does not match one of these, the default behaviour is to add the children of the source element to the corresponding target element.
```
switch (toRootLowerCase(targetChildNode.getName())) {
case PROPERTIES:
case SCRIPTS:
case APPENDERS: { ... }
case LOGGERS: { ... }
default: {
targetChildNode.getChildren().addAll(sourceChildNode.getChildren());
isMerged = true;
break;
}
}
```

If however, two configurations had an `AsyncWaitStrategyFactory` element:
```

```
This element has no children, and the attributes are not merged.

The 'class' attribute after merging would be taken from the first occurrence `a.b.c.Foo` and not from the *override* 'd.e.f.Bar`.

I'll admit I have not tested this scenario, but reviewing the code it *logically* seems like it might cause a problem

NOTE: "Properties", "Scripts", "Appenders", "Filter" and "Loggers" are all collection elements and have no attributes - so it makes sense that for these no attributes are merged, but the default behavior should IMHO handle the possiblity of attributes and perform a merge of the element's attributes similar to that of the `DefaultMergeStrategy#mergeRootProperties` method.

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.