Skip to content

Commit 2ca8348

Browse files
ramanathan1504jethomas-tsivy
authored
Remove the patternFlags attribute from RegexFilter (#4119)
Co-authored-by: Jeff Thomas <jeffrey.thomas@t-systems.com> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
1 parent 019a928 commit 2ca8348

8 files changed

Lines changed: 469 additions & 131 deletions

File tree

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ class AbstractFilterTest {
3434
@Test
3535
void testUnrolledBackwardsCompatible() {
3636
final ConcreteFilter filter = new ConcreteFilter();
37-
final Filter.Result expected = Filter.Result.DENY;
3837
verifyMethodsWithUnrolledVarargs(filter, Filter.Result.DENY);
3938

4039
filter.testResult = Filter.Result.ACCEPT;

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ void testAddMultipleSimpleFilters() {
5454
// into a CompositeFilter.class
5555
filterable.addFilter(filter);
5656
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
57-
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
57+
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
5858
}
5959

6060
@Test
@@ -67,7 +67,7 @@ void testAddMultipleEqualSimpleFilter() {
6767
// into a CompositeFilter.class
6868
filterable.addFilter(filter);
6969
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
70-
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
70+
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
7171
}
7272

7373
@Test
@@ -93,7 +93,7 @@ void testAddMultipleCompositeFilters() {
9393
// into a CompositeFilter.class
9494
filterable.addFilter(compositeFilter);
9595
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
96-
assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFilters().size());
96+
assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
9797
}
9898

9999
@Test
@@ -109,7 +109,7 @@ void testAddSimpleFilterAndCompositeFilter() {
109109
// into a CompositeFilter.class
110110
filterable.addFilter(compositeFilter);
111111
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
112-
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
112+
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
113113
}
114114

115115
@Test
@@ -125,7 +125,7 @@ void testAddCompositeFilterAndSimpleFilter() {
125125
// into a CompositeFilter.class
126126
filterable.addFilter(notInCompositeFilterFilter);
127127
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
128-
assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size());
128+
assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
129129
}
130130

131131
@Test
@@ -170,7 +170,7 @@ void testRemoveSimpleEqualFilterFromMultipleSimpleFilters() {
170170
filterable.addFilter(filterCopy);
171171
filterable.removeFilter(filterCopy);
172172
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
173-
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
173+
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
174174
filterable.removeFilter(filterCopy);
175175
assertEquals(filterOriginal, filterable.getFilter());
176176
filterable.removeFilter(filterOriginal);
@@ -224,7 +224,7 @@ void testRemoveSimpleFilterFromCompositeAndSimpleFilter() {
224224
// should not remove internal filter of compositeFilter
225225
filterable.removeFilter(anotherFilter);
226226
assertInstanceOf(CompositeFilter.class, filterable.getFilter());
227-
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
227+
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
228228
}
229229

230230
@Test
@@ -247,9 +247,9 @@ void testRemoveFiltersFromComposite() {
247247

248248
filterable.addFilter(compositeFilter);
249249
filterable.addFilter(anotherFilter);
250-
assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size());
250+
assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
251251
filterable.removeFilter(filter1);
252-
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
252+
assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
253253
filterable.removeFilter(filter2);
254254
assertSame(anotherFilter, filterable.getFilter());
255255
}
@@ -274,11 +274,7 @@ public boolean equals(final Object o) {
274274

275275
final EqualFilter that = (EqualFilter) o;
276276

277-
if (key != null ? !key.equals(that.key) : that.key != null) {
278-
return false;
279-
}
280-
281-
return true;
277+
return key != null ? key.equals(that.key) : that.key == null;
282278
}
283279

284280
@Override

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java‎

Lines changed: 110 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919
import static org.hamcrest.CoreMatchers.equalTo;
2020
import static org.hamcrest.MatcherAssert.assertThat;
2121
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
22-
import static org.junit.jupiter.api.Assertions.assertNull;
22+
import static org.junit.jupiter.api.Assertions.assertEquals;
23+
import static org.junit.jupiter.api.Assertions.assertFalse;
24+
import static org.junit.jupiter.api.Assertions.assertNotNull;
2325
import static org.junit.jupiter.api.Assertions.assertSame;
26+
import static org.junit.jupiter.api.Assertions.assertThrows;
2427
import static org.junit.jupiter.api.Assertions.assertTrue;
2528

2629
import org.apache.logging.log4j.Level;
@@ -37,19 +40,23 @@
3740
class RegexFilterTest {
3841
@BeforeAll
3942
static void before() {
40-
StatusLogger.getLogger().setLevel(Level.OFF);
43+
StatusLogger.getLogger().getFallbackListener().setLevel(Level.OFF);
4144
}
4245

4346
@Test
4447
void testRegexFilterDoesNotThrowWithAllTheParametersExceptRegexEqualNull() {
4548
assertDoesNotThrow(() -> {
46-
RegexFilter.createFilter(".* test .*", null, null, null, null);
49+
RegexFilter.newBuilder().setRegex(".* test .*").build();
4750
});
4851
}
4952

5053
@Test
5154
void testThresholds() throws Exception {
52-
RegexFilter filter = RegexFilter.createFilter(".* test .*", null, false, null, null);
55+
RegexFilter filter = RegexFilter.newBuilder()
56+
.setRegex(".* test .*")
57+
.setUseRawMsg(false)
58+
.build();
59+
assertNotNull(filter);
5360
filter.start();
5461
assertTrue(filter.isStarted());
5562
assertSame(
@@ -65,55 +72,135 @@ void testThresholds() throws Exception {
6572
.setMessage(new SimpleMessage("test")) //
6673
.build();
6774
assertSame(Filter.Result.DENY, filter.filter(event));
68-
filter = RegexFilter.createFilter(null, null, false, null, null);
69-
assertNull(filter);
75+
final RegexFilter.Builder filterBuilder = RegexFilter.newBuilder();
76+
assertThrows(IllegalArgumentException.class, filterBuilder::build);
7077
}
7178

7279
@Test
7380
void testDotAllPattern() throws Exception {
7481
final String singleLine = "test single line matches";
7582
final String multiLine = "test multi line matches\nsome more lines";
76-
final RegexFilter filter = RegexFilter.createFilter(
77-
".*line.*", new String[] {"DOTALL", "COMMENTS"}, false, Filter.Result.DENY, Filter.Result.ACCEPT);
83+
final RegexFilter filter =
84+
RegexFilter.createFilter("(?xs).*line.*", null, false, Filter.Result.DENY, Filter.Result.ACCEPT);
7885
final Result singleLineResult = filter.filter(null, null, null, (Object) singleLine, null);
7986
final Result multiLineResult = filter.filter(null, null, null, (Object) multiLine, null);
8087
assertThat(singleLineResult, equalTo(Result.DENY));
8188
assertThat(multiLineResult, equalTo(Result.DENY));
8289
}
8390

8491
@Test
85-
void testNoMsg() throws Exception {
86-
final RegexFilter filter = RegexFilter.createFilter(".* test .*", null, false, null, null);
92+
void testNoMsg() {
93+
94+
final RegexFilter filter = RegexFilter.newBuilder()
95+
.setRegex(".* test .*")
96+
.setUseRawMsg(false)
97+
.build();
98+
99+
assertNotNull(filter);
100+
87101
filter.start();
102+
88103
assertTrue(filter.isStarted());
89104
assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, (Object) null, null));
90105
assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, (Message) null, null));
91106
assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, null, (Object[]) null));
92107
}
93108

