Fix validation behavior in RegexFilter (apache#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

Author: JWT007

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

@@ -34,7 +34,6 @@ public class AbstractFilterTest {
     @Test
     public void testUnrolledBackwardsCompatible() {
         final ConcreteFilter filter = new ConcreteFilter();
-        final Filter.Result expected = Filter.Result.DENY;
         verifyMethodsWithUnrolledVarargs(filter, Filter.Result.DENY);
 
         filter.testResult = Filter.Result.ACCEPT;

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

@@ -53,7 +53,7 @@ public void testAddMultipleSimpleFilters() throws Exception {
         // adding a second filter converts the filter
         // into a CompositeFilter.class
         filterable.addFilter(filter);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
@@ -66,7 +66,7 @@ public void testAddMultipleEqualSimpleFilter() throws Exception {
         // adding a second filter converts the filter
         // into a CompositeFilter.class
         filterable.addFilter(filter);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
@@ -92,7 +92,7 @@ public void testAddMultipleCompositeFilters() throws Exception {
         // adding a second filter converts the filter
         // into a CompositeFilter.class
         filterable.addFilter(compositeFilter);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
@@ -108,7 +108,7 @@ public void testAddSimpleFilterAndCompositeFilter() throws Exception {
         // adding a second filter converts the filter
         // into a CompositeFilter.class
         filterable.addFilter(compositeFilter);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
@@ -124,7 +124,7 @@ public void testAddCompositeFilterAndSimpleFilter() throws Exception {
         // adding a second filter converts the filter
         // into a CompositeFilter.class
         filterable.addFilter(notInCompositeFilterFilter);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
@@ -169,7 +169,7 @@ public void testRemoveSimpleEqualFilterFromMultipleSimpleFilters() throws Except
         filterable.addFilter(filterOriginal);
         filterable.addFilter(filterCopy);
         filterable.removeFilter(filterCopy);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
         filterable.removeFilter(filterCopy);
         assertEquals(filterOriginal, filterable.getFilter());
@@ -223,7 +223,7 @@ public void testRemoveSimpleFilterFromCompositeAndSimpleFilter() {
 
         // should not remove internal filter of compositeFilter
         filterable.removeFilter(anotherFilter);
-        assertTrue(filterable.getFilter() instanceof CompositeFilter);
+        assertInstanceOf(CompositeFilter.class, filterable.getFilter());
         assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
@@ -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

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;
@@ -94,11 +93,10 @@ public 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);
 
@@ -116,26 +114,25 @@ 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);
 
         final Result rawResult = rawFilter.filter(null, null, null, msg, params);
         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);
 
@@ -151,8 +148,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());
 
@@ -172,13 +172,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);
 
@@ -201,7 +200,6 @@ void testBuilderWithoutRegexNotValid() {
         assertFalse(builder.isValid());
 
         assertNull(builder.build());
-
     }
 
     /**
@@ -217,6 +215,5 @@ void testBuilderWithInvalidRegexNotValid() {
         assertFalse(builder.isValid());
 
         assertNull(builder.build());
-
     }
 }