Skip to content

Commit

Permalink
Fix issue #2380: Log messages with insufficient parameters should not…
Browse files Browse the repository at this point in the history
… throw exception.
  • Loading branch information
SeasonPanPan authored and ppkarwasz committed Mar 25, 2024
1 parent 11209a3 commit c890ca0
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
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;
Expand Down Expand Up @@ -68,15 +67,12 @@ 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);
@Test
void formatWithInsufficientArgs() {
final String pattern = "Test message {}-{} {}";
final Object[] args = {"a", "b"};
final String message = ParameterFormatter.format(pattern, args, args.length);
assertThat(message).isEqualTo("Test message a-b {}");
}

@ParameterizedTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@
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;
Expand Down Expand Up @@ -185,50 +180,21 @@ void testSerializable(final Object arg) {
assertThat(actual.getFormattedMessage()).isEqualTo(expected.getFormattedMessage());
}

static Stream<Object[]> 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();
});
@Test
void formatToWithInsufficientArgs() {
final String pattern = "Test message {}-{} {}";
final Object[] args = {"a", "b"};
final ParameterizedMessage message = new ParameterizedMessage(pattern, args);
final StringBuilder buffer = new StringBuilder();
message.formatTo(buffer);
assertThat(buffer.toString()).isEqualTo("Test message a-b {}");
}

@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<String> formattedMessageSupplier) {

// Verify the formatted message
final String formattedMessage = formattedMessageSupplier.get();
assertThat(formattedMessage).isEqualTo(pattern);

// Verify the logged failure
final List<StatusData> 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);
@Test
void formatWithInsufficientArgs() {
final String pattern = "Test message {}-{} {}";
final Object[] args = {"a", "b"};
final String message = ParameterizedMessage.format(pattern, args);
assertThat(message).isEqualTo("Test message a-b {}");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -240,14 +240,6 @@ static void formatMessage(
return;
}

// Fail if there are insufficient arguments
if (analysis.placeholderCount > args.length) {
final String message = String.format(
"found %d argument placeholders, but provided %d for pattern `%s`",
analysis.placeholderCount, args.length, pattern);
throw new IllegalArgumentException(message);
}

// Fast-path for patterns containing no escapes
if (analysis.escapedCharFound) {
formatMessageContainingEscapes(buffer, pattern, args, argCount, analysis);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?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">
<issue id="2380" link="https://github.com/apache/logging-log4j2/pull/2380"/>
<description format="asciidoc">
Fix that parameterized message formatting throws an exception when there are insufficient number of parameters.
It previously simply didn't replace the '{}' sequence. The behavior changed in 2.21.0 and should be restored for backward compatibility.
</description>
</entry>

0 comments on commit c890ca0

Please sign in to comment.