This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch 2.x-ParameterFormatter-insufficient-args in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit db8604c07442ab637714be93246768cd3c370648 Author: Volkan Yazıcı <[email protected]> AuthorDate: Mon Mar 4 11:23:29 2024 +0100 Don't fail on insufficient parameters in `ParameterFormatter` (#2337) --- .../log4j/test/junit/UsingStatusLoggerMock.java | 2 ++ .../log4j/message/ParameterFormatterTest.java | 27 +++++++++++++++++++--- .../logging/log4j/message/ParameterFormatter.java | 13 ++++++++--- ...ix_ParameterFormatter_for_insufficient_args.xml | 7 ++++++ 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusLoggerMock.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusLoggerMock.java index 105eef9e2d..bcada66f7c 100644 --- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusLoggerMock.java +++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusLoggerMock.java @@ -24,6 +24,7 @@ import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.Target; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.ResourceLock; /** * Shortcut to {@link StatusLoggerMockExtension}. @@ -32,4 +33,5 @@ import org.junit.jupiter.api.extension.ExtendWith; @Target({TYPE, METHOD}) @Documented @ExtendWith({ExtensionContextAnchor.class, StatusLoggerMockExtension.class}) +@ResourceLock("log4j2.StatusLogger") public @interface UsingStatusLoggerMock {} diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java index af7a3acc77..9565e3c03e 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java @@ -17,21 +17,27 @@ package org.apache.logging.log4j.message; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; +import org.apache.logging.log4j.status.StatusLogger; +import org.apache.logging.log4j.test.junit.UsingStatusLoggerMock; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentCaptor; /** * Tests {@link ParameterFormatter}. */ -public class ParameterFormatterTest { +@UsingStatusLoggerMock +class ParameterFormatterTest { @ParameterizedTest @CsvSource({ @@ -49,7 +55,7 @@ public class ParameterFormatterTest { "4,0:2:4:10,false,{}{}{}a{]b{}", "5,0:2:4:7:10,false,{}{}{}a{}b{}" }) - public void test_pattern_analysis( + void test_pattern_analysis( final int placeholderCount, final String placeholderCharIndicesString, final boolean escapedPlaceholderFound, @@ -65,9 +71,24 @@ public class ParameterFormatterTest { } } + @ParameterizedTest + @CsvSource({"1,foo {}", "2,bar {}{}"}) + void insufficient_args_should_not_throw_an_exception(final int placeholderCount, final String pattern) { + final int argCount = placeholderCount - 1; + final String formattedPattern = ParameterFormatter.format(pattern, new Object[argCount], argCount); + assertThat(formattedPattern).isEqualTo(pattern); + final ArgumentCaptor<Throwable> errorCaptor = ArgumentCaptor.forClass(Throwable.class); + verify(StatusLogger.getLogger()).error(eq("parameter formatting failure"), errorCaptor.capture()); + final Throwable error = errorCaptor.getValue(); + assertThat(error) + .hasMessage( + "found %d argument placeholders, but provided %d for pattern `%s`", + placeholderCount, argCount, pattern); + } + @ParameterizedTest @MethodSource("messageFormattingTestCases") - void assertMessageFormatting( + void test_message_formatting( final String pattern, final Object[] args, final int argCount, final String expectedFormattedMessage) { MessagePatternAnalysis analysis = ParameterFormatter.analyzePattern(pattern, -1); final StringBuilder buffer = new StringBuilder(); diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java index e059f52d59..e6c0dfa6b8 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java @@ -26,6 +26,8 @@ import java.util.Date; import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.StringBuilders; /** @@ -62,6 +64,8 @@ final class ParameterFormatter { private static final char DELIM_STOP = '}'; private static final char ESCAPE_CHAR = '\\'; + private static final Logger STATUS_LOGGER = StatusLogger.getLogger(); + private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ").withZone(ZoneId.systemDefault()); @@ -236,7 +240,10 @@ final class ParameterFormatter { final String message = String.format( "found %d argument placeholders, but provided %d for pattern `%s`", analysis.placeholderCount, args.length, pattern); - throw new IllegalArgumentException(message); + final Throwable error = new IllegalArgumentException(message); + STATUS_LOGGER.error("parameter formatting failure", error); + buffer.append(pattern); + return; } // Fast-path for patterns containing no escapes @@ -250,7 +257,7 @@ final class ParameterFormatter { } } - static void formatMessageContainingNoEscapes( + private static void formatMessageContainingNoEscapes( final StringBuilder buffer, final String pattern, final Object[] args, @@ -271,7 +278,7 @@ final class ParameterFormatter { buffer.append(pattern, precedingTextStartIndex, pattern.length()); } - static void formatMessageContainingEscapes( + private static void formatMessageContainingEscapes( final StringBuilder buffer, final String pattern, final Object[] args, diff --git a/src/changelog/.2.x.x/fix_ParameterFormatter_for_insufficient_args.xml b/src/changelog/.2.x.x/fix_ParameterFormatter_for_insufficient_args.xml new file mode 100644 index 0000000000..88a4d2f751 --- /dev/null +++ b/src/changelog/.2.x.x/fix_ParameterFormatter_for_insufficient_args.xml @@ -0,0 +1,7 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xmlns="http://logging.apache.org/log4j/changelog" + xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd" + type="fixed"> + <description format="asciidoc">Fix that parameterized message formatting doesn't throw an exception when there are insufficient number of parameters</description> +</entry>
