vy commented on a change in pull request #444:
URL: https://github.com/apache/logging-log4j2/pull/444#discussion_r532463534
##########
File path:
log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
##########
@@ -493,18 +492,15 @@ public void writeSeparator() {
final S state) {
Objects.requireNonNull(emitter, "emitter");
stringBuilder.append('"');
- formattableBuffer.setLength(0);
- emitter.accept(formattableBuffer, state);
- final int length = formattableBuffer.length();
- // Handle max. string length complying input.
- if (length <= maxStringLength) {
- quoteString(formattableBuffer, 0, length);
- }
- // Handle max. string length violating input.
- else {
- quoteString(formattableBuffer, 0, maxStringLength);
+ int position = stringBuilder.length();
+ emitter.accept(stringBuilder, state);
+ final int length = stringBuilder.length() - position;
+ if (length > maxStringLength) {
+ // Handle max. string length violating input
+ stringBuilder.setLength(position + maxStringLength);
stringBuilder.append(quotedTruncatedStringSuffix);
}
+ StringBuilders.escapeJson(stringBuilder, position);
Review comment:
I fail to see the benefit of this change and have a few objections:
1. I am not inclined to use a separate code base (i.e.,
`StringBuilders.escapeJson()`) given all the necessary JSON logic, including
quoting, is already there. Access to `StringBuilders.escapeJson()` violates the
self-containment contract of the class.
2. `StringBuilders.escapeJson()` might add extra characters violating the
max. string length constraint.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
