Skip to content

Commit

Permalink
Don't fail on insufficient parameters in ParameterFormatter (#2337)
Browse files Browse the repository at this point in the history
  • Loading branch information
vy committed Mar 4, 2024
1 parent c542041 commit db8604c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand All @@ -32,4 +33,5 @@
@Target({TYPE, METHOD})
@Documented
@ExtendWith({ExtensionContextAnchor.class, StatusLoggerMockExtension.class})
@ResourceLock("log4j2.StatusLogger")
public @interface UsingStatusLoggerMock {}
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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,
Expand All @@ -65,9 +71,24 @@ public void test_pattern_analysis(
}
}

@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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
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;

/**
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -236,7 +240,10 @@ static void formatMessage(
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
Expand All @@ -250,7 +257,7 @@ static void formatMessage(
}
}

static void formatMessageContainingNoEscapes(
private static void formatMessageContainingNoEscapes(
final StringBuilder buffer,
final String pattern,
final Object[] args,
Expand All @@ -271,7 +278,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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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>

0 comments on commit db8604c

Please sign in to comment.