Removed @PluginAttribute 'patternFlags' argument from @PluginFactory for RegexFilter. (#3086) by JWT007 · Pull Request #3512 · apache/logging-log4j2

JWT007 · 2025-03-02T20:26:47Z

Removed @PluginAttribute 'patternFlags' argument from https://github.com/pluginfactory for RegexFilter.

Actually created new https://github.com/pluginfactory without the argument and deprecated old 'createFilter' method.

See ticket for discussion of the reasoning - short version - pattern flags can be passed as part of RegEx expression.

Also guarded the "PatternCompile" method against excepttions - logging and returning null if the pattern fails to compile.

Recreated from 2.x and superceding PR #3463

… createFilter. (apache#3086)

+ made AbstractFiltter.AbstractFilterBuilder onMatch/onMismatch fields protected + added AbstractFilter(AbstractFilterBuilder) constructor + added RegexFilter.Builder implementation + added RegexFilter(Builder) constructor + moved RegexFilter Pattern compile into constructor + added fields to persist configuration propertties + getters (regexExpression, patternFlags) + changed private constructor to accept builder as argument + renamed private method 'targetMessageTest' to more approprriate 'getMessageTextByType' + added Javadoc + grouped deprecations

+ added validation checks to RegexFilter + added JVerify nullability annotations to RegexFilter + updated javadoc + replaced deprecated usages of CompositeFilter#getFilters with CompositeFilter#getFiltersArray in AbstractFilterableTest

JWT007 · 2025-03-02T20:28:25Z

New branch based on 2.x branch - replacing PR #3463

github-actions · 2025-03-02T20:53:09Z

Job	Requested goals	Build Tool Version	Build Outcome
build-macos-latest	clean install	3.9.8	✅
build-ubuntu-latest	clean install	3.9.8	✅
build-windows-latest	clean install	3.9.8	✅

Generated by gradle/develocity-actions

vy

Thanks so much for the hard work @JWT007! Dropped some remarks.

vy · 2025-03-14T08:19:43Z

    public Result filter(final LogEvent event) {
-        final String text = targetMessageTest(event.getMessage());
-        return filter(text);
+        Objects.requireNonNull(event, "The 'event' argument must not be null.");


Suggested change

Objects.requireNonNull(event, "The 'event' argument must not be null.");

Objects.requireNonNull(event, "event");

vy · 2025-03-14T08:22:05Z

+         * The regular expression to match.
+         */
+        @PluginBuilderAttribute
+        @Required(message = "No 'regex' provided for RegexFilter")


Please don't provide a failure message for cases where the failure system should be providing the necessary context.

Suggested change

@Required(message = "No 'regex' provided for RegexFilter")

@Required

vy · 2025-03-14T08:24:11Z

+        /** {@inheritDoc} */
+        @Override
+        public boolean isValid() {
+            return (Strings.isNotEmpty(this.regex));


Suggested change

return (Strings.isNotEmpty(this.regex));

return Strings.isNotEmpty(this.regex);

vy · 2025-03-14T08:26:04Z

+        public @Nullable RegexFilter build() {
+
+            // validate the "regex" attribute
+            if (Strings.isEmpty(this.regex)) {
+                LOGGER.error("Unable to create RegexFilter: The 'regex' attribute be set to a non-empty String.");
+                return null;
+            }
+
+            // build with *safety* to not throw exceptions
+            try {
+                return new RegexFilter(this);
+            } catch (final Exception ex) {
+                LOGGER.error("Unable to create RegexFilter. {}", ex.getMessage(), ex);
+                return null;
+            }
        }


We had a dev@ discussion about this, and the conclusion is to throw exception on failures while building components. Would you mind updating this build() logic accordingly, please?

vy · 2025-03-14T08:28:14Z

+     * @deprecated pattern flags have been deprecated - they can just be included in the regex-expression.
+     */
+    @Deprecated
+    private static final int DEFAULT_PATTERN_FLAGS = 0;


AFAIU, this constant is only used in toPatternFlags(), could you move it into that method, please?

vy · 2025-03-14T08:29:18Z

+    @Deprecated
+    public String[] getPatternFlags() {
+        return this.patternFlags.clone();
    }


We're adding a new deprecated method? Why don't we simply not add it?

vy · 2025-03-14T08:30:03Z

-                Boolean.TRUE.equals(useRawMsg), Pattern.compile(regex, toPatternFlags(patternFlags)), match, mismatch);
+
+        // LOG4J-3086 - pattern-flags can be embedded in RegEx expression
+        Objects.requireNonNull(regex, "The 'regex' argument must not be null.");


Suggested change

Objects.requireNonNull(regex, "The 'regex' argument must not be null.");

Objects.requireNonNull(regex, "regex");

ppkarwasz · 2025-03-05T12:40:17Z

+         */
        @PluginBuilderAttribute(ATTR_ON_MATCH)
-        private Result onMatch = Result.NEUTRAL;
+        protected Result onMatch = Result.NEUTRAL;


There is already a getOnMatch(), no need to expose the field too.

ppkarwasz · 2025-03-05T12:40:32Z

+         */
        @PluginBuilderAttribute(ATTR_ON_MISMATCH)
-        private Result onMismatch = Result.DENY;
+        protected Result onMismatch = Result.DENY;


There is already a getOnMismatch(), no need to expose the field too.

vy · 2026-06-11T15:46:41Z

I've resurrected this in #4119. Thanks so much for the ground work @JWT007, and apologies for the late processing.

jethomas-tsi and others added 7 commits March 2, 2025 21:11

Removed 'patternFlags' @PluginAttribute from RegexFilter @pluginfactory…

d40639e

… createFilter. (apache#3086)

A few more improvements + added tests (apache#3086)

7ef0c54

Fixed a reverse logic check in a validation (apache#3086)

4463a82

Fix Spotless errors (apache#3086)

4439b08

Fix RegexFilterTest (apache#3086)

33dfec3

JWT007 mentioned this pull request Mar 2, 2025

Removed @PluginAttribute 'patternFlags' argument from @PluginFactory for RegexFilter. (#3086) #3463

Closed

JWT007 requested a review from ppkarwasz March 2, 2025 20:28

JWT007 self-assigned this Mar 2, 2025

JWT007 added bug Incorrect, unexpected, or unintended behavior of existing code java Pull requests that update Java code configuration Affects the configuration system in a general way labels Mar 2, 2025

JWT007 linked an issue Mar 2, 2025 that may be closed by this pull request

Fix RegexFilter pattern flags #3086

Closed

vy requested changes Mar 14, 2025

View reviewed changes

ppkarwasz added this to Log4j bug tracker Apr 3, 2025

github-project-automation Bot moved this to To triage in Log4j bug tracker Apr 3, 2025

ppkarwasz added this to the 2.25.0 milestone Apr 13, 2025

ppkarwasz requested changes May 16, 2025

View reviewed changes

ppkarwasz modified the milestones: 2.25.0, 2.25.1 May 25, 2025

ppkarwasz removed this from the 2.25.1 milestone Jun 21, 2025

vy added a commit to ramanathan1504/logging-log4j2 that referenced this pull request Jun 11, 2026

Apply review feedback shared in apache#3512

88fdae8

vy closed this Jun 11, 2026

github-project-automation Bot moved this from To triage to Done in Log4j bug tracker Jun 11, 2026

	Objects.requireNonNull(event, "The 'event' argument must not be null.");
	Objects.requireNonNull(event, "event");

	@Required(message = "No 'regex' provided for RegexFilter")
	@Required

	return (Strings.isNotEmpty(this.regex));
	return Strings.isNotEmpty(this.regex);

	Objects.requireNonNull(regex, "The 'regex' argument must not be null.");
	Objects.requireNonNull(regex, "regex");

Uh oh!

Conversation

JWT007 commented Mar 2, 2025

Uh oh!

JWT007 commented Mar 2, 2025

Uh oh!

github-actions Bot commented Mar 2, 2025

Generated by gradle/develocity-actions

Uh oh!

vy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vy commented Jun 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants