ppkarwasz commented on code in PR #1639:
URL: https://github.com/apache/logging-log4j2/pull/1639#discussion_r1279544085
##########
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:
This allocation is way too big: the number of arguments provided by the user
is an upper bound for the number of parameters. The additional 57 `{}`
placeholders provided by the user are just literal `{}` as far as we are
concerned.
##########
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:
Nice way to bootstrap the pattern analysis data, but I would make sure that:
- `placeholderCharIndices` has size 10 at least,
- if the number of indices exceeds a preconfigured threshold (e.g. 256),
the `MessagePatternAnalysis` is dropped by `clear()`.
--
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]
