Clarify LogFormatUtils limitLength vs replaceNewlines parameters

[mum-viadee]

During the refactoring of the LogFormatUtils a potential bug has been introduced in 5.3.12 in commit 90fdcf8.

The parameter limitLength in the methodformatValue(...) is used when calling the overloaded method, but it is used for the parameter replaceNewlines that has a totally different meaning. This is on the one hand confusing and on the other hand could affect the fix for CVE-2021-22096

[rstoyanchev]

@mum-viadee thanks for you comment. This was an intentional change, adapting the existing method to the new, three argument, formatValue method, with the idea that when limitLength is true, we also default to replacing newlines, both of which aim for more compact output.

In the framework, this is used in codecs for varied output of the request or response body at DEBUG vs TRACE level. Is that where you are seeing the impact?

[mum-viadee]

I was searching for commits clarifying the impact of CVE-2021-22096 on our application. The commit looked like a fix for CR/LF injections. Therfore I was afraid, that this was a bug because it looked like the CR/LF were only removed when limitLength was true on the existing code.

[senglu]

I think that there is other bug (change in behaviour) in the commit.

Version before changeset truncate to size 100 only when limitLength was true:
return (limitLength && str.length() > 100 ? str.substring(0, 100) + " (truncated)..." : str);

New version contains:

public static String formatValue(@Nullable Object value, boolean limitLength) {
      return formatValue(value, 100, limitLength);  // 100 goes to maxLength
}
...
		if (maxLength != -1) {
			result = (result.length() > maxLength ? result.substring(0, maxLength) + " (truncated)..." : result);
		}

So it sets maxLength to 100 regardless of limitLength value, so it will truncate result in all cases for two parameters formatValue.

Fix:

public static String formatValue(@Nullable Object value, boolean limitLength) {
      return formatValue(value, limitLength ? 100 : -1, limitLength); 
}

[rstoyanchev]

@senglu you're looking at an intermediate change. Currently (and in 5.3.12) it is this:

spring-framework/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java

Line 46 in ff1485f

return formatValue(value, (limitLength ? 100 : -1), limitLength);

[rstoyanchev]

I've updated the Javadoc to make the intent more clear.

@mum-viadee, thanks for feedback. For the future, please use the usual channels for security related concerns or issues. We don't discuss those in public.