Fix off-by-one in StackTraceStringResolver (#3212)

Author: vy

log4j-layout-template-json-test/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolverTest.java

@@ -16,6 +16,7 @@
  */
 package org.apache.logging.log4j.layout.template.json.resolver;
 
+import static java.util.Collections.singletonList;
 import static org.apache.logging.log4j.layout.template.json.TestHelpers.CONFIGURATION;
 import static org.apache.logging.log4j.layout.template.json.TestHelpers.JAVA_BASE_PREFIX;
 import static org.apache.logging.log4j.layout.template.json.TestHelpers.asMap;
@@ -40,6 +41,7 @@
 import org.apache.logging.log4j.core.impl.Log4jLogEvent;
 import org.apache.logging.log4j.layout.template.json.JsonTemplateLayout;
 import org.apache.logging.log4j.layout.template.json.JsonTemplateLayoutDefaults;
+import org.apache.logging.log4j.layout.template.json.util.TruncatingBufferedPrintWriter;
 import org.apache.logging.log4j.util.Constants;
 import org.assertj.core.api.AbstractStringAssert;
 import org.junit.jupiter.api.Nested;
@@ -593,6 +595,67 @@ private String pointMatcherRegex(final Throwable exception) {
         private String matchingRegex(final String string) {
             return "[" + string.charAt(0) + "]" + Pattern.quote(string.substring(1));
         }
+
+        @Test
+        void should_not_fail_on_truncated_output_not_ending_with_newline() {
+
+            // Try to find an exception whose truncated stack trace does not end with a newline
+            final int maxStringLength = 100;
+            final float maxByteCountPerChar =
+                    JsonTemplateLayoutDefaults.getCharset().newEncoder().maxBytesPerChar();
+            final int maxStringByteCount =
+                    Math.toIntExact(Math.round(Math.ceil(maxByteCountPerChar * maxStringLength)));
+            final TruncatingBufferedPrintWriter writer = TruncatingBufferedPrintWriter.ofCapacity(maxStringByteCount);
+            Exception exception;
+            String message = "m";
+            do {
+                exception = new Exception(message);
+                exception.printStackTrace(writer);
+                if (writer.truncated() && writer.buffer()[writer.length() - 1] != '\n') {
+                    break;
+                }
+                writer.close();
+                message += "m";
+            } while (true);
+
+            // Create the event template
+            final String eventTemplate = writeJson(asMap(
+                    "ex",
+                    asMap(
+                            "$resolver",
+                            "exception",
+                            "field",
+                            "stackTrace",
+                            "stackTrace",
+                            asMap(
+                                    "stringified",
+                                    asMap(
+                                            "truncation",
+                                            asMap(
+                                                    "suffix",
+                                                    TRUNCATION_SUFFIX,
+                                                    "pointMatcherStrings",
+                                                    singletonList("this string shouldn't match with anything")))))));
+
+            // Create the layout
+            final JsonTemplateLayout layout = JsonTemplateLayout.newBuilder()
+                    .setConfiguration(CONFIGURATION)
+                    .setEventTemplate(eventTemplate)
+                    .setMaxStringLength(maxStringLength)
+                    .setStackTraceEnabled(true)
+                    .build();
+
+            // Create the log event
+            final LogEvent logEvent =
+                    Log4jLogEvent.newBuilder().setThrown(exception).build();
+
+            // Check the serialized event
+            usingSerializedLogEventAccessor(layout, logEvent, accessor -> {
+                final int expectedStackTraceLength = maxStringLength + TRUNCATION_SUFFIX.length();
+                final String stackTrace = accessor.getString("ex");
+                assertThat(stackTrace).hasSizeLessThan(expectedStackTraceLength);
+            });
+        }
     }
 
     @Test

log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolver.java

@@ -121,7 +121,7 @@ private void truncate(
         for (; ; ) {
 
             // Find the next label start, if present.
-            final int labeledLineStartIndex = findLabeledLineStartIndex(srcWriter, startIndex, srcWriter.length());
+            final int labeledLineStartIndex = findLabeledLineStartIndex(srcWriter, startIndex);
             final int endIndex = labeledLineStartIndex >= 0 ? labeledLineStartIndex : srcWriter.length();
 
             // Copy up to the truncation point, if it matches.
@@ -197,26 +197,27 @@ private int findTruncationPointIndex(
         return -1;
     }
 
-    private static int findLabeledLineStartIndex(final CharSequence buffer, final int startIndex, final int endIndex) {
+    private static int findLabeledLineStartIndex(final CharSequence buffer, final int startIndex) {
         // Note that the index arithmetic in this method is not guarded.
         // That is, there are no `Math.addExact()` or `Math.subtractExact()` usages.
         // Since we know a priori that we are already operating within buffer limits.
-        for (int bufferIndex = startIndex; bufferIndex < endIndex; ) {
+        final int bufferLength = buffer.length();
+        for (int bufferIndex = startIndex; bufferIndex < bufferLength; ) {
 
             // Find the next line start, if exists.
-            final int lineStartIndex = findLineStartIndex(buffer, bufferIndex, endIndex);
+            final int lineStartIndex = findLineStartIndex(buffer, bufferIndex);
             if (lineStartIndex < 0) {
                 break;
             }
             bufferIndex = lineStartIndex;
 
             // Skip tabs.
-            while (bufferIndex < endIndex && '\t' == buffer.charAt(bufferIndex)) {
+            while (bufferIndex < bufferLength && '\t' == buffer.charAt(bufferIndex)) {
                 bufferIndex++;
             }
 
             // Search for the `Caused by: ` occurrence.
-            if (bufferIndex < (endIndex - 11)
+            if (bufferIndex < (bufferLength - 11)
                     && buffer.charAt(bufferIndex) == 'C'
                     && buffer.charAt(bufferIndex + 1) == 'a'
                     && buffer.charAt(bufferIndex + 2) == 'u'
@@ -232,7 +233,7 @@ private static int findLabeledLineStartIndex(final CharSequence buffer, final in
             }
 
             // Search for the `Suppressed: ` occurrence.
-            else if (bufferIndex < (endIndex - 12)
+            else if (bufferIndex < (bufferLength - 12)
                     && buffer.charAt(bufferIndex) == 'S'
                     && buffer.charAt(bufferIndex + 1) == 'u'
                     && buffer.charAt(bufferIndex + 2) == 'p'
@@ -251,13 +252,11 @@ else if (bufferIndex < (endIndex - 12)
         return -1;
     }
 
-    private static int findLineStartIndex(final CharSequence buffer, final int startIndex, final int endIndex) {
-        char prevChar = '-';
-        for (int i = startIndex; i <= endIndex; i++) {
-            if (prevChar == '\n') {
-                return i;
+    private static int findLineStartIndex(final CharSequence buffer, final int startIndex) {
+        for (int bufferIndex = startIndex; bufferIndex < buffer.length(); bufferIndex++) {
+            if (buffer.charAt(bufferIndex) == '\n' && (bufferIndex + 1) < buffer.length()) {
+                return bufferIndex + 1;
             }
-            prevChar = buffer.charAt(i);
         }
         return -1;
     }

src/changelog/.2.x.x/3212_fix_JTL_StackTraceStringResolver.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+  <issue id="3212" link="https://github.com/apache/logging-log4j2/pull/3212"/>
+  <description format="asciidoc">Fix `ArrayIndexOutOfBoundsException` in JSON Template Layout truncated exception resolver</description>
+</entry>