DefaultMergeStrategy - problem merging filters

オープン
  • 何か問題がありますか？
#3,173 コメント 6 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
35/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
停滞
技術スタック
java
領域
backend

調査の方向性

DefaultMergeStrategy#updateFilterNode から始め、issue の XML 設定例を使って 2 つの merge 分岐を比較します。ソースフィルターの子要素が保持されるか、また複合フィルターがネストされるのではなくフラット化されるかを確認します。マージされた設定が、期待される複合構造を持つすべてのフィルターおよび KeyValuePair の子要素を保持していれば完了です。

索引モデルが issue の本文から書いたものです。

説明

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 elsecase).

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<Node> children = filtersNode.getChildren();
        children.add(targetChildNode);
        children.add(node);
        final List<Node> 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<Node> children = filtersNode.getChildren();
        children.add(targetChildNode);
        children.add(node);
        final List<Node> 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):

<Configuration>
  <DynamicThresholdFilter key="loginId" defaultThreshold="ERROR"> 
    <KeyValuePair key="alice" value="DEBUG"/> 
    <KeyValuePair key="bob" value="INFO"/> 
  </DynamicThresholdFilter>
</Configuration>

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 ifand elsebranches of the original code.

<Configuration>
  <CompositeFilter>
     <Filter1/>
     <Filter2/>
  </CompositeFilter>
</Configuration>
<Configuration>
  <CompositeFilter>
     <Filter3/>
     <Filter4/>
  </CompositeFilter>
</Configuration>

Would result in:

<Configuration>
  <CompositeFilter>
     <Filter1/>
     <Filter2/>
     <CompositeFilter>
       <Filter3/>
       <Filter4/>
    </CompositeFilter>
  </CompositeFilter>
</Configuration>

But should probably be:

<Configuration>
  <CompositeFilter>
     <Filter1/>
     <Filter2/>
     <Filter3/>
     <Filter4/>
  </CompositeFilter>
</Configuration>
主要言語
Java
スター
3.6k
フォーク
1.8k
平均マージ
5日 6時間
マージ済み PR（30日）
24

環境構築

  • Dockerfile・Docker Compose ファイルなし
  • プルリクエストのテンプレートあり
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

apache/logging-log4j2 のほかの issue

apache/logging-log4j2 の issue をすべて見る

似ている issue

Java の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。