You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2021/01/14 16:29:55 UTC

[logging-log4j2] branch LOG4J2-2998 updated (cef32ec -> a6e6fd7)

This is an automated email from the ASF dual-hosted git repository.

vy pushed a change to branch LOG4J2-2998
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.


 discard cef32ec  LOG4J2-2998 Fix truncation of excessive strings ending with a high surrogate in JsonWriter.
     new a6e6fd7  LOG4J2-2998 Fix truncation of excessive strings ending with a high surrogate in JsonWriter.

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (cef32ec)
            \
             N -- N -- N   refs/heads/LOG4J2-2998 (a6e6fd7)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/changes/changes.xml | 3 +++
 1 file changed, 3 insertions(+)


[logging-log4j2] 01/01: LOG4J2-2998 Fix truncation of excessive strings ending with a high surrogate in JsonWriter.

Posted by vy...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vy pushed a commit to branch LOG4J2-2998
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit a6e6fd7a60039d9ed287b3182dbc28c8076bacd2
Author: Volkan Yazici <vo...@gmail.com>
AuthorDate: Thu Jan 14 17:28:45 2021 +0100

    LOG4J2-2998 Fix truncation of excessive strings ending with a high surrogate in JsonWriter.
---
 .../layout/template/json/util/JsonWriter.java      |  12 +-
 .../layout/template/json/util/JsonWriterTest.java  | 142 +++++++++++++++++++--
 src/changes/changes.xml                            |   3 +
 3 files changed, 142 insertions(+), 15 deletions(-)

diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
index ac35468..835abcc 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
@@ -581,7 +581,11 @@ public final class JsonWriter implements AutoCloseable, Cloneable {
             final CharSequence seq,
             final int offset,
             final int length) {
-        final int limit = offset + length;
+        final int surrogateCorrection =
+                Character.isHighSurrogate(seq.charAt(offset + length - 1))
+                        ? -1
+                        : 0;
+        final int limit = offset + length + surrogateCorrection;
         int i = offset;
         outer:
         while (i < limit) {
@@ -654,7 +658,11 @@ public final class JsonWriter implements AutoCloseable, Cloneable {
             final char[] buffer,
             final int offset,
             final int length) {
-        final int limit = offset + length;
+        final int surrogateCorrection =
+                Character.isHighSurrogate(buffer[offset + length - 1])
+                        ? -1
+                        : 0;
+        final int limit = offset + length + surrogateCorrection;
         int i = offset;
         outer:
         while (i < limit) {
diff --git a/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/util/JsonWriterTest.java b/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/util/JsonWriterTest.java
index b18368e..c7c9c64 100644
--- a/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/util/JsonWriterTest.java
+++ b/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/util/JsonWriterTest.java
@@ -49,6 +49,18 @@ class JsonWriterTest {
             .setTruncatedStringSuffix("~")
             .build();
 
+    private static final int SURROGATE_CODE_POINT = 65536;
+
+    private static final char[] SURROGATE_PAIR = new char[2];
+    static {
+        // noinspection ResultOfMethodCallIgnored
+        Character.toChars(SURROGATE_CODE_POINT, SURROGATE_PAIR, 0);
+    }
+
+    private static final char HI_SURROGATE = SURROGATE_PAIR[0];
+
+    private static final char LO_SURROGATE = SURROGATE_PAIR[1];
+
     private static synchronized <V> V withLockedWriterReturning(
             final Function<JsonWriter, V> consumer) {
         synchronized (WRITER) {
@@ -64,6 +76,23 @@ class JsonWriterTest {
     }
 
     @Test
+    void test_surrogate_code_point() {
+        Assertions
+                .assertThat(HI_SURROGATE)
+                .matches(Character::isHighSurrogate, "is high surrogate");
+        Assertions
+                .assertThat(LO_SURROGATE)
+                .matches(Character::isLowSurrogate, "is low surrogate");
+        Assertions
+                .assertThat(Character.isSurrogatePair(HI_SURROGATE, LO_SURROGATE))
+                .as("is surrogate pair")
+                .isTrue();
+        Assertions
+                .assertThat(SURROGATE_CODE_POINT)
+                .matches(Character::isDefined, "is defined");
+    }
+
+    @Test
     void test_writeValue_null_Object() {
         expectNull(writer -> writer.writeValue(null));
     }
@@ -402,6 +431,27 @@ class JsonWriterTest {
     }
 
     @Test
+    void test_writeString_emitter_excessive_string_ending_with_high_surrogate() {
+        withLockedWriter(writer -> {
+            final int maxStringLength = writer.getMaxStringLength();
+            @SuppressWarnings("StringBufferReplaceableByString")
+            final String excessiveString = new StringBuilder()
+                    .append(Strings.repeat("x", maxStringLength - 1))
+                    .append(HI_SURROGATE)
+                    .append(LO_SURROGATE)
+                    .toString();
+            final String expectedJson = "\"" +
+                    Strings.repeat("x", maxStringLength - 1) +
+                    writer.getTruncatedStringSuffix() +
+                    '"';
+            final BiConsumer<StringBuilder, String> emitter = StringBuilder::append;
+            final String actualJson =
+                    writer.use(() -> writer.writeString(emitter, excessiveString));
+            Assertions.assertThat(actualJson).isEqualTo(expectedJson);
+        });
+    }
+
+    @Test
     void test_writeString_null_formattable() {
         expectNull(writer -> writer.writeString((StringBuilderFormattable) null));
     }
@@ -433,6 +483,27 @@ class JsonWriterTest {
     }
 
     @Test
+    void test_writeString_formattable_excessive_string_ending_with_high_surrogate() {
+        withLockedWriter(writer -> {
+            final int maxStringLength = writer.getMaxStringLength();
+            @SuppressWarnings("StringBufferReplaceableByString")
+            final String excessiveString = new StringBuilder()
+                    .append(Strings.repeat("x", maxStringLength - 1))
+                    .append(HI_SURROGATE)
+                    .append(LO_SURROGATE)
+                    .toString();
+            final String expectedJson = "\"" +
+                    Strings.repeat("x", maxStringLength - 1) +
+                    writer.getTruncatedStringSuffix() +
+                    '"';
+            final String actualJson = writer.use(() ->
+                    writer.writeString(stringBuilder ->
+                            stringBuilder.append(excessiveString)));
+            Assertions.assertThat(actualJson).isEqualTo(expectedJson);
+        });
+    }
+
+    @Test
     void test_writeString_null_seq_1() {
         expectNull(writer -> writer.writeString((CharSequence) null));
     }
@@ -474,9 +545,31 @@ class JsonWriterTest {
     }
 
     @Test
+    void test_writeString_excessive_seq_ending_with_high_surrogate() {
+        withLockedWriter(writer -> {
+            final int maxStringLength = writer.getMaxStringLength();
+            @SuppressWarnings("StringBufferReplaceableByString")
+            final CharSequence seq = new StringBuilder()
+                    .append(Strings.repeat("x", maxStringLength - 1))
+                    .append(HI_SURROGATE)
+                    .append(LO_SURROGATE)
+                    .toString();
+            final String expectedJson = "\"" +
+                    Strings.repeat("x", maxStringLength - 1) +
+                    writer.getTruncatedStringSuffix() +
+                    '"';
+            final String actualJson = writer.use(() -> writer.writeString(seq));
+            Assertions.assertThat(actualJson).isEqualTo(expectedJson);
+        });
+    }
+
+    @Test
     void test_writeString_seq() throws IOException {
-        testQuoting((final Character c) -> {
-            final String s = "" + c;
+        final char[] surrogates = new char[2];
+        testQuoting((final Integer codePoint) -> {
+            // noinspection ResultOfMethodCallIgnored
+            Character.toChars(codePoint, surrogates, 0);
+            final String s = new String(surrogates);
             return withLockedWriterReturning(writer ->
                     writer.use(() -> writer.writeString(s)));
         });
@@ -526,31 +619,54 @@ class JsonWriterTest {
     }
 
     @Test
+    void test_writerString_excessive_buffer_ending_with_high_surrogate() {
+        withLockedWriter(writer -> {
+            final int maxStringLength = writer.getMaxStringLength();
+            @SuppressWarnings("StringBufferReplaceableByString")
+            final char[] buffer = new StringBuilder()
+                    .append(Strings.repeat("x", maxStringLength - 1))
+                    .append(HI_SURROGATE)
+                    .append(LO_SURROGATE)
+                    .toString()
+                    .toCharArray();
+            final String expectedJson = "\"" +
+                    Strings.repeat("x", maxStringLength - 1) +
+                    writer.getTruncatedStringSuffix() +
+                    '"';
+            final String actualJson = writer.use(() -> writer.writeString(buffer));
+            Assertions.assertThat(actualJson).isEqualTo(expectedJson);
+        });
+    }
+
+    @Test
     void test_writeString_buffer() throws IOException {
-        final char[] buffer = new char[1];
-        testQuoting((final Character c) -> {
-            buffer[0] = c;
+        final char[] buffer = new char[2];
+        testQuoting((final Integer codePoint) -> {
+            // noinspection ResultOfMethodCallIgnored
+            Character.toChars(codePoint, buffer, 0);
             return withLockedWriterReturning(writer ->
                     writer.use(() -> writer.writeString(buffer)));
         });
     }
 
     private static void testQuoting(
-            final Function<Character, String> quoter) throws IOException {
+            final Function<Integer, String> quoter) throws IOException {
         final SoftAssertions assertions = new SoftAssertions();
-        for (char c = Character.MIN_VALUE;; c++) {
-            final String s = "" + c;
+        final char[] surrogates = new char[2];
+        for (int codePoint = Character.MIN_CODE_POINT;
+             codePoint <= Character.MAX_CODE_POINT;
+             codePoint++) {
+            // noinspection ResultOfMethodCallIgnored
+            Character.toChars(codePoint, surrogates, 0);
+            final String s = new String(surrogates);
             final String expectedJson = JacksonFixture
                     .getObjectMapper()
                     .writeValueAsString(s);
-            final String actualJson = quoter.apply(c);
+            final String actualJson = quoter.apply(codePoint);
             assertions
                     .assertThat(actualJson)
-                    .as("c='%c' (%d)", c, (int) c)
+                    .as("codePoint='%s' (%d)", s, codePoint)
                     .isEqualTo(expectedJson);
-            if (c == Character.MAX_VALUE) {
-                break;
-            }
         }
         assertions.assertAll();
     }
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 566bd4f..7ceecb4 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -31,6 +31,9 @@
     -->
     <release version="2.14.1" date="2021-MM-DD" description="GA Release 2.14.1">
       <action issue="LOG4J2-2972" dev="vy" type="fix">
+        Fix truncation of excessive strings ending with a high surrogate in JsonWriter.
+      </action>
+      <action issue="LOG4J2-2972" dev="vy" type="fix">
         Refactor AsyncAppender and AppenderControl for handling of Throwables.
       </action>
       <action issue="LOG4J2-2985" dev="vy" type="fix">