From d40639e8f5fc72d9788d796d766ee44f44bd537f Mon Sep 17 00:00:00 2001
From: Jeff Thomas <jeffrey.thomas@t-systems.com>
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<Field> 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 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="3086" link="https://github.com/apache/logging-log4j2/issues/3086"/>
+ <description format="asciidoc">
+ Removed 'patternFlags' @PluginAttribute from RegexFilter @PluginFactory createFilter.
+ </description>
+</entry>
From 6063bba4a23ed28d7586cea4188bb4aef98534f4 Mon Sep 17 00:00:00 2001
From: Jeff Thomas <jeffrey.thomas@t-systems.com>
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<B extends AbstractFilterBuild
public static final String ATTR_ON_MISMATCH = "onMismatch";
public static final String ATTR_ON_MATCH = "onMatch";
+ /**
+ * The action to perform when a match occurs.
+ */
@PluginBuilderAttribute(ATTR_ON_MATCH)
- private Result onMatch = Result.NEUTRAL;
+ protected Result onMatch = Result.NEUTRAL;
+ /**
+ * The action to perform when a mismatch occurs.
+ */
@PluginBuilderAttribute(ATTR_ON_MISMATCH)
- private Result onMismatch = Result.DENY;
+ protected Result onMismatch = Result.DENY;
+ /**
+ * Returns the action to apply when a match occurs
+ * @return the match result
+ */
public Result getOnMatch() {
return onMatch;
}
+ /**
+ * Returns the action to apply when a mismatch occurs
+ * @return the mismatch result
+ */
public Result getOnMismatch() {
return onMismatch;
}
@@ -110,6 +126,19 @@ protected AbstractFilter(final Result onMatch, final Result onMismatch) {
this.onMismatch = onMismatch == null ? Result.DENY : onMismatch;
}
+ /**
+ * Constructs a new instance configured by the given builder
+ * @param builder the builder
+ * @throws NullPointerException if the builder argument is {@code null}
+ */
+ protected AbstractFilter(final 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.
+ * <p>
+ * If the given '{@code msg}' is {@code null} the configured mismatch result will be returned.
+ * </p>
+ * @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}.
+ * <p>
+ * If the raw-message flag is enabled and message is an instance of the following, the raw message format
+ * will be returned.
+ * </p>
+ * <ul>
+ * <li>{@link MessageFormatMessage}</li>
+ * <li>{@link ParameterizedMessage}</li>
+ * <li>{@link StringFormattedMessage}</li>
+ * <li>{@link StructuredDataMessage}</li>
+ * </ul>
+ * <p>
+ * If the '{@code useRawMessage}' flag is disabled <i>OR</i> the message is not one of the above
+ * implementations, the message's formatted message will be returned.
+ * </p>
+ * <h3>Developer Note</h3>
+ * <p>
+ * While `Message#getFormat()` is broken in general, it still makes sense for certain types.
+ * Hence, suppress the deprecation warning.
+ * </p>
+ *
+ * @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<RegexFilter.Builder>
+ implements org.apache.logging.log4j.core.util.Builder<RegexFilter> {
+
+ /* 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<Field> 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> {
*/
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 <jeffrey.thomas@t-systems.com>
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.
+ * <p>
+ * If the regular-expression is not set, or cannot be compiled to a valid pattern the validation will fail.
+ * </p>
+ * @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 <jeff.w.thomas@outlook.com>
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}
+ * <p>
+ * This implementation performs the filter evaluation against the given message formatted with
+ * the given parameters.
+ * </p>
+ * <p>
+ * The following method arguments are ignored by this filter method implementation:
+ * <ul>
+ * <li>{@code logger}</li>
+ * <li>{@code level}</li>
+ * <li>{@code marker}</li>
+ * </ul>
+ * </p>
+ */
@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}
+ * <p>
+ * This implementation performs the filter evaluation against the given message.
+ * </p>
+ * <p>
+ * The following method arguments are ignored by this filter method implementation:
+ * <ul>
+ * <li>{@code logger}</li>
+ * <li>{@code level}</li>
+ * <li>{@code marker}</li>
+ * <li>{@code throwable}</li>
+ * </ul>
+ * </p>
+ */
@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}
+ * <p>
+ * This implementation performs the filter evaluation against the given message.
+ * </p>
+ * <p>
+ * The following method arguments are ignored by this filter method implementation:
+ * <ul>
+ * <li>{@code logger}</li>
+ * <li>{@code level}</li>
+ * <li>{@code marker}</li>
+ * <li>{@code throwable}</li>
+ * </ul>
+ * </p>
+ */
@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 <i>entire</i>
+ * message matches the configured regex pattern; otherwise, {@code onMismatch}.
* <p>
- * 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.
* </p>
* @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<RegexFilter.Buil
* The regular expression to match.
*/
@PluginBuilderAttribute
- private String regex;
+ @Required(message = "No 'regex' provided for RegexFilter")
+ private @Nullable String regex;
/**
* If {@code true}, for {@link ParameterizedMessage}, {@link StringFormattedMessage},
@@ -211,11 +278,9 @@ public static final class Builder extends AbstractFilterBuilder<RegexFilter.Buil
* the message field will be used as the match target.
*/
@PluginBuilderAttribute
- private Boolean useRawMsg;
+ private @Nullable Boolean useRawMsg;
- /**
- * Private constructor.
- */
+ /** Private constructor. */
private Builder() {
super();
}
@@ -227,7 +292,7 @@ private Builder() {
* @return this builder
*/
public Builder setRegex(final String regex) {
- this.regex = regex;
+ this.regex = Assert.requireNonEmpty(regex, "The 'regex' attribute must not be null or empty.");
return this;
}
@@ -243,30 +308,21 @@ public Builder setUseRawMsg(final boolean useRawMsg) {
return this;
}
- /**
- * {@inheritDoc}
- */
- @Override
- public boolean isValid() {
- boolean valid = true;
- if (!isRegexValid()) {
- valid = false;
- }
- return valid;
- }
-
/**
* 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() {
+ public @Nullable RegexFilter build() {
- if (!isValid()) {
+ // validate the "regex" attribute
+ if (this.regex == null) {
+ LOGGER.error("Unable to create RegexFilter: The 'regex' attribute must be provided.");
return null;
}
+ // build with *safety* to not throw exceptions
try {
return new RegexFilter(this);
} catch (final Exception ex) {
@@ -274,25 +330,6 @@ public RegexFilter build() {
return null;
}
}
-
- /**
- * Validates the 'regex' attribute.
- * <p>
- * If the regular-expression is not set, or cannot be compiled to a valid pattern the validation will fail.
- * </p>
- * @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 <jeff.w.thomas@outlook.com>
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 <jeff.w.thomas@outlook.com>
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 <jeff.w.thomas@outlook.com>
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.
*