Skip to content

Commit 283dcff

Browse files
JWT007JWT007
committed
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
1 parent 0ac7478 commit 283dcff

File tree

4 files changed

+159
-140
lines changed

4 files changed

+159
-140
lines changed

‎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: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import static org.junit.jupiter.api.Assertions.assertNotNull;
2525
import static org.junit.jupiter.api.Assertions.assertNull;
2626
import static org.junit.jupiter.api.Assertions.assertSame;
27-
import static org.junit.jupiter.api.Assertions.assertThrows;
2827
import static org.junit.jupiter.api.Assertions.assertTrue;
2928

3029
import org.apache.logging.log4j.Level;
@@ -92,11 +91,10 @@ void testDotAllPattern() throws Exception {
9291
@Test
9392
void testNoMsg() {
9493

95-
final RegexFilter filter =
96-
RegexFilter.newBuilder()
97-
.setRegex(".* test .*")
98-
.setUseRawMsg(false)
99-
.build();
94+
final RegexFilter filter = RegexFilter.newBuilder()
95+
.setRegex(".* test .*")
96+
.setUseRawMsg(false)
97+
.build();
10098

10199
assertNotNull(filter);
102100

@@ -114,26 +112,25 @@ void testParameterizedMsg() {
114112
final Object[] params = {"foo", "bar"};
115113

116114
// match against raw message
117-
final RegexFilter rawFilter =
118-
RegexFilter.newBuilder()
119-
.setRegex("params \\{\\} \\{\\}")
120-
.setUseRawMsg(true)
121-
.setOnMatch(Result.ACCEPT)
122-
.setOnMismatch(Result.DENY)
123-
.build();
115+
final RegexFilter rawFilter = RegexFilter.newBuilder()
116+
.setRegex("params \\{\\} \\{\\}")
117+
.setUseRawMsg(true)
118+
.setOnMatch(Result.ACCEPT)
119+
.setOnMismatch(Result.DENY)
120+
.build();
124121

125122
assertNotNull(rawFilter);
126123

127124
final Result rawResult = rawFilter.filter(null, null, null, msg, params);
128125
assertThat(rawResult, equalTo(Result.ACCEPT));
129126

130127
// match against formatted message
131-
final RegexFilter fmtFilter =
132-
RegexFilter.newBuilder()
133-
.setRegex("params foo bar")
134-
.setUseRawMsg(false)
135-
.setOnMatch(Result.ACCEPT)
136-
.setOnMismatch(Result.DENY).build();
128+
final RegexFilter fmtFilter = RegexFilter.newBuilder()
129+
.setRegex("params foo bar")
130+
.setUseRawMsg(false)
131+
.setOnMatch(Result.ACCEPT)
132+
.setOnMismatch(Result.DENY)
133+
.build();
137134

138135
assertNotNull(fmtFilter);
139136

@@ -149,8 +146,11 @@ void testWithValidRegex() {
149146

150147
final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores
151148

152-
final RegexFilter.Builder builder =
153-
RegexFilter.newBuilder().setRegex(regex).setUseRawMsg(false).setOnMatch(Result.ACCEPT).setOnMismatch(Result.DENY);
149+
final RegexFilter.Builder builder = RegexFilter.newBuilder()
150+
.setRegex(regex)
151+
.setUseRawMsg(false)
152+
.setOnMatch(Result.ACCEPT)
153+
.setOnMismatch(Result.DENY);
154154

155155
assertTrue(builder.isValid());
156156

@@ -170,13 +170,12 @@ void testRegexFilterGetters() {
170170

171171
final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores
172172

173-
final RegexFilter filter =
174-
RegexFilter.newBuilder()
175-
.setRegex(regex)
176-
.setUseRawMsg(false)
177-
.setOnMatch(Result.ACCEPT)
178-
.setOnMismatch(Result.DENY)
179-
.build();
173+
final RegexFilter filter = RegexFilter.newBuilder()
174+
.setRegex(regex)
175+
.setUseRawMsg(false)
176+
.setOnMatch(Result.ACCEPT)
177+
.setOnMismatch(Result.DENY)
178+
.build();
180179

181180
assertNotNull(filter);
182181

@@ -199,7 +198,6 @@ void testBuilderWithoutRegexNotValid() {
199198
assertFalse(builder.isValid());
200199

201200
assertNull(builder.build());
202-
203201
}
204202

205203
/**
@@ -215,6 +213,5 @@ void testBuilderWithInvalidRegexNotValid() {
215213
assertFalse(builder.isValid());
216214

217215
assertNull(builder.build());
218-
219216
}
220217
}

0 commit comments

Comments
 (0)