vy commented on code in PR #1639:
URL: https://github.com/apache/logging-log4j2/pull/1639#discussion_r1280739791
##########
log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java:
##########
@@ -119,30 +119,28 @@ public Message memento() {
return new ParameterizedMessage(messagePattern, getTrimmedParams());
}
- private void init(final String messagePattern, final int argCount, final
Object[] paramArray) {
+ private void init(final String messagePattern, final int argCount, final
Object[] args) {
this.varargs = null;
this.messagePattern = messagePattern;
this.argCount = argCount;
- final int placeholderCount = count(messagePattern, indices);
- initThrowable(paramArray, argCount, placeholderCount);
- this.usedCount = Math.min(placeholderCount, argCount);
+ this.patternAnalysis = analyzePattern(messagePattern, patternAnalysis);
+ this.throwable = determineThrowable(args, argCount,
patternAnalysis.placeholderCount);
}
- private static int count(final String messagePattern, final int[] indices)
{
- try {
- // try the fast path first
- return
ParameterFormatter.countArgumentPlaceholders2(messagePattern, indices);
- } catch (final Exception ex) { // fallback if more than int[] length
(256) parameter placeholders
- return
ParameterFormatter.countArgumentPlaceholders(messagePattern);
- }
+ private static MessagePatternAnalysis analyzePattern(final String pattern,
final MessagePatternAnalysis patternAnalysis) {
+ return (patternAnalysis != null &&
ParameterFormatter.analyzePattern(pattern, patternAnalysis))
+ ? patternAnalysis
+ : ParameterFormatter.analyzePattern(pattern);
Review Comment:
See the changes in a996006f6d1fcccd7bd37c4ec5e308ddaf891fb3, please.
##########
log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java:
##########
@@ -65,310 +65,235 @@ final class ParameterFormatter {
private static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ")
.withZone(ZoneId.systemDefault());
- private ParameterFormatter() {
- }
+ private ParameterFormatter() {}
/**
- * Counts the number of unescaped placeholders in the given messagePattern.
+ * Analyzes – finds argument placeholder (i.e., {@literal "{}"})
occurrences, etc. – the given message pattern.
+ * <p>
+ * Only {@literal "{}"} strings are treated as argument placeholders.
+ * Escaped or incomplete argument placeholders will be ignored.
+ * Some invalid argument placeholder examples:
+ * </p>
+ * <pre>
+ * { }
+ * foo\{}
+ * {bar
+ * {buzz}
+ * </pre>
*
- * @param messagePattern the message pattern to be analyzed.
- * @return the number of unescaped placeholders.
+ * @param pattern a message pattern to be analyzed
+ * @return the analysis result
*/
- static int countArgumentPlaceholders(final String messagePattern) {
- if (messagePattern == null) {
- return 0;
- }
- final int length = messagePattern.length();
- int result = 0;
- boolean isEscaped = false;
- for (int i = 0; i < length - 1; i++) {
- final char curChar = messagePattern.charAt(i);
- if (curChar == ESCAPE_CHAR) {
- isEscaped = !isEscaped;
- } else if (curChar == DELIM_START) {
- if (!isEscaped && messagePattern.charAt(i + 1) == DELIM_STOP) {
- result++;
- i++;
- }
- isEscaped = false;
- } else {
- isEscaped = false;
- }
- }
- return result;
+ static MessagePatternAnalysis analyzePattern(final String pattern) {
+ final int maxPlaceholderCount = pattern == null ? 0 : pattern.length()
>> 1;
+ MessagePatternAnalysis analysis = new MessagePatternAnalysis();
+ analysis.placeholderCharIndices = new int[maxPlaceholderCount];
Review Comment:
Right. This wasn't easy to implement, see
a996006f6d1fcccd7bd37c4ec5e308ddaf891fb3. I would appreciate it if you can skim
through those changes, in particular,
`PLACEHOLDER_CHAR_INDEX_BUFFER_INITIAL_SIZE` and
`PLACEHOLDER_CHAR_INDEX_BUFFER_SIZE_INCREMENT` constants.
##########
log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java:
##########
@@ -17,156 +17,167 @@
package org.apache.logging.log4j.message;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
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 static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
/**
* 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 \\{} {}{}",
Review Comment:
@ppkarwasz, good point. Added in 763bc271b1bd3dbe4bed0361d2bbab6d688abb28.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
