Skip to content

Port log4j-api changes from 2.x #2208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 26, 2024
Merged

Port log4j-api changes from 2.x #2208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c92ffe
`ParameterFormatter` rewrite with improved escape handling (#1639)
vy Aug 14, 2023
b720cc2
[1223] Change FormattedMessage pattern heuristic
ppkarwasz Oct 23, 2023
67be357
Port `log4j-api` changes from `2.x`
ppkarwasz Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
234 changes: 92 additions & 142 deletions log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,156 +16,106 @@
*/
package org.apache.logging.log4j.message;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.assertj.core.api.Assertions.assertThat;

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.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.MethodSource;

/**
* Tests {@link ParameterFormatter}.
*/
public class ParameterFormatterTest {

@Test
public void testCountArgumentPlaceholders() {
assertEquals(0, ParameterFormatter.countArgumentPlaceholders(""));
assertEquals(0, ParameterFormatter.countArgumentPlaceholders("aaa"));
assertEquals(0, ParameterFormatter.countArgumentPlaceholders("\\{}"));
assertEquals(1, ParameterFormatter.countArgumentPlaceholders("{}"));
assertEquals(1, ParameterFormatter.countArgumentPlaceholders("{}\\{}"));
assertEquals(2, ParameterFormatter.countArgumentPlaceholders("{}{}"));
assertEquals(3, ParameterFormatter.countArgumentPlaceholders("{}{}{}"));
assertEquals(4, ParameterFormatter.countArgumentPlaceholders("{}{}{}aa{}"));
assertEquals(4, ParameterFormatter.countArgumentPlaceholders("{}{}{}a{]b{}"));
assertEquals(5, ParameterFormatter.countArgumentPlaceholders("{}{}{}a{}b{}"));
}

@Test
public void testFormat3StringArgs() {
final String testMsg = "Test message {}{} {}";
final String[] args = {"a", "b", "c"};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message ab c", result);
}

@Test
public void testFormatNullArgs() {
final String testMsg = "Test message {} {} {} {} {} {}";
final String[] args = {"a", null, "c", null, null, null};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message a null c null null null", result);
}

@Test
public void testFormatStringArgsIgnoresSuperfluousArgs() {
final String testMsg = "Test message {}{} {}";
final String[] args = {"a", "b", "c", "unnecessary", "superfluous"};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message ab c", result);
}

@Test
public void testFormatStringArgsWithEscape() {
final String testMsg = "Test message \\{}{} {}";
final String[] args = {"a", "b", "c"};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message {}a b", result);
}

@Test
public void testFormatStringArgsWithTrailingEscape() {
final String testMsg = "Test message {}{} {}\\";
final String[] args = {"a", "b", "c"};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message ab c\\", result);
}

@Test
public void testFormatStringArgsWithTrailingEscapedEscape() {
final String testMsg = "Test message {}{} {}\\\\";
final String[] args = {"a", "b", "c"};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message ab c\\\\", result);
}

@Test
public void testFormatStringArgsWithEscapedEscape() {
final String testMsg = "Test message \\\\{}{} {}";
final String[] args = {"a", "b", "c"};
final String result = ParameterFormatter.format(testMsg, args);
assertEquals("Test message \\ab c", result);
}

@Test
public void testFormatMessage3StringArgs() {
final String testMsg = "Test message {}{} {}";
final String[] args = {"a", "b", "c"};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
final String result = sb.toString();
assertEquals("Test message ab c", result);
}

@Test
public void testFormatMessageNullArgs() {
final String testMsg = "Test message {} {} {} {} {} {}";
final String[] args = {"a", null, "c", null, null, null};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 6);
final String result = sb.toString();
assertEquals("Test message a null c null null null", result);
}

@Test
public void testFormatMessageStringArgsIgnoresSuperfluousArgs() {
final String testMsg = "Test message {}{} {}";
final String[] args = {"a", "b", "c", "unnecessary", "superfluous"};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 5);
final String result = sb.toString();
assertEquals("Test message ab c", result);
}

@Test
public void testFormatMessageStringArgsWithEscape() {
final String testMsg = "Test message \\{}{} {}";
final String[] args = {"a", "b", "c"};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
final String result = sb.toString();
assertEquals("Test message {}a b", result);
}

@Test
public void testFormatMessageStringArgsWithTrailingEscape() {
final String testMsg = "Test message {}{} {}\\";
final String[] args = {"a", "b", "c"};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
final String result = sb.toString();
assertEquals("Test message ab c\\", result);
}

@Test
public void testFormatMessageStringArgsWithTrailingEscapedEscape() {
final String testMsg = "Test message {}{} {}\\\\";
final String[] args = {"a", "b", "c"};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
final String result = sb.toString();
assertEquals("Test message ab c\\\\", result);
}

