Skip to content

Commit

Permalink
Fix off-by-one in StackTraceStringResolver (#3216)
Browse files Browse the repository at this point in the history
Fixes #3194, ports #3212.
  • Loading branch information
vy authored Nov 18, 2024
1 parent 81e1f06 commit efba2fb
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,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.
Expand Down Expand Up @@ -195,26 +195,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'
Expand All @@ -230,7 +231,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'
Expand All @@ -249,13 +250,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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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="3216" link="https://github.com/apache/logging-log4j2/pull/3216"/>
<description format="asciidoc">Fix `ArrayIndexOutOfBoundsException` in JSON Template Layout truncated exception resolver</description>
</entry>

0 comments on commit efba2fb

Please sign in to comment.