From f592d2919bd52769846b582bf9aeb5e3e6e934f0 Mon Sep 17 00:00:00 2001 From: JWT Date: Sun, 3 Nov 2024 22:11:56 +0100 Subject: [PATCH] Improve validation for StringMatchFilter for null/empty text #3153 (#3158) Add improved validation in StringMatchFilter for null/empty text Co-authored-by: Jeff Thomas --- .../core/filter/StringMatchFilterTest.java | 88 +++++++++++++++++++ .../log4j2-stringmatchfilter-3153-nok.xml | 20 +++++ .../log4j2-stringmatchfilter-3153-ok.xml | 20 +++++ .../log4j/core/filter/StringMatchFilter.java | 13 ++- .../3153_fix_StringMatchFilter_guardNPE.xml | 8 ++ 5 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/StringMatchFilterTest.java create mode 100644 log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-nok.xml create mode 100644 log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-ok.xml create mode 100644 src/changelog/.2.x.x/3153_fix_StringMatchFilter_guardNPE.xml diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/StringMatchFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/StringMatchFilterTest.java new file mode 100644 index 00000000000..0972279b07a --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/StringMatchFilterTest.java @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.logging.log4j.core.filter; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.apache.logging.log4j.core.Filter; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.test.junit.LoggerContextSource; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * Unit test for {@link StringMatchFilter}. + */ +public class StringMatchFilterTest { + + /** + * Test that if no match-string is set on the builder, the '{@link StringMatchFilter.Builder#build()}' returns + * {@code null}. + */ + @Test + public void testFilterBuilderFailsWithNullText() { + Assertions.assertNull(StringMatchFilter.newBuilder().build()); + } + + /** + * Test that if a {@code null} string is set as a match-pattern, an {@code IllegalArgumentExeption} is thrown. + */ + @Test + void testFilterBuilderFailsWithExceptionOnNullText() { + Assertions.assertThrows(IllegalArgumentException.class, () -> StringMatchFilter.newBuilder() + .setMatchString(null)); + } + + /** + * Test that if an empty ({@code ""}) string is set as a match-pattern, an {@code IllegalArgumentException} is thrown. + */ + @Test + void testFilterBuilderFailsWithExceptionOnEmptyText() { + Assertions.assertThrows(IllegalArgumentException.class, () -> StringMatchFilter.newBuilder() + .setMatchString("")); + } + + /** + * Test that if a {@link StringMatchFilter} is specified with a 'text' attribute it is correctly instantiated. + * + * @param configuration the configuration + */ + @Test + @LoggerContextSource("log4j2-stringmatchfilter-3153-ok.xml") + void testConfigurationWithTextPOS(final Configuration configuration) { + final Filter filter = configuration.getFilter(); + assertNotNull(filter, "The filter should not be null."); + assertInstanceOf( + StringMatchFilter.class, filter, "Expected a StringMatchFilter, but got: " + filter.getClass()); + assertEquals("FooBar", filter.toString()); + } + + /** + * Test that if a {@link StringMatchFilter} is specified without a 'text' attribute it is not instantiated. + * + * @param configuration the configuration + */ + @Test + @LoggerContextSource("log4j2-stringmatchfilter-3153-nok.xml") + void testConfigurationWithTextNEG(final Configuration configuration) { + final Filter filter = configuration.getFilter(); + assertNull(filter, "The filter should be null."); + } +} diff --git a/log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-nok.xml b/log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-nok.xml new file mode 100644 index 00000000000..8f3755a2201 --- /dev/null +++ b/log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-nok.xml @@ -0,0 +1,20 @@ + + + + + diff --git a/log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-ok.xml b/log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-ok.xml new file mode 100644 index 00000000000..1ac8c144af7 --- /dev/null +++ b/log4j-core-test/src/test/resources/log4j2-stringmatchfilter-3153-ok.xml @@ -0,0 +1,20 @@ + + + + + diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java index 96a91c18304..31de4258661 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java @@ -25,6 +25,8 @@ import org.apache.logging.log4j.core.config.plugins.Plugin; 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.validation.constraints.Required; +import org.apache.logging.log4j.core.util.Assert; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.util.PerformanceSensitive; @@ -41,7 +43,7 @@ public final class StringMatchFilter extends AbstractFilter { private StringMatchFilter(final String text, final Result onMatch, final Result onMismatch) { super(onMatch, onMismatch); - this.text = text; + this.text = Assert.requireNonEmpty(text, "text"); } @Override @@ -235,8 +237,10 @@ public static StringMatchFilter.Builder newBuilder() { public static class Builder extends AbstractFilterBuilder implements org.apache.logging.log4j.core.util.Builder { + @PluginBuilderAttribute - private String text = ""; + @Required(message = "No text provided for StringMatchFilter") + private String text; /** * Sets the text to search in event messages. @@ -244,12 +248,15 @@ public static class Builder extends AbstractFilterBuilder + + + Add improved validation to StringMatchFilter for null/empty text. GitHub issue #3153. +