From d40639e8f5fc72d9788d796d766ee44f44bd537f Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Wed, 12 Feb 2025 14:07:22 +0100 Subject: [PATCH 1/7] Removed 'patternFlags' @PluginAttribute from RegexFilter @PluginFactory createFilter. (#3086) --- .../log4j/core/filter/RegexFilter.java | 77 +++++++++++-------- ...remove_patternflags_from_PluginFactory.xml | 10 +++ 2 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 src/changelog/2.25.0/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java index 62d41b31f59..84cdbfad47b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java @@ -16,9 +16,6 @@ */ package org.apache.logging.log4j.core.filter; -import java.lang.reflect.Field; -import java.util.Arrays; -import java.util.Comparator; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.logging.log4j.Level; @@ -29,7 +26,6 @@ import org.apache.logging.log4j.core.config.Node; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttribute; -import org.apache.logging.log4j.core.config.plugins.PluginElement; import org.apache.logging.log4j.core.config.plugins.PluginFactory; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFormatMessage; @@ -43,7 +39,6 @@ @Plugin(name = "RegexFilter", category = Node.CATEGORY, elementType = Filter.ELEMENT_TYPE, printObject = true) public final class RegexFilter extends AbstractFilter { - private static final int DEFAULT_PATTERN_FLAGS = 0; private final Pattern pattern; private final boolean useRawMessage; @@ -110,10 +105,7 @@ private Result filter(final String msg) { @Override public String toString() { - final StringBuilder sb = new StringBuilder(); - sb.append("useRaw=").append(useRawMessage); - sb.append(", pattern=").append(pattern.toString()); - return sb.toString(); + return "useRaw=" + useRawMessage + ", pattern=" + pattern.toString(); } /** @@ -123,6 +115,40 @@ public String toString() { * The regular expression to match. * @param patternFlags * An array of Strings where each String is a {@link Pattern#compile(String, int)} compilation flag. + * (no longer used - pattern flags can be embedded in regex-expression. + * @param useRawMsg + * If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage}, and {@link MessageFormatMessage}, the message format pattern; for {@link StructuredDataMessage}, the message field will be used as the match target. + * @param match + * The action to perform when a match occurs. + * @param mismatch + * The action to perform when a mismatch occurs. + * @return The RegexFilter. + * @throws IllegalAccessException When there is no access to the definition of the specified member. + * @throws IllegalArgumentException When passed an illegal or inappropriate argument. + * @deprecated use {@link #createFilter(String, Boolean, Result, Result)} + */ + @Deprecated + // TODO Consider refactoring to use AbstractFilter.AbstractFilterBuilder + public static RegexFilter createFilter( + // @formatter:off + @PluginAttribute("regex") final String regex, + final String[] patternFlags, + @PluginAttribute("useRawMsg") final Boolean useRawMsg, + @PluginAttribute("onMatch") final Result match, + @PluginAttribute("onMismatch") final Result mismatch) + // @formatter:on + throws IllegalArgumentException, IllegalAccessException { + + // LOG4J-3086 - pattern-flags can be embedded in RegEx expression + + return createFilter(regex, useRawMsg, match, mismatch); + } + + /** + * Creates a Filter that matches a regular expression. + * + * @param regex + * The regular expression to match. * @param useRawMsg * If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage}, and {@link MessageFormatMessage}, the message format pattern; for {@link StructuredDataMessage}, the message field will be used as the match target. * @param match @@ -138,40 +164,23 @@ public String toString() { public static RegexFilter createFilter( // @formatter:off @PluginAttribute("regex") final String regex, - @PluginElement("PatternFlags") final String[] patternFlags, @PluginAttribute("useRawMsg") final Boolean useRawMsg, @PluginAttribute("onMatch") final Result match, @PluginAttribute("onMismatch") final Result mismatch) // @formatter:on throws IllegalArgumentException, IllegalAccessException { + boolean raw = Boolean.TRUE.equals(useRawMsg); if (regex == null) { LOGGER.error("A regular expression must be provided for RegexFilter"); return null; } - return new RegexFilter( - Boolean.TRUE.equals(useRawMsg), Pattern.compile(regex, toPatternFlags(patternFlags)), match, mismatch); - } - - private static int toPatternFlags(final String[] patternFlags) - throws IllegalArgumentException, IllegalAccessException { - if (patternFlags == null || patternFlags.length == 0) { - return DEFAULT_PATTERN_FLAGS; - } - final Field[] fields = Pattern.class.getDeclaredFields(); - final Comparator comparator = (f1, f2) -> f1.getName().compareTo(f2.getName()); - Arrays.sort(fields, comparator); - final String[] fieldNames = new String[fields.length]; - for (int i = 0; i < fields.length; i++) { - fieldNames[i] = fields[i].getName(); - } - int flags = DEFAULT_PATTERN_FLAGS; - for (final String test : patternFlags) { - final int index = Arrays.binarySearch(fieldNames, test); - if (index >= 0) { - final Field field = fields[index]; - flags |= field.getInt(Pattern.class); - } + final Pattern pattern; + try { + pattern = Pattern.compile(regex); + } catch (final Exception ex) { + LOGGER.error("Unable to compile regular expression: {}", regex, ex); + return null; } - return flags; + return new RegexFilter(raw, pattern, match, mismatch); } } diff --git a/src/changelog/2.25.0/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml b/src/changelog/2.25.0/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml new file mode 100644 index 00000000000..0e61653f85c --- /dev/null +++ b/src/changelog/2.25.0/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml @@ -0,0 +1,10 @@ + + + + + Removed 'patternFlags' @PluginAttribute from RegexFilter @PluginFactory createFilter. + + From 6063bba4a23ed28d7586cea4188bb4aef98534f4 Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Mon, 17 Feb 2025 17:05:26 +0100 Subject: [PATCH 2/7] Updates per PR Code Review (#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 --- .../log4j/core/filter/RegexFilterTest.java | 11 +- .../log4j/core/filter/AbstractFilter.java | 33 +- .../log4j/core/filter/RegexFilter.java | 358 ++++++++++++++---- .../logging/log4j/core/util/Builder.java | 4 + ...remove_patternflags_from_PluginFactory.xml | 0 5 files changed, 328 insertions(+), 78 deletions(-) rename src/changelog/{2.25.0 => .2.x.x}/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml (100%) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java index 671d998258b..e5801342dd6 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java @@ -19,6 +19,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -43,13 +44,17 @@ static void before() { @Test void testRegexFilterDoesNotThrowWithAllTheParametersExceptRegexEqualNull() { assertDoesNotThrow(() -> { - RegexFilter.createFilter(".* test .*", null, null, null, null); + RegexFilter.newBuilder().setRegex(".* test .*").build(); }); } @Test void testThresholds() throws Exception { - RegexFilter filter = RegexFilter.createFilter(".* test .*", null, false, null, null); + RegexFilter filter = RegexFilter.newBuilder() + .setRegex(".* test .*") + .setUseRawMsg(false) + .build(); + assertNotNull(filter); filter.start(); assertTrue(filter.isStarted()); assertSame( @@ -65,7 +70,7 @@ void testThresholds() throws Exception { .setMessage(new SimpleMessage("test")) // .build(); assertSame(Filter.Result.DENY, filter.filter(event)); - filter = RegexFilter.createFilter(null, null, false, null, null); + filter = RegexFilter.newBuilder().build(); assertNull(filter); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilter.java index 397390bcbc3..05b1e6b275b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilter.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.core.filter; +import java.util.Objects; +import java.util.Optional; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.core.AbstractLifeCycle; @@ -43,16 +45,30 @@ public abstract static class AbstractFilterBuilder builder) { + + Objects.requireNonNull(builder, "The 'builder' argument cannot be null."); + + this.onMatch = Optional.ofNullable(builder.onMatch).orElse(Result.NEUTRAL); + this.onMismatch = Optional.ofNullable(builder.onMismatch).orElse(Result.DENY); + } + @Override protected boolean equalsImpl(final Object obj) { if (this == obj) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java index 84cdbfad47b..d26d157bcc7 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java @@ -16,7 +16,10 @@ */ package org.apache.logging.log4j.core.filter; -import java.util.regex.Matcher; +import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.Comparator; +import java.util.Objects; import java.util.regex.Pattern; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; @@ -26,7 +29,9 @@ import org.apache.logging.log4j.core.config.Node; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttribute; -import org.apache.logging.log4j.core.config.plugins.PluginFactory; +import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; +import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory; +import org.apache.logging.log4j.core.config.plugins.PluginElement; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFormatMessage; import org.apache.logging.log4j.message.ParameterizedMessage; @@ -39,53 +44,132 @@ @Plugin(name = "RegexFilter", category = Node.CATEGORY, elementType = Filter.ELEMENT_TYPE, printObject = true) public final class RegexFilter extends AbstractFilter { + /** The regular-expression. */ + private final String regex; + + /** The pattern compiled from the regular-expression. */ private final Pattern pattern; + + /** Flag: if {@code true} use message format-pattern / field for the match target. */ private final boolean useRawMessage; - private RegexFilter(final boolean raw, final Pattern pattern, final Result onMatch, final Result onMismatch) { - super(onMatch, onMismatch); - this.pattern = pattern; - this.useRawMessage = raw; + /** + * Constructs a new {@code RegexFilter} configured by the given builder. + * @param builder the builder + * @throws IllegalArgumentException if the regular expression cannot be compiled to a pattern + */ + private RegexFilter(final Builder builder) { + + super(builder); + + this.regex = builder.regex; + this.useRawMessage = Boolean.TRUE.equals(builder.useRawMsg); + + try { + this.pattern = Pattern.compile(regex); + } catch (final Exception ex) { + throw new IllegalArgumentException("Unable to compile regular expression: '" + regex + "'.", ex); + } + } + + /** + * Returns the regular-expression. + * @return the regular-expression (it may be an empty string but never {@code null}) + */ + public String getRegex() { + return this.regex; + } + + /** + * Returns the compiled regular-expression pattern. + * @return the pattern (will never be {@code null} + */ + public Pattern getPattern() { + return this.pattern; + } + + /** + * Returns whether the raw-message should be used. + * @return {@code} if the raw message should be used; otherwise, {@code false} + */ + public boolean isUseRawMessage() { + return this.useRawMessage; } + /** {@inheritDoc} */ @Override public Result filter( final Logger logger, final Level level, final Marker marker, final String msg, final Object... params) { - if (useRawMessage || params == null || params.length == 0) { - return filter(msg); - } - return filter(ParameterizedMessage.format(msg, params)); + return (useRawMessage || params == null || params.length == 0) + ? filter(msg) + : filter(ParameterizedMessage.format(msg, params)); } + /** {@inheritDoc} */ @Override public Result filter( final Logger logger, final Level level, final Marker marker, final Object msg, final Throwable t) { - if (msg == null) { - return onMismatch; - } - return filter(msg.toString()); + return (msg == null) ? this.onMismatch : filter(msg.toString()); } + /** {@inheritDoc} */ @Override public Result filter( final Logger logger, final Level level, final Marker marker, final Message msg, final Throwable t) { if (msg == null) { return onMismatch; } - final String text = targetMessageTest(msg); - return filter(text); + return filter(getMessageTextByType(msg)); } + /** {@inheritDoc} */ @Override public Result filter(final LogEvent event) { - final String text = targetMessageTest(event.getMessage()); - return filter(text); + return filter(getMessageTextByType(event.getMessage())); + } + + /** + * Apply the filter to the given message and return the match/mismatch result. + *

+ * If the given '{@code msg}' is {@code null} the configured mismatch result will be returned. + *

+ * @param msg the message + * @return the filter result + */ + private Result filter(final String msg) { + if (msg == null) { + return onMismatch; + } + return pattern.matcher(msg).matches() ? onMatch : onMismatch; } - // While `Message#getFormat()` is broken in general, it still makes sense for certain types. - // Hence, suppress the deprecation warning. + /** + * Tests the filter pattern against the given Log4j {@code Message}. + *

+ * If the raw-message flag is enabled and message is an instance of the following, the raw message format + * will be returned. + *

+ *
    + *
  • {@link MessageFormatMessage}
    • + *
  • {@link ParameterizedMessage}
    • + *
  • {@link StringFormattedMessage}
    • + *
  • {@link StructuredDataMessage}
    • + *
+ *

+ * If the '{@code useRawMessage}' flag is disabled OR the message is not one of the above + * implementations, the message's formatted message will be returned. + *

+ *

Developer Note

+ *

+ * While `Message#getFormat()` is broken in general, it still makes sense for certain types. + * Hence, suppress the deprecation warning. + *

+ * + * @param message the message + * @return the target message based on configuration and message-type + */ @SuppressWarnings("deprecation") - private String targetMessageTest(final Message message) { + private String getMessageTextByType(final Message message) { return useRawMessage && (message instanceof ParameterizedMessage || message instanceof StringFormattedMessage @@ -95,92 +179,220 @@ private String targetMessageTest(final Message message) { : message.getFormattedMessage(); } - private Result filter(final String msg) { - if (msg == null) { - return onMismatch; - } - final Matcher m = pattern.matcher(msg); - return m.matches() ? onMatch : onMismatch; - } - @Override public String toString() { - return "useRaw=" + useRawMessage + ", pattern=" + pattern.toString(); + return "useRawMessage=" + useRawMessage + ", regex=" + regex + ", pattern=" + pattern.toString(); } /** - * Creates a Filter that matches a regular expression. + * Creates a new builder instance. * - * @param regex - * The regular expression to match. - * @param patternFlags - * An array of Strings where each String is a {@link Pattern#compile(String, int)} compilation flag. - * (no longer used - pattern flags can be embedded in regex-expression. - * @param useRawMsg - * If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage}, and {@link MessageFormatMessage}, the message format pattern; for {@link StructuredDataMessage}, the message field will be used as the match target. - * @param match - * The action to perform when a match occurs. - * @param mismatch - * The action to perform when a mismatch occurs. - * @return The RegexFilter. - * @throws IllegalAccessException When there is no access to the definition of the specified member. - * @throws IllegalArgumentException When passed an illegal or inappropriate argument. - * @deprecated use {@link #createFilter(String, Boolean, Result, Result)} + * @return the new builder instance + */ + @PluginBuilderFactory + public static RegexFilter.Builder newBuilder() { + return new RegexFilter.Builder(); + } + + /** + * A {@link RegexFilter} builder instance. + */ + public static final class Builder extends AbstractFilterBuilder + implements org.apache.logging.log4j.core.util.Builder { + + /* NOTE: LOG4J-3086 - No patternFlags in builder - this functionality has been deprecated/removed. */ + + /** + * The regular expression to match. + */ + @PluginBuilderAttribute + private String regex; + + /** + * If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage}, + * and {@link MessageFormatMessage}, the message format pattern; for {@link StructuredDataMessage}, + * the message field will be used as the match target. + */ + @PluginBuilderAttribute + private Boolean useRawMsg; + + /** + * Private constructor. + */ + private Builder() { + super(); + } + + /** + * Sets the regular-expression. + * + * @param regex the regular-expression + * @return this builder + */ + public Builder setRegex(final String regex) { + this.regex = regex; + return this; + } + + /** + * Sets the use raw msg flag. + * + * @param useRawMsg {@code true} if the message format-patter/field will be used as match target; + * otherwise, {@code false} + * @return this builder + */ + public Builder setUseRawMsg(final boolean useRawMsg) { + this.useRawMsg = useRawMsg; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public boolean isValid() { + return (regex != null); + } + + /** + * Builds and returns a {@link RegexFilter} instance configured by this builder. + * + * @return the created {@link RegexFilter} or {@code null} if the builder is misconfigured + */ + @Override + public RegexFilter build() { + + if (!isValid()) { + return null; + } + + try { + return new RegexFilter(this); + } catch (final Exception ex) { + LOGGER.error("Unable to create RegexFilter. {}", ex.getMessage(), ex); + return null; + } + } + } + + /* + * DEPRECATIONS: + * The constructor/fields/methods below have been deprecated. + * - the 'create***' factory methods should no longer be used - use the builder instead + * - pattern-flags should now be passed via the regular expression itself + */ + + /** + * @deprecated pattern flags have been deprecated - they can just be included in the regex-expression. */ @Deprecated - // TODO Consider refactoring to use AbstractFilter.AbstractFilterBuilder - public static RegexFilter createFilter( - // @formatter:off - @PluginAttribute("regex") final String regex, - final String[] patternFlags, - @PluginAttribute("useRawMsg") final Boolean useRawMsg, - @PluginAttribute("onMatch") final Result match, - @PluginAttribute("onMismatch") final Result mismatch) - // @formatter:on - throws IllegalArgumentException, IllegalAccessException { + private static final int DEFAULT_PATTERN_FLAGS = 0; - // LOG4J-3086 - pattern-flags can be embedded in RegEx expression + /** + * @deprecated - pattern flags no longer supported. + */ + @Deprecated + private String[] patternFlags = new String[0]; - return createFilter(regex, useRawMsg, match, mismatch); + /** + * @deprecated use {@link RegexFilter.Builder} instead + */ + @Deprecated + @SuppressWarnings("MagicConstant") + private RegexFilter( + final boolean useRawMessage, + final String regex, + final String[] patternFlags, + final Result onMatch, + final Result onMismatch) { + super(onMatch, onMismatch); + this.regex = Objects.requireNonNull(regex, "The 'regex' argument must be provided for RegexFilter"); + this.patternFlags = patternFlags == null ? new String[0] : patternFlags.clone(); + try { + int flags = toPatternFlags(this.patternFlags); + this.pattern = Pattern.compile(regex, flags); + } catch (final Exception ex) { + throw new IllegalArgumentException("Unable to compile regular expression: '" + regex + "'.", ex); + } + this.useRawMessage = useRawMessage; + } + + /** + * Returns the pattern-flags applied to the regular-expression when compiling the pattern. + * + * @return the pattern-flags (maybe empty but never {@code null} + * @deprecated pattern-flags are no longer supported + */ + @Deprecated + public String[] getPatternFlags() { + return this.patternFlags.clone(); } /** * Creates a Filter that matches a regular expression. * - * @param regex - * The regular expression to match. - * @param useRawMsg - * If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage}, and {@link MessageFormatMessage}, the message format pattern; for {@link StructuredDataMessage}, the message field will be used as the match target. - * @param match - * The action to perform when a match occurs. - * @param mismatch - * The action to perform when a mismatch occurs. + * @param regex The regular expression to match. + * @param patternFlags An array of Strings where each String is a {@link Pattern#compile(String, int)} compilation flag. + * (no longer used - pattern flags can be embedded in regex-expression. + * @param useRawMsg If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage}, + * and {@link MessageFormatMessage}, the message format pattern; for {@link StructuredDataMessage}, + * the message field will be used as the match target. + * @param match The action to perform when a match occurs. + * @param mismatch The action to perform when a mismatch occurs. * @return The RegexFilter. - * @throws IllegalAccessException When there is no access to the definition of the specified member. + * @throws IllegalAccessException When there is no access to the definition of the specified member. * @throws IllegalArgumentException When passed an illegal or inappropriate argument. + * @deprecated use {@link #newBuilder} to instantiate builder */ - // TODO Consider refactoring to use AbstractFilter.AbstractFilterBuilder - @PluginFactory + @Deprecated public static RegexFilter createFilter( // @formatter:off @PluginAttribute("regex") final String regex, + @PluginElement("PatternFlags") final String[] patternFlags, @PluginAttribute("useRawMsg") final Boolean useRawMsg, @PluginAttribute("onMatch") final Result match, @PluginAttribute("onMismatch") final Result mismatch) // @formatter:on throws IllegalArgumentException, IllegalAccessException { + + // LOG4J-3086 - pattern-flags can be embedded in RegEx expression + boolean raw = Boolean.TRUE.equals(useRawMsg); if (regex == null) { LOGGER.error("A regular expression must be provided for RegexFilter"); return null; } - final Pattern pattern; + try { - pattern = Pattern.compile(regex); + return new RegexFilter(raw, regex, patternFlags, match, mismatch); } catch (final Exception ex) { - LOGGER.error("Unable to compile regular expression: {}", regex, ex); + LOGGER.error("Unable to create RegexFilter. {}", ex.getMessage(), ex); return null; } - return new RegexFilter(raw, pattern, match, mismatch); + } + + /** @deprecated pattern flags have been deprecated - they can just be included in the regex-expression. */ + @Deprecated + private static int toPatternFlags(final String[] patternFlags) + throws IllegalArgumentException, IllegalAccessException { + if (patternFlags == null || patternFlags.length == 0) { + return DEFAULT_PATTERN_FLAGS; + } + final Field[] fields = Pattern.class.getDeclaredFields(); + final Comparator comparator = (f1, f2) -> f1.getName().compareTo(f2.getName()); + Arrays.sort(fields, comparator); + final String[] fieldNames = new String[fields.length]; + for (int i = 0; i < fields.length; i++) { + fieldNames[i] = fields[i].getName(); + } + int flags = DEFAULT_PATTERN_FLAGS; + for (final String test : patternFlags) { + final int index = Arrays.binarySearch(fieldNames, test); + if (index >= 0) { + final Field field = fields[index]; + flags |= field.getInt(Pattern.class); + } + } + return flags; } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Builder.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Builder.java index 10bc1a9f52e..8a772dc7330 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Builder.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Builder.java @@ -44,6 +44,10 @@ public interface Builder { */ T build(); + /** + * Validates that the builder is properly configured to build. + * @return {@code true} if the builder configuration is valid; otherwise, {@code false} + */ default boolean isValid() { return PluginBuilder.validateFields(this, getErrorPrefix()); } diff --git a/src/changelog/2.25.0/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml b/src/changelog/.2.x.x/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml similarity index 100% rename from src/changelog/2.25.0/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml rename to src/changelog/.2.x.x/3086_change_RegexFilter_remove_patternflags_from_PluginFactory.xml From 7ef0c546ce3e69f8be9e535574f82ec129c2ce5c Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Mon, 17 Feb 2025 17:47:17 +0100 Subject: [PATCH 3/7] A few more improvements + added tests (#3086) --- .../log4j/core/filter/RegexFilterTest.java | 126 +++++++++++++++--- .../log4j/core/filter/RegexFilter.java | 42 ++++-- 2 files changed, 142 insertions(+), 26 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java index e5801342dd6..6cb07a5f7df 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java @@ -19,9 +19,12 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.apache.logging.log4j.Level; @@ -87,9 +90,18 @@ void testDotAllPattern() throws Exception { } @Test - void testNoMsg() throws Exception { - final RegexFilter filter = RegexFilter.createFilter(".* test .*", null, false, null, null); + void testNoMsg() { + + final RegexFilter filter = + RegexFilter.newBuilder() + .setRegex(".* test .*") + .setUseRawMsg(false) + .build(); + + assertNotNull(filter); + filter.start(); + assertTrue(filter.isStarted()); assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, (Object) null, null)); assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, (Message) null, null)); @@ -97,28 +109,112 @@ void testNoMsg() throws Exception { } @Test - void testParameterizedMsg() throws Exception { + void testParameterizedMsg() { final String msg = "params {} {}"; final Object[] params = {"foo", "bar"}; // match against raw message - final RegexFilter rawFilter = RegexFilter.createFilter( - "params \\{\\} \\{\\}", - null, - true, // useRawMsg - Result.ACCEPT, - Result.DENY); + final RegexFilter rawFilter = + RegexFilter.newBuilder() + .setRegex("params \\{\\} \\{\\}") + .setUseRawMsg(true) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY) + .build(); + + assertNotNull(rawFilter); + final Result rawResult = rawFilter.filter(null, null, null, msg, params); assertThat(rawResult, equalTo(Result.ACCEPT)); // match against formatted message - final RegexFilter fmtFilter = RegexFilter.createFilter( - "params foo bar", - null, - false, // useRawMsg - Result.ACCEPT, - Result.DENY); + final RegexFilter fmtFilter = + RegexFilter.newBuilder() + .setRegex("params foo bar") + .setUseRawMsg(false) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY).build(); + + assertNotNull(fmtFilter); + final Result fmtResult = fmtFilter.filter(null, null, null, msg, params); assertThat(fmtResult, equalTo(Result.ACCEPT)); } + + /** + * A builder with no 'regex' expression should both be invalid and return null on 'build()'. + */ + @Test + void testWithValidRegex() { + + final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores + + final RegexFilter.Builder builder = + RegexFilter.newBuilder().setRegex(regex).setUseRawMsg(false).setOnMatch(Result.ACCEPT).setOnMismatch(Result.DENY); + + assertTrue(builder.isValid()); + + final RegexFilter filter = builder.build(); + + assertNotNull(filter); + + assertEquals(Result.ACCEPT, filter.filter("Hello_123")); + + assertEquals(Result.DENY, filter.filter("Hello@123")); + + assertEquals(regex, filter.getRegex()); + } + + @Test + void testRegexFilterGetters() { + + final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores + + final RegexFilter filter = + RegexFilter.newBuilder() + .setRegex(regex) + .setUseRawMsg(false) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY) + .build(); + + assertNotNull(filter); + + assertEquals(regex, filter.getRegex()); + assertFalse(filter.isUseRawMessage()); + assertEquals(Result.ACCEPT, filter.getOnMatch()); + assertEquals(Result.DENY, filter.getOnMismatch()); + assertNotNull(filter.getPattern()); + assertEquals(regex, filter.getPattern().pattern()); + } + + /** + * A builder with no 'regex' expression should both be invalid and return null on 'build()'. + */ + @Test + void testBuilderWithoutRegexNotValid() { + + final RegexFilter.Builder builder = RegexFilter.newBuilder(); + + assertFalse(builder.isValid()); + + assertNull(builder.build()); + + } + + /** + * A builder with an invalid 'regex' expression should return null on 'build()'. + */ + @Test + void testBuilderWithInvalidRegexNotValid() { + + final RegexFilter.Builder builder = RegexFilter.newBuilder(); + + builder.setRegex("[a-z"); + + assertFalse(builder.isValid()); + + assertNull(builder.build()); + + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java index d26d157bcc7..6c842e66593 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java @@ -21,6 +21,7 @@ import java.util.Comparator; import java.util.Objects; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.core.Filter; @@ -44,9 +45,6 @@ @Plugin(name = "RegexFilter", category = Node.CATEGORY, elementType = Filter.ELEMENT_TYPE, printObject = true) public final class RegexFilter extends AbstractFilter { - /** The regular-expression. */ - private final String regex; - /** The pattern compiled from the regular-expression. */ private final Pattern pattern; @@ -62,13 +60,12 @@ private RegexFilter(final Builder builder) { super(builder); - this.regex = builder.regex; this.useRawMessage = Boolean.TRUE.equals(builder.useRawMsg); try { - this.pattern = Pattern.compile(regex); + this.pattern = Pattern.compile(builder.regex); } catch (final Exception ex) { - throw new IllegalArgumentException("Unable to compile regular expression: '" + regex + "'.", ex); + throw new IllegalArgumentException("Unable to compile regular expression: '" + builder.regex + "'.", ex); } } @@ -77,7 +74,7 @@ private RegexFilter(final Builder builder) { * @return the regular-expression (it may be an empty string but never {@code null}) */ public String getRegex() { - return this.regex; + return this.pattern.pattern(); } /** @@ -136,7 +133,7 @@ public Result filter(final LogEvent event) { * @param msg the message * @return the filter result */ - private Result filter(final String msg) { + public Result filter(final String msg) { if (msg == null) { return onMismatch; } @@ -181,7 +178,7 @@ private String getMessageTextByType(final Message message) { @Override public String toString() { - return "useRawMessage=" + useRawMessage + ", regex=" + regex + ", pattern=" + pattern.toString(); + return "useRawMessage=" + useRawMessage + ", pattern=" + pattern.toString(); } /** @@ -251,7 +248,11 @@ public Builder setUseRawMsg(final boolean useRawMsg) { */ @Override public boolean isValid() { - return (regex != null); + boolean valid = true; + if (!isRegexValid()) { + valid = false; + } + return valid; } /** @@ -273,6 +274,25 @@ public RegexFilter build() { return null; } } + + /** + * Validates the 'regex' attribute. + *

+ * If the regular-expression is not set, or cannot be compiled to a valid pattern the validation will fail. + *

+ * @return {@code true} if the regular-expression is valid; otherwise, {@code false} + */ + private boolean isRegexValid() { + if (regex == null) { + return false; + } + try { + Pattern.compile(regex); + } catch (final PatternSyntaxException ex) { + return false; + } + return true; + } } /* @@ -306,7 +326,7 @@ private RegexFilter( final Result onMatch, final Result onMismatch) { super(onMatch, onMismatch); - this.regex = Objects.requireNonNull(regex, "The 'regex' argument must be provided for RegexFilter"); + Objects.requireNonNull(regex, "The 'regex' argument must be provided for RegexFilter"); this.patternFlags = patternFlags == null ? new String[0] : patternFlags.clone(); try { int flags = toPatternFlags(this.patternFlags); From d08606028cf18d831c31e9b185b8b0ce3cc44309 Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Sun, 2 Mar 2025 16:11:09 +0100 Subject: [PATCH 4/7] Fix validation behavior in RegexFilter (#3086) + added validation checks to RegexFilter + added JVerify nullability annotations to RegexFilter + updated javadoc + replaced deprecated usages of CompositeFilter#getFilters with CompositeFilter#getFiltersArray in AbstractFilterableTest --- .../log4j/core/filter/AbstractFilterTest.java | 1 - .../core/filter/AbstractFilterableTest.java | 24 +- .../log4j/core/filter/RegexFilterTest.java | 57 +++-- .../log4j/core/filter/RegexFilter.java | 217 ++++++++++-------- 4 files changed, 159 insertions(+), 140 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterTest.java index 8996b9f2f16..fc42abe2937 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterTest.java @@ -34,7 +34,6 @@ class AbstractFilterTest { @Test void testUnrolledBackwardsCompatible() { final ConcreteFilter filter = new ConcreteFilter(); - final Filter.Result expected = Filter.Result.DENY; verifyMethodsWithUnrolledVarargs(filter, Filter.Result.DENY); filter.testResult = Filter.Result.ACCEPT; diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java index 4f117a1ccf5..bd3d3f51b20 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java @@ -54,7 +54,7 @@ void testAddMultipleSimpleFilters() { // into a CompositeFilter.class filterable.addFilter(filter); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); } @Test @@ -67,7 +67,7 @@ void testAddMultipleEqualSimpleFilter() { // into a CompositeFilter.class filterable.addFilter(filter); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); } @Test @@ -93,7 +93,7 @@ void testAddMultipleCompositeFilters() { // into a CompositeFilter.class filterable.addFilter(compositeFilter); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); } @Test @@ -109,7 +109,7 @@ void testAddSimpleFilterAndCompositeFilter() { // into a CompositeFilter.class filterable.addFilter(compositeFilter); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); } @Test @@ -125,7 +125,7 @@ void testAddCompositeFilterAndSimpleFilter() { // into a CompositeFilter.class filterable.addFilter(notInCompositeFilterFilter); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); } @Test @@ -170,7 +170,7 @@ void testRemoveSimpleEqualFilterFromMultipleSimpleFilters() { filterable.addFilter(filterCopy); filterable.removeFilter(filterCopy); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); filterable.removeFilter(filterCopy); assertEquals(filterOriginal, filterable.getFilter()); filterable.removeFilter(filterOriginal); @@ -224,7 +224,7 @@ void testRemoveSimpleFilterFromCompositeAndSimpleFilter() { // should not remove internal filter of compositeFilter filterable.removeFilter(anotherFilter); assertInstanceOf(CompositeFilter.class, filterable.getFilter()); - assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); } @Test @@ -247,9 +247,9 @@ void testRemoveFiltersFromComposite() { filterable.addFilter(compositeFilter); filterable.addFilter(anotherFilter); - assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); filterable.removeFilter(filter1); - assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size()); + assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length); filterable.removeFilter(filter2); assertSame(anotherFilter, filterable.getFilter()); } @@ -274,11 +274,7 @@ public boolean equals(final Object o) { final EqualFilter that = (EqualFilter) o; - if (key != null ? !key.equals(that.key) : that.key != null) { - return false; - } - - return true; + return key != null ? key.equals(that.key) : that.key == null; } @Override diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java index 6cb07a5f7df..b7a128b5bfb 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java @@ -24,7 +24,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.apache.logging.log4j.Level; @@ -92,11 +91,10 @@ void testDotAllPattern() throws Exception { @Test void testNoMsg() { - final RegexFilter filter = - RegexFilter.newBuilder() - .setRegex(".* test .*") - .setUseRawMsg(false) - .build(); + final RegexFilter filter = RegexFilter.newBuilder() + .setRegex(".* test .*") + .setUseRawMsg(false) + .build(); assertNotNull(filter); @@ -114,13 +112,12 @@ void testParameterizedMsg() { final Object[] params = {"foo", "bar"}; // match against raw message - final RegexFilter rawFilter = - RegexFilter.newBuilder() - .setRegex("params \\{\\} \\{\\}") - .setUseRawMsg(true) - .setOnMatch(Result.ACCEPT) - .setOnMismatch(Result.DENY) - .build(); + final RegexFilter rawFilter = RegexFilter.newBuilder() + .setRegex("params \\{\\} \\{\\}") + .setUseRawMsg(true) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY) + .build(); assertNotNull(rawFilter); @@ -128,12 +125,12 @@ void testParameterizedMsg() { assertThat(rawResult, equalTo(Result.ACCEPT)); // match against formatted message - final RegexFilter fmtFilter = - RegexFilter.newBuilder() - .setRegex("params foo bar") - .setUseRawMsg(false) - .setOnMatch(Result.ACCEPT) - .setOnMismatch(Result.DENY).build(); + final RegexFilter fmtFilter = RegexFilter.newBuilder() + .setRegex("params foo bar") + .setUseRawMsg(false) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY) + .build(); assertNotNull(fmtFilter); @@ -149,8 +146,11 @@ void testWithValidRegex() { final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores - final RegexFilter.Builder builder = - RegexFilter.newBuilder().setRegex(regex).setUseRawMsg(false).setOnMatch(Result.ACCEPT).setOnMismatch(Result.DENY); + final RegexFilter.Builder builder = RegexFilter.newBuilder() + .setRegex(regex) + .setUseRawMsg(false) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY); assertTrue(builder.isValid()); @@ -170,13 +170,12 @@ void testRegexFilterGetters() { final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores - final RegexFilter filter = - RegexFilter.newBuilder() - .setRegex(regex) - .setUseRawMsg(false) - .setOnMatch(Result.ACCEPT) - .setOnMismatch(Result.DENY) - .build(); + final RegexFilter filter = RegexFilter.newBuilder() + .setRegex(regex) + .setUseRawMsg(false) + .setOnMatch(Result.ACCEPT) + .setOnMismatch(Result.DENY) + .build(); assertNotNull(filter); @@ -199,7 +198,6 @@ void testBuilderWithoutRegexNotValid() { assertFalse(builder.isValid()); assertNull(builder.build()); - } /** @@ -215,6 +213,5 @@ void testBuilderWithInvalidRegexNotValid() { assertFalse(builder.isValid()); assertNull(builder.build()); - } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java index 6c842e66593..ffe470af485 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java @@ -21,7 +21,6 @@ import java.util.Comparator; import java.util.Objects; import java.util.regex.Pattern; -import java.util.regex.PatternSyntaxException; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.core.Filter; @@ -33,16 +32,23 @@ import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory; import org.apache.logging.log4j.core.config.plugins.PluginElement; +import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required; +import org.apache.logging.log4j.core.util.Assert; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFormatMessage; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.message.StringFormattedMessage; import org.apache.logging.log4j.message.StructuredDataMessage; +import org.apache.logging.log4j.util.Strings; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; /** - * A filter that matches the given regular expression pattern against messages. + * This filter returns the {@code onMatch} result if the message exactly matches the configured + * "{@code regex}" regular-expression pattern; otherwise, it returns the {@code onMismatch} result. */ @Plugin(name = "RegexFilter", category = Node.CATEGORY, elementType = Filter.ELEMENT_TYPE, printObject = true) +@NullMarked public final class RegexFilter extends AbstractFilter { /** The pattern compiled from the regular-expression. */ @@ -54,12 +60,16 @@ public final class RegexFilter extends AbstractFilter { /** * Constructs a new {@code RegexFilter} configured by the given builder. * @param builder the builder - * @throws IllegalArgumentException if the regular expression cannot be compiled to a pattern + * @throws IllegalArgumentException if the regular expression is not configured or cannot be compiled to a pattern */ private RegexFilter(final Builder builder) { super(builder); + if (Strings.isNotBlank(builder.regex)) { + throw new IllegalArgumentException("The 'regex' attribute must not be null or empty."); + } + this.useRawMessage = Boolean.TRUE.equals(builder.useRawMsg); try { @@ -69,14 +79,6 @@ private RegexFilter(final Builder builder) { } } - /** - * Returns the regular-expression. - * @return the regular-expression (it may be an empty string but never {@code null}) - */ - public String getRegex() { - return this.pattern.pattern(); - } - /** * Returns the compiled regular-expression pattern. * @return the pattern (will never be {@code null} @@ -85,59 +87,123 @@ public Pattern getPattern() { return this.pattern; } + /** + * Returns the regular-expression. + * @return the regular-expression (it may be an empty string but never {@code null}) + */ + public String getRegex() { + return this.pattern.pattern(); + } + /** * Returns whether the raw-message should be used. - * @return {@code} if the raw message should be used; otherwise, {@code false} + * @return {@code true} if the raw message should be used; otherwise, {@code false} */ public boolean isUseRawMessage() { return this.useRawMessage; } - /** {@inheritDoc} */ + /** + * {@inheritDoc} + *

+ * This implementation performs the filter evaluation against the given message formatted with + * the given parameters. + *

+ *

+ * The following method arguments are ignored by this filter method implementation: + *

    + *
  • {@code logger}
    • + *
  • {@code level}
    • + *
  • {@code marker}
    • + *
+ *

+ */ @Override public Result filter( - final Logger logger, final Level level, final Marker marker, final String msg, final Object... params) { + final @Nullable Logger logger, + final @Nullable Level level, + final @Nullable Marker marker, + final @Nullable String msg, + final @Nullable Object @Nullable ... params) { + return (useRawMessage || params == null || params.length == 0) ? filter(msg) : filter(ParameterizedMessage.format(msg, params)); } - /** {@inheritDoc} */ + /** + * {@inheritDoc} + *

+ * This implementation performs the filter evaluation against the given message. + *

+ *

+ * The following method arguments are ignored by this filter method implementation: + *

    + *
  • {@code logger}
    • + *
  • {@code level}
    • + *
  • {@code marker}
    • + *
  • {@code throwable}
    • + *
+ *

+ */ @Override public Result filter( - final Logger logger, final Level level, final Marker marker, final Object msg, final Throwable t) { - return (msg == null) ? this.onMismatch : filter(msg.toString()); + final @Nullable Logger logger, + final @Nullable Level level, + final @Nullable Marker marker, + final @Nullable Object message, + final @Nullable Throwable throwable) { + + return (message == null) ? this.onMismatch : filter(message.toString()); } - /** {@inheritDoc} */ + /** + * {@inheritDoc} + *

+ * This implementation performs the filter evaluation against the given message. + *

+ *

+ * The following method arguments are ignored by this filter method implementation: + *

    + *
  • {@code logger}
    • + *
  • {@code level}
    • + *
  • {@code marker}
    • + *
  • {@code throwable}
    • + *
+ *

+ */ @Override public Result filter( - final Logger logger, final Level level, final Marker marker, final Message msg, final Throwable t) { - if (msg == null) { - return onMismatch; - } - return filter(getMessageTextByType(msg)); + final @Nullable Logger logger, + final @Nullable Level level, + final @Nullable Marker marker, + final @Nullable Message message, + final @Nullable Throwable throwable) { + return (message == null) ? this.onMismatch : filter(getMessageTextByType(message)); } - /** {@inheritDoc} */ + /** + * {@inheritDoc} + * + * @throws NullPointerException if the {@code event} argument is {@code null} + */ @Override public Result filter(final LogEvent event) { + Objects.requireNonNull(event, "The 'event' argument must not be null."); return filter(getMessageTextByType(event.getMessage())); } /** - * Apply the filter to the given message and return the match/mismatch result. + * Apply the filter to the given message and return the {@code onMatch} result if the entire + * message matches the configured regex pattern; otherwise, {@code onMismatch}. *

- * If the given '{@code msg}' is {@code null} the configured mismatch result will be returned. + * If the given '{@code msg}' is {@code null} the configured {@code onMismatch} result will be returned. *

* @param msg the message - * @return the filter result + * @return the {@code onMatch} result if the pattern matches; otherwise, the {@code onMismatch} result */ - public Result filter(final String msg) { - if (msg == null) { - return onMismatch; - } - return pattern.matcher(msg).matches() ? onMatch : onMismatch; + public Result filter(final @Nullable String msg) { + return (msg != null && pattern.matcher(msg).matches()) ? onMatch : onMismatch; } /** @@ -176,6 +242,7 @@ private String getMessageTextByType(final Message message) { : message.getFormattedMessage(); } + /** {@inheritDoc} */ @Override public String toString() { return "useRawMessage=" + useRawMessage + ", pattern=" + pattern.toString(); @@ -183,12 +250,11 @@ public String toString() { /** * Creates a new builder instance. - * * @return the new builder instance */ @PluginBuilderFactory - public static RegexFilter.Builder newBuilder() { - return new RegexFilter.Builder(); + public static Builder newBuilder() { + return new Builder(); } /** @@ -203,7 +269,8 @@ public static final class Builder extends AbstractFilterBuilder - * If the regular-expression is not set, or cannot be compiled to a valid pattern the validation will fail. - *

- * @return {@code true} if the regular-expression is valid; otherwise, {@code false} - */ - private boolean isRegexValid() { - if (regex == null) { - return false; - } - try { - Pattern.compile(regex); - } catch (final PatternSyntaxException ex) { - return false; - } - return true; - } } /* @@ -320,11 +357,11 @@ private boolean isRegexValid() { @Deprecated @SuppressWarnings("MagicConstant") private RegexFilter( - final boolean useRawMessage, final String regex, - final String[] patternFlags, - final Result onMatch, - final Result onMismatch) { + final boolean useRawMessage, + final @Nullable String @Nullable [] patternFlags, + final @Nullable Result onMatch, + final @Nullable Result onMismatch) { super(onMatch, onMismatch); Objects.requireNonNull(regex, "The 'regex' argument must be provided for RegexFilter"); this.patternFlags = patternFlags == null ? new String[0] : patternFlags.clone(); @@ -368,32 +405,22 @@ public String[] getPatternFlags() { public static RegexFilter createFilter( // @formatter:off @PluginAttribute("regex") final String regex, - @PluginElement("PatternFlags") final String[] patternFlags, - @PluginAttribute("useRawMsg") final Boolean useRawMsg, - @PluginAttribute("onMatch") final Result match, - @PluginAttribute("onMismatch") final Result mismatch) + @PluginElement("PatternFlags") final String @Nullable [] patternFlags, + @PluginAttribute("useRawMsg") final @Nullable Boolean useRawMsg, + @PluginAttribute("onMatch") final @Nullable Result match, + @PluginAttribute("onMismatch") final @Nullable Result mismatch) // @formatter:on throws IllegalArgumentException, IllegalAccessException { // LOG4J-3086 - pattern-flags can be embedded in RegEx expression + Objects.requireNonNull(regex, "The 'regex' argument must not be null."); - boolean raw = Boolean.TRUE.equals(useRawMsg); - if (regex == null) { - LOGGER.error("A regular expression must be provided for RegexFilter"); - return null; - } - - try { - return new RegexFilter(raw, regex, patternFlags, match, mismatch); - } catch (final Exception ex) { - LOGGER.error("Unable to create RegexFilter. {}", ex.getMessage(), ex); - return null; - } + return new RegexFilter(regex, Boolean.TRUE.equals(useRawMsg), patternFlags, match, mismatch); } /** @deprecated pattern flags have been deprecated - they can just be included in the regex-expression. */ @Deprecated - private static int toPatternFlags(final String[] patternFlags) + private static int toPatternFlags(final String @Nullable [] patternFlags) throws IllegalArgumentException, IllegalAccessException { if (patternFlags == null || patternFlags.length == 0) { return DEFAULT_PATTERN_FLAGS; From 4463a824ce843cae63aa5b6f4c0e2bdfecdca2f9 Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Sun, 2 Mar 2025 19:49:09 +0100 Subject: [PATCH 5/7] Fixed a reverse logic check in a validation (#3086) --- .../log4j/core/config/CompositeConfigurationTest.java | 9 ++++----- .../apache/logging/log4j/core/filter/RegexFilter.java | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java index 69956c1c845..fffd7bfda14 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java @@ -293,11 +293,10 @@ public void evaluate() throws Throwable { private void runTest(final LoggerContextRule rule, final Statement statement) { try { rule.apply( - statement, - Description.createTestDescription( - getClass(), - Thread.currentThread().getStackTrace()[1].getMethodName())) - .evaluate(); + statement, + Description.createTestDescription(getClass(), + Thread.currentThread().getStackTrace()[1].getMethodName())) + .evaluate(); } catch (final Throwable e) { Throwables.rethrow(e); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java index ffe470af485..d97f70ea979 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java @@ -66,7 +66,7 @@ private RegexFilter(final Builder builder) { super(builder); - if (Strings.isNotBlank(builder.regex)) { + if (Strings.isBlank(builder.regex)) { throw new IllegalArgumentException("The 'regex' attribute must not be null or empty."); } @@ -317,8 +317,8 @@ public Builder setUseRawMsg(final boolean useRawMsg) { public @Nullable RegexFilter build() { // validate the "regex" attribute - if (this.regex == null) { - LOGGER.error("Unable to create RegexFilter: The 'regex' attribute must be provided."); + if (Strings.isEmpty(this.regex)) { + LOGGER.error("Unable to create RegexFilter: The 'regex' attribute be set to a non-empty String."); return null; } From 4439b08fbada1efae7aa94fc3bfbdad983f05532 Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Sun, 2 Mar 2025 19:58:38 +0100 Subject: [PATCH 6/7] Fix Spotless errors (#3086) --- .../log4j/core/config/CompositeConfigurationTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java index fffd7bfda14..69956c1c845 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/CompositeConfigurationTest.java @@ -293,10 +293,11 @@ public void evaluate() throws Throwable { private void runTest(final LoggerContextRule rule, final Statement statement) { try { rule.apply( - statement, - Description.createTestDescription(getClass(), - Thread.currentThread().getStackTrace()[1].getMethodName())) - .evaluate(); + statement, + Description.createTestDescription( + getClass(), + Thread.currentThread().getStackTrace()[1].getMethodName())) + .evaluate(); } catch (final Throwable e) { Throwables.rethrow(e); } From 33dfec35913206c4eb5e24c7460820dec2beaee4 Mon Sep 17 00:00:00 2001 From: Jeff Thomas Date: Sun, 2 Mar 2025 20:21:55 +0100 Subject: [PATCH 7/7] Fix RegexFilterTest (#3086) --- .../apache/logging/log4j/core/filter/RegexFilterTest.java | 8 +------- .../org/apache/logging/log4j/core/filter/RegexFilter.java | 6 ++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java index b7a128b5bfb..3bf96c607fd 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java @@ -40,7 +40,7 @@ class RegexFilterTest { @BeforeAll static void before() { - StatusLogger.getLogger().setLevel(Level.OFF); + StatusLogger.getLogger().getFallbackListener().setLevel(Level.OFF); } @Test @@ -152,8 +152,6 @@ void testWithValidRegex() { .setOnMatch(Result.ACCEPT) .setOnMismatch(Result.DENY); - assertTrue(builder.isValid()); - final RegexFilter filter = builder.build(); assertNotNull(filter); @@ -195,8 +193,6 @@ void testBuilderWithoutRegexNotValid() { final RegexFilter.Builder builder = RegexFilter.newBuilder(); - assertFalse(builder.isValid()); - assertNull(builder.build()); } @@ -210,8 +206,6 @@ void testBuilderWithInvalidRegexNotValid() { builder.setRegex("[a-z"); - assertFalse(builder.isValid()); - assertNull(builder.build()); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java index d97f70ea979..839089b35bb 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/RegexFilter.java @@ -308,6 +308,12 @@ public Builder setUseRawMsg(final boolean useRawMsg) { return this; } + /** {@inheritDoc} */ + @Override + public boolean isValid() { + return (Strings.isNotEmpty(this.regex)); + } + /** * Builds and returns a {@link RegexFilter} instance configured by this builder. *