Skip to content

Commit 8c92ffe

Browse files
vyppkarwasz
vy
authored and
ppkarwasz
committed
ParameterFormatter rewrite with improved escape handling (#1639)
1 parent 65a837b commit 8c92ffe

File tree

8 files changed

+514
-711
lines changed

8 files changed

+514
-711
lines changed

‎log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java

Lines changed: 92 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -16,156 +16,106 @@
1616
*/
1717
package org.apache.logging.log4j.message;
1818

19-
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
import static org.assertj.core.api.Assertions.assertThat;
2020

2121
import java.util.ArrayList;
22+
import java.util.Arrays;
2223
import java.util.Collections;
2324
import java.util.List;
25+
import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis;
2426
import org.junit.jupiter.api.Test;
27+
import org.junit.jupiter.params.ParameterizedTest;
28+
import org.junit.jupiter.params.provider.CsvSource;
29+
import org.junit.jupiter.params.provider.MethodSource;
2530

2631
/**
2732
* Tests {@link ParameterFormatter}.
2833
*/
2934
public class ParameterFormatterTest {
3035

31-
@Test
32-
public void testCountArgumentPlaceholders() {
33-
assertEquals(0, ParameterFormatter.countArgumentPlaceholders(""));
34-
assertEquals(0, ParameterFormatter.countArgumentPlaceholders("aaa"));
35-
assertEquals(0, ParameterFormatter.countArgumentPlaceholders("\\{}"));
36-
assertEquals(1, ParameterFormatter.countArgumentPlaceholders("{}"));
37-
assertEquals(1, ParameterFormatter.countArgumentPlaceholders("{}\\{}"));
38-
assertEquals(2, ParameterFormatter.countArgumentPlaceholders("{}{}"));
39-
assertEquals(3, ParameterFormatter.countArgumentPlaceholders("{}{}{}"));
40-
assertEquals(4, ParameterFormatter.countArgumentPlaceholders("{}{}{}aa{}"));
41-
assertEquals(4, ParameterFormatter.countArgumentPlaceholders("{}{}{}a{]b{}"));
42-
assertEquals(5, ParameterFormatter.countArgumentPlaceholders("{}{}{}a{}b{}"));
43-
}
44-
45-
@Test
46-
public void testFormat3StringArgs() {
47-
final String testMsg = "Test message {}{} {}";
48-
final String[] args = {"a", "b", "c"};
49-
final String result = ParameterFormatter.format(testMsg, args);
50-
assertEquals("Test message ab c", result);
51-
}
52-
53-
@Test
54-
public void testFormatNullArgs() {
55-
final String testMsg = "Test message {} {} {} {} {} {}";
56-
final String[] args = {"a", null, "c", null, null, null};
57-
final String result = ParameterFormatter.format(testMsg, args);
58-
assertEquals("Test message a null c null null null", result);
59-
}
60-
61-
@Test
62-
public void testFormatStringArgsIgnoresSuperfluousArgs() {
63-
final String testMsg = "Test message {}{} {}";
64-
final String[] args = {"a", "b", "c", "unnecessary", "superfluous"};
65-
final String result = ParameterFormatter.format(testMsg, args);
66-
assertEquals("Test message ab c", result);
67-
}
68-
69-
@Test
70-
public void testFormatStringArgsWithEscape() {
71-
final String testMsg = "Test message \\{}{} {}";
72-
final String[] args = {"a", "b", "c"};
73-
final String result = ParameterFormatter.format(testMsg, args);
74-
assertEquals("Test message {}a b", result);
75-
}
76-
77-
@Test
78-
public void testFormatStringArgsWithTrailingEscape() {
79-
final String testMsg = "Test message {}{} {}\\";
80-
final String[] args = {"a", "b", "c"};
81-
final String result = ParameterFormatter.format(testMsg, args);
82-
assertEquals("Test message ab c\\", result);
83-
}
84-
85-
@Test
86-
public void testFormatStringArgsWithTrailingEscapedEscape() {
87-
final String testMsg = "Test message {}{} {}\\\\";
88-
final String[] args = {"a", "b", "c"};
89-
final String result = ParameterFormatter.format(testMsg, args);
90-
assertEquals("Test message ab c\\\\", result);
91-
}
92-
93-
@Test
94-
public void testFormatStringArgsWithEscapedEscape() {
95-
final String testMsg = "Test message \\\\{}{} {}";
96-
final String[] args = {"a", "b", "c"};
97-
final String result = ParameterFormatter.format(testMsg, args);
98-
assertEquals("Test message \\ab c", result);
99-
}
100-
101-
@Test
102-
public void testFormatMessage3StringArgs() {
103-
final String testMsg = "Test message {}{} {}";
104-
final String[] args = {"a", "b", "c"};
105-
final StringBuilder sb = new StringBuilder();
106-
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
107-
final String result = sb.toString();
108-
assertEquals("Test message ab c", result);
109-
}
110-
111-
@Test
112-
public void testFormatMessageNullArgs() {
113-
final String testMsg = "Test message {} {} {} {} {} {}";
114-
final String[] args = {"a", null, "c", null, null, null};
115-
final StringBuilder sb = new StringBuilder();
116-
ParameterFormatter.formatMessage(sb, testMsg, args, 6);
117-
final String result = sb.toString();
118-
assertEquals("Test message a null c null null null", result);
119-
}
120-
121-
@Test
122-
public void testFormatMessageStringArgsIgnoresSuperfluousArgs() {
123-
final String testMsg = "Test message {}{} {}";
124-
final String[] args = {"a", "b", "c", "unnecessary", "superfluous"};
125-
final StringBuilder sb = new StringBuilder();
126-
ParameterFormatter.formatMessage(sb, testMsg, args, 5);
127-
final String result = sb.toString();
128-
assertEquals("Test message ab c", result);
129-
}
130-
131-
@Test
132-
public void testFormatMessageStringArgsWithEscape() {
133-
final String testMsg = "Test message \\{}{} {}";
134-
final String[] args = {"a", "b", "c"};
135-
final StringBuilder sb = new StringBuilder();
136-
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
137-
final String result = sb.toString();
138-
assertEquals("Test message {}a b", result);
139-
}
140-
141-
@Test
142-
public void testFormatMessageStringArgsWithTrailingEscape() {
143-
final String testMsg = "Test message {}{} {}\\";
144-
final String[] args = {"a", "b", "c"};
145-
final StringBuilder sb = new StringBuilder();
146-
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
147-
final String result = sb.toString();
148-
assertEquals("Test message ab c\\", result);
149-
}
150-
151-
@Test
152-
public void testFormatMessageStringArgsWithTrailingEscapedEscape() {
153-
final String testMsg = "Test message {}{} {}\\\\";
154-
final String[] args = {"a", "b", "c"};
155-
final StringBuilder sb = new StringBuilder();
156-
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
157-
final String result = sb.toString();
158-
assertEquals("Test message ab c\\\\", result);
159-
}
160-
161-
@Test
162-
public void testFormatMessageStringArgsWithEscapedEscape() {
163-
final String testMsg = "Test message \\\\{}{} {}";
164-
final String[] args = {"a", "b", "c"};
165-
final StringBuilder sb = new StringBuilder();
166-
ParameterFormatter.formatMessage(sb, testMsg, args, 3);
167-
final String result = sb.toString();
168-
assertEquals("Test message \\ab c", result);
36+
@ParameterizedTest
37+
@CsvSource({
38+
"0,,false,",
39+
"0,,false,aaa",
40+
"0,,true,\\{}",
41+
"1,0,false,{}",
42+
"1,0,true,{}\\{}",
43+
"1,2,true,\\\\{}",
44+
"2,8:10,true,foo \\{} {}{}",
45+
"2,8:10,true,foo {\\} {}{}",
46+
"2,0:2,false,{}{}",
47+
"3,0:2:4,false,{}{}{}",
48+
"4,0:2:4:8,false,{}{}{}aa{}",
49+
"4,0:2:4:10,false,{}{}{}a{]b{}",
50+
"5,0:2:4:7:10,false,{}{}{}a{}b{}"
51+
})
52+
public void test_pattern_analysis(
53+
final int placeholderCount,
54+
final String placeholderCharIndicesString,
55+
final boolean escapedPlaceholderFound,
56+
final String pattern) {
57+
MessagePatternAnalysis analysis = ParameterFormatter.analyzePattern(pattern, placeholderCount);
58+
assertThat(analysis.placeholderCount).isEqualTo(placeholderCount);
59+
if (placeholderCount > 0) {
60+
final int[] placeholderCharIndices = Arrays.stream(placeholderCharIndicesString.split(":"))
61+
.mapToInt(Integer::parseInt)
62+
.toArray();
63+
assertThat(analysis.placeholderCharIndices).startsWith(placeholderCharIndices);
64+
assertThat(analysis.escapedCharFound).isEqualTo(escapedPlaceholderFound);
65+
}
66+
}
67+
68+
@ParameterizedTest
69+
@MethodSource("messageFormattingTestCases")
70+
void assertMessageFormatting(
71+
final String pattern, final Object[] args, final int argCount, final String expectedFormattedMessage) {
72+
MessagePatternAnalysis analysis = ParameterFormatter.analyzePattern(pattern, -1);
73+
final StringBuilder buffer = new StringBuilder();
74+
ParameterFormatter.formatMessage(buffer, pattern, args, argCount, analysis);
75+
String actualFormattedMessage = buffer.toString();
76+
assertThat(actualFormattedMessage).isEqualTo(expectedFormattedMessage);
77+
}
78+
79+
static Object[][] messageFormattingTestCases() {
80+
return new Object[][] {
81+
new Object[] {"Test message {}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message ab c"},
82+
new Object[] {
83+
"Test message {} {} {} {} {} {}",
84+
new Object[] {"a", null, "c", null, null, null},
85+
6,
86+
"Test message a null c null null null"
87+
},
88+
new Object[] {
89+
"Test message {}{} {}",
90+
new Object[] {"a", "b", "c", "unnecessary", "superfluous"},
91+
5,
92+
"Test message ab c"
93+
},
94+
new Object[] {"Test message \\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message {}a b"},
95+
new Object[] {"Test message {}{} {}\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
96+
new Object[] {"Test message {}{} {}\\\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
97+
new Object[] {"Test message \\\\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message \\ab c"},
98+
new Object[] {"Test message {}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message ab c"},
99+
new Object[] {
100+
"Test message {} {} {} {} {} {}",
101+
new Object[] {"a", null, "c", null, null, null},
102+
6,
103+
"Test message a null c null null null"
104+
},
105+
new Object[] {
106+
"Test message {}{} {}",
107+
new Object[] {"a", "b", "c", "unnecessary", "superfluous"},
108+
5,
109+
"Test message ab c"
110+
},
111+
new Object[] {"Test message \\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message {}a b"},
112+
new Object[] {"Test message {}{} {}\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
113+
new Object[] {"Test message {}{} {}\\\\", new Object[] {"a", "b", "c"}, 3, "Test message ab c\\"},
114+
new Object[] {"Test message \\\\{}{} {}", new Object[] {"a", "b", "c"}, 3, "Test message \\ab c"},
115+
new Object[] {"foo \\\\\\{} {}", new Object[] {"bar"}, 1, "foo \\{} bar"},
116+
new Object[] {"missing arg {} {}", new Object[] {1, 2}, 1, "missing arg 1 {}"},
117+
new Object[] {"foo {\\} {}", new Object[] {"bar"}, 1, "foo {\\} bar"}
118+
};
169119
}
170120

171121
@Test
@@ -177,7 +127,7 @@ public void testDeepToString() {
177127
list.add(2);
178128
final String actual = ParameterFormatter.deepToString(list);
179129
final String expected = "[1, [..." + ParameterFormatter.identityToString(list) + "...], 2]";
180-
assertEquals(expected, actual);
130+
assertThat(actual).isEqualTo(expected);
181131
}
182132

183133
@Test
@@ -191,7 +141,7 @@ public void testDeepToStringUsingNonRecursiveButConsequentObjects() {
191141
list.add(3);
192142
final String actual = ParameterFormatter.deepToString(list);
193143
final String expected = "[1, [0], 2, [0], 3]";
194-
assertEquals(expected, actual);
144+
assertThat(actual).isEqualTo(expected);
195145
}
196146

197147
@Test
@@ -203,6 +153,6 @@ public void testIdentityToString() {
203153
list.add(2);
204154
final String actual = ParameterFormatter.identityToString(list);
205155
final String expected = list.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(list));
206-
assertEquals(expected, actual);
156+
assertThat(actual).isEqualTo(expected);
207157
}
208158
}

‎log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public void testFormatStringArgsWithTrailingEscapedEscape() {
115115
final String testMsg = "Test message {}{} {}\\\\";
116116
final String[] args = {"a", "b", "c"};
117117
final String result = ParameterizedMessage.format(testMsg, args);
118-
assertEquals("Test message ab c\\\\", result);
118+
assertEquals("Test message ab c\\", result);
119119
}
120120

121121
@Test

‎log4j-api-test/src/test/java/org/apache/logging/log4j/message/ReusableParameterizedMessageTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void testFormatStringArgsWithTrailingEscapedEscape() {
112112
final String[] args = {"a", "b", "c"};
113113
final String result =
114114
new ReusableParameterizedMessage().set(testMsg, (Object[]) args).getFormattedMessage();
115-
assertEquals("Test message ab c\\\\", result);
115+
assertEquals("Test message ab c\\", result);
116116
}
117117

118118
@Test

0 commit comments

Comments
 (0)