Skip to content

Commit

Permalink
SONARJAVA-4857: Fix FP in S3457 on java.util.logging strings with sin…
Browse files Browse the repository at this point in the history
…gle quotes (#4707)
  • Loading branch information
ADarko22 committed Mar 11, 2024
1 parent b020df8 commit bc4c770
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ void foo(Calendar c) throws IOException {
logger4.log(level, "message " + "...");
logger4.log(level, "message " + param1, new Exception()); // Noncompliant {{Lambda should be used to defer string concatenation.}}

logger4.log(level, "Can't load library \"{0}\"!", "foo"); // Noncompliant {{Single quote "'" must be escaped.}}
logger4.log(level, "Can''t load library \"{0}\"!", "foo"); // Compliant, escaping the single quote with ''
logger4.log(level, "Can't load library \"\"!", new Throwable()); // Compliant, will print: Can't load library ""!
logger4.log(level, "Can't load library \"\"!"); // Compliant, will print: Can't load library ""!

// slf4jLog ========================================================================================================
// slf4jLog is a facade, various logging frameworks can be used under it. It implies that we will only report issues when
// there are obvious mistakes, not when it depends on the underlying framework (even if it works correctly with the common one).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -81,6 +82,7 @@ public abstract class AbstractPrintfChecker extends AbstractMethodDetection {
.build();

protected static final Pattern MESSAGE_FORMAT_PATTERN = Pattern.compile("\\{(?<index>\\d+)(?<type>,\\w+)?(?<style>,[^}]*)?\\}");
protected static final Predicate<String> MESSAGE_FORMAT_PATTERN_PREDICATE = MESSAGE_FORMAT_PATTERN.asPredicate();

@Override
protected MethodMatchers getMethodInvocationMatchers() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private void verifyParametersForMisuse(MethodInvocationTree mit, List<Expression
}
param = param.substring(param.indexOf('$') + 1);
} else if (param.charAt(0) == '<') {
//refers to previous argument
// refers to previous argument
argIndex = Math.max(0, argIndex - 1);
} else {
index++;
Expand Down Expand Up @@ -231,17 +231,22 @@ private boolean checkUnbalancedQuotes(MethodInvocationTree mit, String formatStr
if (LEVELS.contains(mit.methodSymbol().name())) {
return false;
}

String withoutParam = MESSAGE_FORMAT_PATTERN.matcher(formatString).replaceAll("");
int numberQuote = 0;
for (int i = 0; i < withoutParam.length(); ++i) {
if (withoutParam.charAt(i) == '\'') {
numberQuote++;
}
}

boolean unbalancedQuotes = (numberQuote % 2) != 0;
if (unbalancedQuotes) {

if (unbalancedQuotes && MESSAGE_FORMAT_PATTERN_PREDICATE.test(formatString)) {
// Single quotes should be escaped only when unbalanced and in MessageFormat pattern.
reportIssue(mit.arguments().get(0), "Single quote \"'\" must be escaped.");
}

return unbalancedQuotes;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,36 @@ <h2>How to fix it</h2>
<p>A <code>printf-</code>-style format string is a string that contains placeholders, which are replaced by values when the string is printed or
logged. Mismatch in the format specifiers and the arguments provided can lead to incorrect strings being created.</p>
<p>To avoid issues, a developer should ensure that the provided arguments match format specifiers.</p>
<p>Note that <a href="https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/text/MessageFormat.html">MessageFormat</a> is used by most
logging mechanisms, for example <code>java.util.logging.Logger</code>, thus the <em>single quote</em> must be escaped by a <em>double single
quote</em>.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
String.format("Too many arguments %d and %d", 1, 2, 3); // Noncompliant; the third argument '3' is unused
String.format("First {0} and then {1}", "foo", "bar"); //Noncompliant. It appears there is confusion with the use of "java.text.MessageFormat"; parameters "foo" and "bar" will be ignored here
void logging(org.slf4j.Logger slf4jLog, java.util.logging.Logger logger) {
String.format("Too many arguments %d and %d", 1, 2, 3); // Noncompliant - the third argument '3' is unused
String.format("First {0} and then {1}", "foo", "bar"); //Noncompliant - it appears there is confusion with the use of "java.text.MessageFormat" - parameters "foo" and "bar" will be ignored here

org.slf4j.Logger slf4jLog;
slf4jLog.debug("The number: ", 1); // Noncompliant - String contains no format specifiers.

slf4jLog.debug("The number: ", 1); // Noncompliant - String contains no format specifiers.

logger.log(level, "Can't load library \"{0}\"!", "foo"); // Noncompliant - the single quote ' must be escaped
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
String.format("Too many arguments %d and %d", 1, 2);
String.format("First %s and then %s", "foo", "bar");
void logging(org.slf4j.Logger slf4jLog, java.util.logging.Logger logger) {
String.format("Too many arguments %d and %d", 1, 2);
String.format("First %s and then %s", "foo", "bar");

slf4jLog.debug("The number: {}", 1);

org.slf4j.Logger slf4jLog;
slf4jLog.debug("The number: {}", 1);
logger.log(level, "Can''t load library \"{0}\"!", "foo");
}
</pre>
<h2>Resources</h2>
<ul>
<li> <a href="https://wiki.sei.cmu.edu/confluence/x/J9YxBQ">CERT, FIO47-C.</a> - Use valid format strings </li>
<li> <a href="https://docs.oracle.com/javase/8/docs/api/java/text/MessageFormat.html">java.text.MessageFormat</a> </li>
<li> <a href="https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/text/MessageFormat.html">java.text.MessageFormat</a> </li>
</ul>

0 comments on commit bc4c770

Please sign in to comment.