Repository: logging-log4j2 Updated Branches: refs/heads/release-2.x 52929d579 -> 363984f1f
[LOG4J2-2373] Reduce unnecessary character movement in the StringBuilders.escapeJson function. This closes #190 Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/363984f1 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/363984f1 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/363984f1 Branch: refs/heads/release-2.x Commit: 363984f1f78f738f1ef9c726dea2cb485fc6bb7b Parents: 52929d5 Author: kmeurer <[email protected]> Authored: Wed Jul 11 16:50:12 2018 -0400 Committer: Carter Kozak <[email protected]> Committed: Wed Jul 11 22:00:43 2018 -0400 ---------------------------------------------------------------------- .../logging/log4j/util/StringBuilders.java | 66 ++++++++++++++------ .../logging/log4j/util/StringBuildersTest.java | 29 +++++++++ src/changes/changes.xml | 4 ++ 3 files changed, 80 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/363984f1/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java ---------------------------------------------------------------------- diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java index a17d195..2a83b24 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java @@ -169,53 +169,81 @@ public final class StringBuilders { } public static void escapeJson(final StringBuilder toAppendTo, final int start) { - for (int i = toAppendTo.length() - 1; i >= start; i--) { // backwards: length may change + int escapeCount = 0; + for (int i = start; i < toAppendTo.length(); i++) { + final char c = toAppendTo.charAt(i); + switch (c) { + case '\b': + case '\t': + case '\f': + case '\n': + case '\r': + case '"': + case '\\': + escapeCount++; + break; + default: + if (Character.isISOControl(c)) { + escapeCount += 5; + } + } + } + + int lastChar = toAppendTo.length() - 1; + toAppendTo.setLength(toAppendTo.length() + escapeCount); + int lastPos = toAppendTo.length() - 1; + + for (int i = lastChar; lastPos > i; i--) { final char c = toAppendTo.charAt(i); switch (c) { case '\b': - toAppendTo.setCharAt(i, '\\'); - toAppendTo.insert(i + 1, 'b'); + lastPos = escapeAndDecrement(toAppendTo, lastPos, 'b'); break; case '\t': - toAppendTo.setCharAt(i, '\\'); - toAppendTo.insert(i + 1, 't'); + lastPos = escapeAndDecrement(toAppendTo, lastPos, 't'); break; case '\f': - toAppendTo.setCharAt(i, '\\'); - toAppendTo.insert(i + 1, 'f'); + lastPos = escapeAndDecrement(toAppendTo, lastPos, 'f'); break; case '\n': - // Json string newline character must be encoded as literal "\n" - toAppendTo.setCharAt(i, '\\'); - toAppendTo.insert(i + 1, 'n'); + lastPos = escapeAndDecrement(toAppendTo, lastPos, 'n'); break; case '\r': - toAppendTo.setCharAt(i, '\\'); - toAppendTo.insert(i + 1, 'r'); + lastPos = escapeAndDecrement(toAppendTo, lastPos, 'r'); break; case '"': case '\\': - // only " and \ need to be escaped; other escapes are optional - toAppendTo.insert(i, '\\'); + lastPos = escapeAndDecrement(toAppendTo, lastPos, c); break; default: if (Character.isISOControl(c)) { - // all iso control characters are in U+00xx - toAppendTo.setCharAt(i, '\\'); - toAppendTo.insert(i + 1, "u0000"); - toAppendTo.setCharAt(i + 4, Chars.getUpperCaseHex((c & 0xF0) >> 4)); - toAppendTo.setCharAt(i + 5, Chars.getUpperCaseHex(c & 0xF)); + // all iso control characters are in U+00xx, JSON output format is "\\u00XX" + toAppendTo.setCharAt(lastPos--, Chars.getUpperCaseHex(c & 0xF)); + toAppendTo.setCharAt(lastPos--, Chars.getUpperCaseHex((c & 0xF0) >> 4)); + toAppendTo.setCharAt(lastPos--, '0'); + toAppendTo.setCharAt(lastPos--, '0'); + toAppendTo.setCharAt(lastPos--, 'u'); + toAppendTo.setCharAt(lastPos--, '\\'); + } else { + toAppendTo.setCharAt(lastPos, c); + lastPos--; } } } } + private static int escapeAndDecrement(StringBuilder toAppendTo, int lastPos, char c) { + toAppendTo.setCharAt(lastPos--, c); + toAppendTo.setCharAt(lastPos--, '\\'); + return lastPos; + } + public static void escapeXml(final StringBuilder toAppendTo, final int start) { for (int i = toAppendTo.length() - 1; i >= start; i--) { // backwards: length may change final char c = toAppendTo.charAt(i); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/363984f1/log4j-api/src/test/java/org/apache/logging/log4j/util/StringBuildersTest.java ---------------------------------------------------------------------- diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/util/StringBuildersTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/util/StringBuildersTest.java index b52e3f6..7c74073 100644 --- a/log4j-api/src/test/java/org/apache/logging/log4j/util/StringBuildersTest.java +++ b/log4j-api/src/test/java/org/apache/logging/log4j/util/StringBuildersTest.java @@ -47,5 +47,34 @@ public class StringBuildersTest { assertTrue("trimmed OK", sb.capacity() <= Constants.MAX_REUSABLE_MESSAGE_SIZE); } + @Test + public void escapeJsonCharactersCorrectly() { + String jsonValueNotEscaped = "{\"field\n1\":\"value_1\"}"; + String jsonValueEscaped = "{\\\"field\\n1\\\":\\\"value_1\\\"}"; + + StringBuilder sb = new StringBuilder(); + sb.append(jsonValueNotEscaped); + assertEquals(jsonValueNotEscaped, sb.toString()); + StringBuilders.escapeJson(sb, 0); + assertEquals(jsonValueEscaped, sb.toString()); + + sb = new StringBuilder(); + String jsonValuePartiallyEscaped = "{\"field\n1\":\\\"value_1\\\"}"; + sb.append(jsonValueNotEscaped); + assertEquals(jsonValueNotEscaped, sb.toString()); + StringBuilders.escapeJson(sb, 10); + assertEquals(jsonValuePartiallyEscaped, sb.toString()); + } + @Test + public void escapeJsonCharactersISOControl() { + String jsonValueNotEscaped = "{\"field\n1\":\"value" + (char) 0x8F + "_1\"}"; + String jsonValueEscaped = "{\\\"field\\n1\\\":\\\"value\\u008F_1\\\"}"; + + StringBuilder sb = new StringBuilder(); + sb.append(jsonValueNotEscaped); + assertEquals(jsonValueNotEscaped, sb.toString()); + StringBuilders.escapeJson(sb, 0); + assertEquals(jsonValueEscaped, sb.toString()); + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/363984f1/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 3c63e49..670ec21 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -144,6 +144,10 @@ <action issue="LOG4J2-2368" dev="ckozak" type="fix"> Nested logging doesn't clobber AbstractStringLayout cached StringBuidlers </action> + <action issue="LOG4J2-2373" dev="ckozak" type="fix" due-to="Kevin Meurer"> + StringBuilders.escapeJson implementation runs in linear time. Escaping large JSON strings + in EncodingPatternConverter and MapMessage will perform significantly better. + </action> </release> <release version="2.11.0" date="2018-03-11" description="GA Release 2.11.0"> <action issue="LOG4J2-2104" dev="rgoers" type="fix">
