FormattedMessage doesn't need to use regular expressions

[jvz]

In order to determine which message format syntax to use in FormattedMessage, this uses a regular expression. This can be simplified by flipping the ordering to check for a parametrized message first before assuming it's a printf-style message.

[adwsingh]

@jvz I was looking into this. How do you suggest we check for a parametrized message, by doing countArgumentPlaceholders(msg) > 0 ?

[ppkarwasz]

@adwsingh, that sounds about right.

I marked you as "assignee" to prevent other contributors to work on it. It's not an obligation.

[adwsingh]

@ppkarwasz after reading the docs I am not so sure of my implementation anymore as I think it would break users.

In docs we specify,

The message pattern passed to a FormattedMessage is first checked to see if it is a valid java.text.MessageFormat pattern. If it is, a MessageFormatMessage is used to format it. If not it is next checked to see if it contains any tokens that are valid format specifiers for String.format(). If so, a StringFormattedMessage is used to format it. Finally, if the pattern doesn't match either of those then a ParameterizedMessage is used to format it.

For a message like logger.error("Test message {} %s", "abc");, we would now log Test message abc %s instead of Test message {} abc, which would go against what we specify in the docs.

[ppkarwasz]

@adwsingh,

Yes, it is a breaking change to silence Code scanning alert #51: checking for an unescaped {} is much faster than checking for a java.util.Formatter format specifier.

I believe we can change the documentation to state:

The message factory supports the following format specifiers:

• those specified by java.text.MessageFormat,
• those specified by java.util.Formatter,
• {} placeholders (cf. ParameterizedMessage).
  Mixing specifier from these 3 categories is not supported.

BTW: The regex we use currently is identical to java.util.Formatter.formatSpecifier used by the JRE, so this does not change anything security-wise.