Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix off-by-one in StackTraceStringResolver #3216

Merged
merged 3 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>