Skip to content

Commit f592d29

Browse files
JWT007jethomas-tsi
JWT007
and
jethomas-tsi
authored
Improve validation for StringMatchFilter for null/empty text #3153 (#3158)
Add improved validation in StringMatchFilter for null/empty text Co-authored-by: Jeff Thomas <[email protected]>
1 parent 70f058d commit f592d29

File tree

5 files changed

+146
-3
lines changed

5 files changed

+146
-3
lines changed

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

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.filter;
18+
19+
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
21+
import static org.junit.jupiter.api.Assertions.assertNotNull;
22+
import static org.junit.jupiter.api.Assertions.assertNull;
23+
24+
import org.apache.logging.log4j.core.Filter;
25+
import org.apache.logging.log4j.core.config.Configuration;
26+
import org.apache.logging.log4j.core.test.junit.LoggerContextSource;
27+
import org.junit.jupiter.api.Assertions;
28+
import org.junit.jupiter.api.Test;
29+
30+
/**
31+
* Unit test for {@link StringMatchFilter}.
32+
*/
33+
public class StringMatchFilterTest {
34+
35+
/**
36+
* Test that if no match-string is set on the builder, the '{@link StringMatchFilter.Builder#build()}' returns
37+
* {@code null}.
38+
*/
39+
@Test
40+
public void testFilterBuilderFailsWithNullText() {
41+
Assertions.assertNull(StringMatchFilter.newBuilder().build());
42+
}
43+
44+
/**
45+
* Test that if a {@code null} string is set as a match-pattern, an {@code IllegalArgumentExeption} is thrown.
46+
*/
47+
@Test
48+
void testFilterBuilderFailsWithExceptionOnNullText() {
49+
Assertions.assertThrows(IllegalArgumentException.class, () -> StringMatchFilter.newBuilder()
50+
.setMatchString(null));
51+
}
52+
53+
/**
54+
* Test that if an empty ({@code ""}) string is set as a match-pattern, an {@code IllegalArgumentException} is thrown.
55+
*/
56+
@Test
57+
void testFilterBuilderFailsWithExceptionOnEmptyText() {
58+
Assertions.assertThrows(IllegalArgumentException.class, () -> StringMatchFilter.newBuilder()
59+
.setMatchString(""));
60+
}
61+
62+
/**
63+
* Test that if a {@link StringMatchFilter} is specified with a 'text' attribute it is correctly instantiated.
64+
*
65+
* @param configuration the configuration
66+
*/
67+
@Test
68+
@LoggerContextSource("log4j2-stringmatchfilter-3153-ok.xml")
69+
void testConfigurationWithTextPOS(final Configuration configuration) {
70+
final Filter filter = configuration.getFilter();
71+
assertNotNull(filter, "The filter should not be null.");
72+
assertInstanceOf(
73+
StringMatchFilter.class, filter, "Expected a StringMatchFilter, but got: " + filter.getClass());
74+
assertEquals("FooBar", filter.toString());
75+
}
76+
77+
/**
78+
* Test that if a {@link StringMatchFilter} is specified without a 'text' attribute it is not instantiated.
79+
*
80+
* @param configuration the configuration
81+
*/
82+
@Test
83+
@LoggerContextSource("log4j2-stringmatchfilter-3153-nok.xml")
84+
void testConfigurationWithTextNEG(final Configuration configuration) {
85+
final Filter filter = configuration.getFilter();
86+
assertNull(filter, "The filter should be null.");
87+
}
88+
}

‎log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-nok.xml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration status="warn">
19+
<StringMatchFfilter/>
20+
</Configuration>

‎log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-ok.xml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration status="warn">
19+
<StringMatchFilter text="FooBar"/>
20+
</Configuration>

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

+10-3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import org.apache.logging.log4j.core.config.plugins.Plugin;
2626
import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
2727
import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
28+
import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
29+
import org.apache.logging.log4j.core.util.Assert;
2830
import org.apache.logging.log4j.message.Message;
2931
import org.apache.logging.log4j.util.PerformanceSensitive;
3032

@@ -41,7 +43,7 @@ public final class StringMatchFilter extends AbstractFilter {
4143

4244
private StringMatchFilter(final String text, final Result onMatch, final Result onMismatch) {
4345
super(onMatch, onMismatch);
44-
this.text = text;
46+
this.text = Assert.requireNonEmpty(text, "text");
4547
}
4648

4749
@Override
@@ -235,21 +237,26 @@ public static StringMatchFilter.Builder newBuilder() {
235237

236238
public static class Builder extends AbstractFilterBuilder<StringMatchFilter.Builder>
237239
implements org.apache.logging.log4j.core.util.Builder<StringMatchFilter> {
240+
238241
@PluginBuilderAttribute
239-
private String text = "";
242+
@Required(message = "No text provided for StringMatchFilter")
243+
private String text;
240244

241245
/**
242246
* Sets the text to search in event messages.
243247
* @param text the text to search in event messages.
244248
* @return this instance.
245249
*/
246250
public StringMatchFilter.Builder setMatchString(final String text) {
247-
this.text = text;
251+
this.text = Assert.requireNonEmpty(text, "The 'text' argument must not be null or empty.");
248252
return this;
249253
}
250254

251255
@Override
252256
public StringMatchFilter build() {
257+
if (!isValid()) {
258+
return null;
259+
}
253260
return new StringMatchFilter(this.text, this.getOnMatch(), this.getOnMismatch());
254261
}
255262
}

‎src/changelog/.2.x.x/3153_fix_StringMatchFilter_guardNPE.xml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="https://logging.apache.org/xml/ns"
4+
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
5+
type="changed">
6+
<issue id="3153" link="https://github.com/apache/logging-log4j2/issues/3153"/>
7+
<description format="asciidoc">Add improved validation to StringMatchFilter for null/empty text. GitHub issue #3153.</description>
8+
</entry>

0 commit comments

Comments
 (0)