94109
@Test
95-
void testParameterizedMsg() throws Exception {
110+
void testParameterizedMsg() {
96111
final String msg = "params {} {}";
97112
final Object[] params = {"foo", "bar"};
98113

99114
// match against raw message
100-
final RegexFilter rawFilter = RegexFilter.createFilter(
101-
"params \\{\\} \\{\\}",
102-
null,
103-
true, // useRawMsg
104-
Result.ACCEPT,
105-
Result.DENY);
115+
final RegexFilter rawFilter = RegexFilter.newBuilder()
116+
.setRegex("params \\{\\} \\{\\}")
117+
.setUseRawMsg(true)
118+
.setOnMatch(Result.ACCEPT)
119+
.setOnMismatch(Result.DENY)
120+
.build();
121+
122+
assertNotNull(rawFilter);
123+
106124
final Result rawResult = rawFilter.filter(null, null, null, msg, params);
107125
assertThat(rawResult, equalTo(Result.ACCEPT));
108126

109127
// match against formatted message
110-
final RegexFilter fmtFilter = RegexFilter.createFilter(
111-
"params foo bar",
112-
null,
113-
false, // useRawMsg
114-
Result.ACCEPT,
115-
Result.DENY);
128+
final RegexFilter fmtFilter = RegexFilter.newBuilder()
129+
.setRegex("params foo bar")
130+
.setUseRawMsg(false)
131+
.setOnMatch(Result.ACCEPT)
132+
.setOnMismatch(Result.DENY)
133+
.build();
134+
135+
assertNotNull(fmtFilter);
136+
116137
final Result fmtResult = fmtFilter.filter(null, null, null, msg, params);
117138
assertThat(fmtResult, equalTo(Result.ACCEPT));
118139
}
140+
141+
/**
142+
* A builder with no 'regex' expression should both be invalid and return null on 'build()'.
143+
*/
144+
@Test
145+
void testWithValidRegex() {
146+
147+
final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores
148+
149+
final RegexFilter.Builder builder = RegexFilter.newBuilder()
150+
.setRegex(regex)
151+
.setUseRawMsg(false)
152+
.setOnMatch(Result.ACCEPT)
153+
.setOnMismatch(Result.DENY);
154+
155+
final RegexFilter filter = builder.build();
156+
157+
assertNotNull(filter);
158+
159+
assertEquals(Result.ACCEPT, filter.filter("Hello_123"));
160+
161+
assertEquals(Result.DENY, filter.filter("Hello@123"));
162+
163+
assertEquals(regex, filter.getRegex());
164+
}
165+
166+
@Test
167+
void testRegexFilterGetters() {
168+
169+
final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores
170+
171+
final RegexFilter filter = RegexFilter.newBuilder()
172+
.setRegex(regex)
173+
.setUseRawMsg(false)
174+
.setOnMatch(Result.ACCEPT)
175+
.setOnMismatch(Result.DENY)
176+
.build();
177+
178+
assertNotNull(filter);
179+
180+
assertEquals(regex, filter.getRegex());
181+
assertFalse(filter.isUseRawMessage());
182+
assertEquals(Result.ACCEPT, filter.getOnMatch());
183+
assertEquals(Result.DENY, filter.getOnMismatch());
184+
assertNotNull(filter.getPattern());
185+
assertEquals(regex, filter.getPattern().pattern());
186+
}
187+
188+
/**
189+
* A builder with no 'regex' expression should both be invalid and return null on 'build()'.
190+
*/
191+
@Test
192+
void testBuilderWithoutRegexNotValid() {
193+
final RegexFilter.Builder builder = RegexFilter.newBuilder();
194+
assertThrows(IllegalArgumentException.class, builder::build);
195+
}
196+
197+
/**
198+
* A builder with an invalid 'regex' expression should return null on 'build()'.
199+
*/
200+
@Test
201+
void testBuilderWithInvalidRegexNotValid() {
202+
final RegexFilter.Builder builder = RegexFilter.newBuilder();
203+
builder.setRegex("[a-z");
204+
assertThrows(IllegalArgumentException.class, builder::build);
205+
}
119206
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilter.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
package org.apache.logging.log4j.core.filter;
1818

19+
import java.util.Objects;
20+
import java.util.Optional;
1921
import org.apache.logging.log4j.Level;
2022
import org.apache.logging.log4j.Marker;
2123
import org.apache.logging.log4j.core.AbstractLifeCycle;
@@ -43,16 +45,30 @@ public abstract static class AbstractFilterBuilder<B extends AbstractFilterBuild
4345
public static final String ATTR_ON_MISMATCH = "onMismatch";
4446
public static final String ATTR_ON_MATCH = "onMatch";
4547

48+
/**
49+
* The action to perform when a match occurs.
50+
*/
4651
@PluginBuilderAttribute(ATTR_ON_MATCH)
4752
private Result onMatch = Result.NEUTRAL;
4853

54+
/**
55+
* The action to perform when a mismatch occurs.
56+
*/
4957
@PluginBuilderAttribute(ATTR_ON_MISMATCH)
5058
private Result onMismatch = Result.DENY;
5159

60+
/**
61+
* Returns the action to apply when a match occurs
62+
* @return the match result
63+
*/
5264
public Result getOnMatch() {
5365
return onMatch;
5466
}
5567

68+
/**
69+
* Returns the action to apply when a mismatch occurs
70+
* @return the mismatch result
71+
*/
5672
public Result getOnMismatch() {
5773
return onMismatch;
5874
}
@@ -110,6 +126,18 @@ protected AbstractFilter(final Result onMatch, final Result onMismatch) {
110126
this.onMismatch = onMismatch == null ? Result.DENY : onMismatch;
111127
}
112128

129+
/**
130+
* Constructs a new instance configured by the given builder
131+
* @param builder the builder
132+
* @throws NullPointerException if the builder argument is {@code null}
133+
* @since 2.27.0
134+
*/
135+
protected AbstractFilter(final AbstractFilterBuilder<?> builder) {
136+
Objects.requireNonNull(builder, "builder");
137+
this.onMatch = Optional.ofNullable(builder.onMatch).orElse(Result.NEUTRAL);
138+
this.onMismatch = Optional.ofNullable(builder.onMismatch).orElse(Result.DENY);
139+
}
140+
113141
@Override
114142
protected boolean equalsImpl(final Object obj) {
115143
if (this == obj) {

0 commit comments

Comments
 (0)