Skip to content

Fix properties config parsing for MarkerFilter onMismatch/onMatch attribute casing (Fixes #2791)#4183

Open
LuckysHorizon wants to merge 2 commits into
apache:2.xfrom
LuckysHorizon:fix-2791-properties-filter-case
Open

Fix properties config parsing for MarkerFilter onMismatch/onMatch attribute casing (Fixes #2791)#4183
LuckysHorizon wants to merge 2 commits into
apache:2.xfrom
LuckysHorizon:fix-2791-properties-filter-case

Conversation

@LuckysHorizon

Copy link
Copy Markdown

This PR fixes a properties-configuration parsing compatibility issue reported in #2791.

In .properties configs, MarkerFilter attributes onMatch / onMismatch were treated with strict key matching in PropertiesConfigurationBuilder, which could leave mis-cased keys (for example onMisMatch) unconsumed and produce invalid attribute errors at runtime.

What changed

  1. log4j-core

    • Updated PropertiesConfigurationBuilder#createFilter(...) to resolve onMatch and onMismatch in a case-insensitive way before building the filter.
  2. log4j-core-test

    • Added regression config:
      log4j2-properties-markerfilter-miscase.properties
    • Added regression test:
      MarkerFilterPropertiesCaseTest
    • Test validates that configuration starts successfully with a mis-cased onMisMatch property, preventing regression.

Why

This restores expected compatibility for properties-based MarkerFilter configuration and avoids startup/runtime errors caused by minor attribute casing differences.

Fixes #2791.

How to verify this change (add to PR description or comment)

How to verify

  1. Use Java 17.
  2. From repository root:
  3. Expected result:
    •  MarkerFilterPropertiesCaseTest  passes.
    • Configuration file with  onMisMatch  is accepted and configuration starts.
  4. (Optional full validation)

LuckysHorizon and others added 2 commits July 19, 2026 20:15
…utes (onMatch/onMismatch) and add failing test for mis-cased property

Addresses apache#2791

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ramanathan1504

Copy link
Copy Markdown
Contributor

@LuckysHorizon

Thanks for opening this PR for #2791. I wanted to give you a heads-up on what's been happening in parallel:

Context:
There's already been work on this issue across multiple PRs:

Suggestion:
Since PR #4116 is nearly complete (most feedback addressed, just final touches needed), and it touches a lot of surface area (~60 files), I'd recommend we consolidate effort there rather than split review across multiple PRs.

Would you be open to collaborating on clearing the last few comments on #4116? That way we avoid duplicate review work and get to the finish line faster. If you'd like to contribute, I can brief you on the pending feedback and we can tackle it together.

Let me know your thoughts!

@LuckysHorizon

Copy link
Copy Markdown
Author

@LuckysHorizon

Thanks for opening this PR for #2791. I wanted to give you a heads-up on what's been happening in parallel:

Context: There's already been work on this issue across multiple PRs:

Suggestion: Since PR #4116 is nearly complete (most feedback addressed, just final touches needed), and it touches a lot of surface area (~60 files), I'd recommend we consolidate effort there rather than split review across multiple PRs.

Would you be open to collaborating on clearing the last few comments on #4116? That way we avoid duplicate review work and get to the finish line faster. If you'd like to contribute, I can brief you on the pending feedback and we can tackle it together.

Let me know your thoughts!

Thanks for the context and for the heads-up!

I wasn't aware that #4116 had already progressed this far. My intention with this PR was to provide a focused fix for the compatibility issue reported in #2791 and add a regression test to prevent it from recurring.

I'm happy to avoid duplicating effort. If #4116 is the preferred direction and is close to merging, I'd be glad to help address the remaining review comments there instead. Feel free to point me to the outstanding feedback or any specific areas where I can help.

Thanks again for letting me know!

@ramanathan1504

Copy link
Copy Markdown
Contributor

@LuckysHorizon,

Thanks for understanding! There are currently 3 to 4 pending review comments on PR #4116. Could you please take a look and address them? Before jumping in, please make sure to check the related PR I mentioned earlier, particularly the reviews and comments from @ppkarwasz and @vy.

When making the updates, please handle them comment by comment. For example, if a single review comment contains 3 separate points, please address all 3 points in a single commit, and then reply to the comment with that specific commit ID. I've already done this for two of the comments, so you can check the PR to see how it looks.

This approach makes it much easier for @vy to review, especially since this PR touches so many files. If you're comfortable with this workflow, feel free to go ahead! Otherwise, let me know and I can manage it.

Regarding your earlier question: We have plenty of open issues in our backlog, so feel free to pick and choose what you'd like to work on next. Just please make sure to double-check that there isn't already an open PR for an issue before you start working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Fix handling of onMatch and onMismatch attributes in the properties configuration format

2 participants