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 105eef9e2d2..bcada66f7c9 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.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 @@ @Target({TYPE, METHOD}) @Documented @ExtendWith({ExtensionContextAnchor.class, StatusLoggerMockExtension.class}) +@ResourceLock("log4j2.StatusLogger") public @interface UsingStatusLoggerMock {} diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java index 4ead204b3b0..22e32873bee 100644 --- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java +++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java @@ -15,7 +15,7 @@ * limitations under the license. */ @Export -@Version("2.23.0") +@Version("2.23.1") package org.apache.logging.log4j.test.junit; import org.osgi.annotation.bundle.Export; 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 af7a3acc777..e31796368b1 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,12 +17,14 @@ package org.apache.logging.log4j.message; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; 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.test.junit.UsingStatusLoggerMock; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -31,7 +33,8 @@ /** * Tests {@link ParameterFormatter}. */ -public class ParameterFormatterTest { +@UsingStatusLoggerMock +class ParameterFormatterTest { @ParameterizedTest @CsvSource({ @@ -49,7 +52,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,14 +68,22 @@ public void test_pattern_analysis( } } + @ParameterizedTest + @CsvSource({"1,foo {}", "2,bar {}{}"}) + void format_should_fail_on_insufficient_args(final int placeholderCount, final String pattern) { + final int argCount = placeholderCount - 1; + assertThatThrownBy(() -> ParameterFormatter.format(pattern, new Object[argCount], argCount)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "found %d argument placeholders, but provided %d for pattern `%s`", + placeholderCount, argCount, pattern); + } + @ParameterizedTest @MethodSource("messageFormattingTestCases") - void assertMessageFormatting( + void format_should_work( final String pattern, final Object[] args, final int argCount, final String expectedFormattedMessage) { - MessagePatternAnalysis analysis = ParameterFormatter.analyzePattern(pattern, -1); - final StringBuilder buffer = new StringBuilder(); - ParameterFormatter.formatMessage(buffer, pattern, args, argCount, analysis); - String actualFormattedMessage = buffer.toString(); + final String actualFormattedMessage = ParameterFormatter.format(pattern, args, argCount); assertThat(actualFormattedMessage).isEqualTo(expectedFormattedMessage); } diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java index 102b4039ac9..39a5fe39940 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java @@ -19,17 +19,31 @@ import static org.assertj.core.api.Assertions.assertThat; import java.math.BigDecimal; +import java.util.List; +import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.status.StatusData; +import org.apache.logging.log4j.test.ListStatusListener; import org.apache.logging.log4j.test.junit.Mutable; import org.apache.logging.log4j.test.junit.SerialUtil; +import org.apache.logging.log4j.test.junit.UsingStatusListener; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; -public class ParameterizedMessageTest { +@UsingStatusListener +class ParameterizedMessageTest { + + final ListStatusListener statusListener; + + ParameterizedMessageTest(ListStatusListener statusListener) { + this.statusListener = statusListener; + } @Test - public void testNoArgs() { + void testNoArgs() { final String testMsg = "Test message {}"; ParameterizedMessage msg = new ParameterizedMessage(testMsg, (Object[]) null); String result = msg.getFormattedMessage(); @@ -41,7 +55,7 @@ public void testNoArgs() { } @Test - public void testZeroLength() { + void testZeroLength() { final String testMsg = ""; ParameterizedMessage msg = new ParameterizedMessage(testMsg, new Object[] {"arg"}); String result = msg.getFormattedMessage(); @@ -53,7 +67,7 @@ public void testZeroLength() { } @Test - public void testOneCharLength() { + void testOneCharLength() { final String testMsg = "d"; ParameterizedMessage msg = new ParameterizedMessage(testMsg, new Object[] {"arg"}); String result = msg.getFormattedMessage(); @@ -65,7 +79,7 @@ public void testOneCharLength() { } @Test - public void testFormat3StringArgs() { + void testFormat3StringArgs() { final String testMsg = "Test message {}{} {}"; final String[] args = {"a", "b", "c"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -73,7 +87,7 @@ public void testFormat3StringArgs() { } @Test - public void testFormatNullArgs() { + void testFormatNullArgs() { final String testMsg = "Test message {} {} {} {} {} {}"; final String[] args = {"a", null, "c", null, null, null}; final String result = ParameterizedMessage.format(testMsg, args); @@ -81,7 +95,7 @@ public void testFormatNullArgs() { } @Test - public void testFormatStringArgsIgnoresSuperfluousArgs() { + void testFormatStringArgsIgnoresSuperfluousArgs() { final String testMsg = "Test message {}{} {}"; final String[] args = {"a", "b", "c", "unnecessary", "superfluous"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -89,7 +103,7 @@ public void testFormatStringArgsIgnoresSuperfluousArgs() { } @Test - public void testFormatStringArgsWithEscape() { + void testFormatStringArgsWithEscape() { final String testMsg = "Test message \\{}{} {}"; final String[] args = {"a", "b", "c"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -97,7 +111,7 @@ public void testFormatStringArgsWithEscape() { } @Test - public void testFormatStringArgsWithTrailingEscape() { + void testFormatStringArgsWithTrailingEscape() { final String testMsg = "Test message {}{} {}\\"; final String[] args = {"a", "b", "c"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -105,7 +119,7 @@ public void testFormatStringArgsWithTrailingEscape() { } @Test - public void testFormatStringArgsWithTrailingText() { + void testFormatStringArgsWithTrailingText() { final String testMsg = "Test message {}{} {}Text"; final String[] args = {"a", "b", "c"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -113,7 +127,7 @@ public void testFormatStringArgsWithTrailingText() { } @Test - public void testFormatStringArgsWithTrailingEscapedEscape() { + void testFormatStringArgsWithTrailingEscapedEscape() { final String testMsg = "Test message {}{} {}\\\\"; final String[] args = {"a", "b", "c"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -121,7 +135,7 @@ public void testFormatStringArgsWithTrailingEscapedEscape() { } @Test - public void testFormatStringArgsWithEscapedEscape() { + void testFormatStringArgsWithEscapedEscape() { final String testMsg = "Test message \\\\{}{} {}"; final String[] args = {"a", "b", "c"}; final String result = ParameterizedMessage.format(testMsg, args); @@ -129,7 +143,7 @@ public void testFormatStringArgsWithEscapedEscape() { } @Test - public void testSafeWithMutableParams() { // LOG4J2-763 + void testSafeWithMutableParams() { // LOG4J2-763 final String testMsg = "Test message {}"; final Mutable param = new Mutable().set("abc"); final ParameterizedMessage msg = new ParameterizedMessage(testMsg, param); @@ -170,4 +184,51 @@ void testSerializable(final Object arg) { assertThat(actual).isInstanceOf(ParameterizedMessage.class); assertThat(actual.getFormattedMessage()).isEqualTo(expected.getFormattedMessage()); } + + static Stream testCasesForInsufficientFormatArgs() { + return Stream.of(new Object[] {1, "foo {}"}, new Object[] {2, "bar {}{}"}); + } + + @ParameterizedTest + @MethodSource("testCasesForInsufficientFormatArgs") + void formatTo_should_fail_on_insufficient_args(final int placeholderCount, final String pattern) { + final int argCount = placeholderCount - 1; + verifyFormattingFailureOnInsufficientArgs(placeholderCount, pattern, argCount, () -> { + final ParameterizedMessage message = new ParameterizedMessage(pattern, new Object[argCount]); + final StringBuilder buffer = new StringBuilder(); + message.formatTo(buffer); + return buffer.toString(); + }); + } + + @ParameterizedTest + @MethodSource("testCasesForInsufficientFormatArgs") + void format_should_fail_on_insufficient_args(final int placeholderCount, final String pattern) { + final int argCount = placeholderCount - 1; + verifyFormattingFailureOnInsufficientArgs( + placeholderCount, pattern, argCount, () -> ParameterizedMessage.format(pattern, new Object[argCount])); + } + + private void verifyFormattingFailureOnInsufficientArgs( + final int placeholderCount, + final String pattern, + final int argCount, + final Supplier formattedMessageSupplier) { + + // Verify the formatted message + final String formattedMessage = formattedMessageSupplier.get(); + assertThat(formattedMessage).isEqualTo(pattern); + + // Verify the logged failure + final List statusDataList = statusListener.getStatusData().collect(Collectors.toList()); + assertThat(statusDataList).hasSize(1); + final StatusData statusData = statusDataList.get(0); + assertThat(statusData.getLevel()).isEqualTo(Level.ERROR); + assertThat(statusData.getMessage().getFormattedMessage()).isEqualTo("Unable to format msg: %s", pattern); + assertThat(statusData.getThrowable()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "found %d argument placeholders, but provided %d for pattern `%s`", + placeholderCount, argCount, pattern); + } } 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 e059f52d598..58436f8be30 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 @@ -205,11 +205,12 @@ else if (placeholderCount >= placeholderCharIndices.length) { } /** - * Format the following pattern using provided arguments. + * Format the given pattern using provided arguments. * * @param pattern a formatting pattern * @param args arguments to be formatted * @return the formatted message + * @throws IllegalArgumentException on invalid input */ static String format(final String pattern, final Object[] args, int argCount) { final StringBuilder result = new StringBuilder(); @@ -218,6 +219,14 @@ static String format(final String pattern, final Object[] args, int argCount) { return result.toString(); } + /** + * Format the given pattern using provided arguments into the buffer pointed. + * + * @param buffer a buffer the formatted output will be written to + * @param pattern a formatting pattern + * @param args arguments to be formatted + * @throws IllegalArgumentException on invalid input + */ static void formatMessage( final StringBuilder buffer, final String pattern, @@ -250,7 +259,7 @@ static void formatMessage( } } - static void formatMessageContainingNoEscapes( + private static void formatMessageContainingNoEscapes( final StringBuilder buffer, final String pattern, final Object[] args, @@ -271,7 +280,7 @@ static void formatMessageContainingNoEscapes( 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/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java index 4da3642cca1..a5b27e985ee 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java @@ -26,7 +26,9 @@ import java.io.Serializable; import java.util.Arrays; import java.util.Objects; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.StringBuilderFormattable; import org.apache.logging.log4j.util.internal.SerializationUtil; @@ -79,6 +81,8 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable { private static final long serialVersionUID = -665975803997290697L; + private static final Logger STATUS_LOGGER = StatusLogger.getLogger(); + private static final ThreadLocal FORMAT_BUFFER_HOLDER_REF = Constants.ENABLE_THREADLOCALS ? ThreadLocal.withInitial(FormatBufferHolder::new) : null; @@ -274,7 +278,12 @@ public void formatTo(final StringBuilder buffer) { buffer.append(formattedMessage); } else { final int argCount = args != null ? args.length : 0; - ParameterFormatter.formatMessage(buffer, pattern, args, argCount, patternAnalysis); + try { + ParameterFormatter.formatMessage(buffer, pattern, args, argCount, patternAnalysis); + } catch (final Exception error) { + STATUS_LOGGER.error("Unable to format msg: {}", pattern, error); + buffer.append(pattern); + } } } @@ -285,7 +294,12 @@ public void formatTo(final StringBuilder buffer) { */ public static String format(final String pattern, final Object[] args) { final int argCount = args != null ? args.length : 0; - return ParameterFormatter.format(pattern, args, argCount); + try { + return ParameterFormatter.format(pattern, args, argCount); + } catch (final Exception error) { + STATUS_LOGGER.error("Unable to format msg: {}", pattern, error); + return pattern; + } } @Override 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 00000000000..13d97fd2b86 --- /dev/null +++ b/src/changelog/.2.x.x/fix_ParameterFormatter_for_insufficient_args.xml @@ -0,0 +1,8 @@ + + + + Fix that parameterized message formatting doesn't throw an exception when there are insufficient number of parameters +