@Test
public void testFormatMessageStringArgsWithEscapedEscape() {
final String testMsg = "Test message \\\\{}{} {}";
final String[] args = {"a", "b", "c"};
final StringBuilder sb = new StringBuilder();
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
final String result = sb.toString();
assertEquals("Test message \\ab c", result);
@ParameterizedTest
@CsvSource({
"0,,false,",
"0,,false,aaa",
"0,,true,\\{}",
"1,0,false,{}",
"1,0,true,{}\\{}",
"1,2,true,\\\\{}",
"2,8:10,true,foo \\{} {}{}",
"2,8:10,true,foo {\\} {}{}",
"2,0:2,false,{}{}",
"3,0:2:4,false,{}{}{}",
"4,0:2:4:8,false,{}{}{}aa{}",
"4,0:2:4:10,false,{}{}{}a{]b{}",
"5,0:2:4:7:10,false,{}{}{}a{}b{}"
})
public void test_pattern_analysis(
final int placeholderCount,
final String placeholderCharIndicesString,
final boolean escapedPlaceholderFound,
final String pattern) {
MessagePatternAnalysis analysis = ParameterFormatter.analyzePattern(pattern, placeholderCount);
assertThat(analysis.placeholderCount).isEqualTo(placeholderCount);
if (placeholderCount > 0) {
final int[] placeholderCharIndices = Arrays.stream(placeholderCharIndicesString.split(":"))
.mapToInt(Integer::parseInt)
.toArray();
assertThat(analysis.placeholderCharIndices).startsWith(placeholderCharIndices);
assertThat(analysis.escapedCharFound).isEqualTo(escapedPlaceholderFound);
}
}

@ParameterizedTest
@MethodSource("messageFormattingTestCases")
void assertMessageFormatting(
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();
assertThat(actualFormattedMessage).isEqualTo(expectedFormattedMessage);
}

static Object[][] messageFormattingTestCases() {
return new Object[][] {
new Object[] {"Test message {}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message ab c"},
new Object[] {
"Test message {} {} {} {} {} {}",
new Object[] {"a", null, "c", null, null, null},
6,
"Test message a null c null null null"
},
new Object[] {
"Test message {}{} {}",
new Object[] {"a", "b", "c", "unnecessary", "superfluous"},
5,
"Test message ab c"
},
new Object[] {"Test message \\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message {}a b"},
new Object[] {"Test message {}{} {}\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
new Object[] {"Test message {}{} {}\\\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
new Object[] {"Test message \\\\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message \\ab c"},
new Object[] {"Test message {}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message ab c"},
new Object[] {
"Test message {} {} {} {} {} {}",
new Object[] {"a", null, "c", null, null, null},
6,
"Test message a null c null null null"
},
new Object[] {
"Test message {}{} {}",
new Object[] {"a", "b", "c", "unnecessary", "superfluous"},
5,
"Test message ab c"
},
new Object[] {"Test message \\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message {}a b"},
new Object[] {"Test message {}{} {}\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
new Object[] {"Test message {}{} {}\\\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
new Object[] {"Test message \\\\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message \\ab c"},
new Object[] {"foo \\\\\\{} {}", new Object[] {"bar"}, 1, "foo \\{} bar"},
new Object[] {"missing arg {} {}", new Object[] {1, 2}, 1, "missing arg 1 {}"},
new Object[] {"foo {\\} {}", new Object[] {"bar"}, 1, "foo {\\} bar"}
};
}

@Test
Expand All @@ -177,7 +127,7 @@ public void testDeepToString() {
list.add(2);
final String actual = ParameterFormatter.deepToString(list);
final String expected = "[1, [..." + ParameterFormatter.identityToString(list) + "...], 2]";
assertEquals(expected, actual);
assertThat(actual).isEqualTo(expected);
}

@Test
Expand All @@ -191,7 +141,7 @@ public void testDeepToStringUsingNonRecursiveButConsequentObjects() {
list.add(3);
final String actual = ParameterFormatter.deepToString(list);
final String expected = "[1, [0], 2, [0], 3]";
assertEquals(expected, actual);
assertThat(actual).isEqualTo(expected);
}

@Test
Expand All @@ -203,6 +153,6 @@ public void testIdentityToString() {
list.add(2);
final String actual = ParameterFormatter.identityToString(list);
final String expected = list.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(list));
assertEquals(expected, actual);
assertThat(actual).isEqualTo(expected);
}
}
2 changes: 1 addition & 1 deletion log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void testFormatStringArgsWithTrailingEscapedEscape() {
final String testMsg = "Test message {}{} {}\\\\";
final String[] args = {"a", "b", "c"};
final String result = ParameterizedMessage.format(testMsg, args);
assertEquals("Test message ab c\\\\", result);
assertEquals("Test message ab c\\", result);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...test/src/test/java/org/apache/logging/log4j/message/ReusableParameterizedMessageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void testFormatStringArgsWithTrailingEscapedEscape() {
final String[] args = {"a", "b", "c"};
final String result =
new ReusableParameterizedMessage().set(testMsg, (Object[]) args).getFormattedMessage();
assertEquals("Test message ab c\\\\", result);
assertEquals("Test message ab c\\", result);
}

@Test
Expand Down
12 changes: 7 additions & 5 deletions log4j-api/src/main/java/org/apache/logging/log4j/message/BasicThreadInformation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
*/
package org.apache.logging.log4j.message;

import org.apache.logging.log4j.util.Chars;
import static org.apache.logging.log4j.util.Chars.LF;
import static org.apache.logging.log4j.util.Chars.SPACE;

import org.apache.logging.log4j.util.StringBuilders;

/**
Expand Down Expand Up @@ -78,16 +80,16 @@ public int hashCode() {
*/
@Override
public void printThreadInfo(final StringBuilder sb) {
StringBuilders.appendDqValue(sb, name).append(Chars.SPACE);
StringBuilders.appendDqValue(sb, name).append(SPACE);
if (isDaemon) {
sb.append("daemon ");
}
sb.append("prio=").append(priority).append(" tid=").append(id).append(' ');
if (threadGroupName != null) {
StringBuilders.appendKeyDqValue(sb, "group", threadGroupName);
}
sb.append('\n');
sb.append("\tThread state: ").append(state.name()).append('\n');
sb.append(LF);
sb.append("\tThread state: ").append(state.name()).append(LF);
}

/**
Expand All @@ -98,7 +100,7 @@ public void printThreadInfo(final StringBuilder sb) {
@Override
public void printStack(final StringBuilder sb, final StackTraceElement[] trace) {
for (final StackTraceElement element : trace) {
sb.append("\tat ").append(element).append('\n');
sb.append("\tat ").append(element).append(LF);
}
}
}
37 changes: 26 additions & 11 deletions log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.Arrays;
import java.util.Locale;
import java.util.Objects;
import java.util.regex.Pattern;

/**
* Handles messages that contain a format String. This converts each message into a {@link MessageFormatMessage},
Expand All @@ -30,9 +29,6 @@
*/
public class FormattedMessage implements Message {

private static final String FORMAT_SPECIFIER = "%(\\d+\\$)?([-#+ 0,(\\<]*)?(\\d+)?(\\.\\d+)?([tT])?([a-zA-Z%])";
private static final Pattern MSG_PATTERN = Pattern.compile(FORMAT_SPECIFIER);

private final String messagePattern;
private final Object[] argArray;
private String formattedMessage;
Expand Down Expand Up @@ -168,7 +164,27 @@ public String getFormattedMessage() {
return formattedMessage;
}

/**
* Gets the message implementation to which formatting is delegated.
*
* <ul>
* <li>if {@code msgPattern} contains {@link MessageFormat} format specifiers a {@link MessageFormatMessage}
* is returned,</li>
* <li>if {@code msgPattern} contains {@code {}} placeholders a {@link ParameterizedMessage} is returned,</li>
* <li>if {@code msgPattern} contains {@link Format} specifiers a {@link StringFormattedMessage} is returned
* .</li>
* </ul>
* <p>
* Mixing specifiers from multiple types is not supported.
* </p>
*
* @param msgPattern The message pattern.
* @param args The parameters.
* @param aThrowable The throwable
* @return The message that performs formatting.
*/
protected Message getMessage(final String msgPattern, final Object[] args, final Throwable aThrowable) {
// Check for valid `{ ArgumentIndex [, FormatType [, FormatStyle]] }` format specifiers
try {
final MessageFormat format = new MessageFormat(msgPattern);
final Format[] formats = format.getFormats();
Expand All @@ -178,14 +194,13 @@ protected Message getMessage(final String msgPattern, final Object[] args, final
} catch (final Exception ignored) {
// Obviously, the message is not a proper pattern for MessageFormat.
}
try {
if (MSG_PATTERN.matcher(msgPattern).find()) {
return new StringFormattedMessage(locale, msgPattern, args);
}
} catch (final Exception ignored) {
// Also not properly formatted.
// Check for non-escaped `{}` format specifiers
// This case also includes patterns without any `java.util.Formatter` specifiers
if (ParameterFormatter.analyzePattern(msgPattern, 1).placeholderCount > 0 || msgPattern.indexOf('%') == -1) {
return new ParameterizedMessage(msgPattern, args, aThrowable);
}
return new ParameterizedMessage(msgPattern, args, aThrowable);
// Interpret as `java.util.Formatter` format
return new StringFormattedMessage(locale, msgPattern, args);
}

/**
Expand Down
Loading