Stacktraces larger than maxStringLength are not truncated

[waldeinburg]

Stacktraces larger than resolverContext.writerCapacity causes an ArrayIndexOutOfBoundsException in the call to System::arraycopy from BufferedWriter::write because it attempts to write past the end of the buffer. The message is lost.
For stacktraces, maxStringLength has different semantics than for normal values: Instead of causing truncation, the buffer capacity is set from maxStringLength instead of maxByteCount if set.

The minimal solution would be that BufferedWriter detects if it is going to write past the buffer size and truncates.
The ideal solution would be to truncate as described based on maxStringLength if set and also use a growable buffer to avoid message loss. You could use the same mechanism as ByteArrayOutputStream (c.f. the private method grow) instead of char[].

[vy]

Thanks for the detailed bug report and solution path, @waldeinburg. I am able to reproduce the bug. I will release a fix in a couple of days.

[vy]

Using a dynamically growing buffer is not option, since 1) it will violate garbage-free semantics of the layout and 2) will cause the retain of the overflown buffers since they are recycled. That is why I have just capped the BufferedWriter in 521a463.

Note that the resolverContext#writerCapacity is determined as follows in LogstashLayout.java:

int writerCapacity = builder.maxStringLength > 0
       ? builder.maxStringLength
       : builder.maxByteCount;

There I bet that user is smart enough the provide a decent configuration. That said as I stated in #56, in the successor that will be contributed to log4j-core, these limits will all be defined on char count and there we can validate the decency of the input user configuration.

Would you mind checking the fix, i.e., 521a463, and letting me know if it addresses your issue, please?

[waldeinburg]

The fix works, thanks!
I.e., I get the following results as expected:
If maxStringLength is set, the stack trace is truncated.
If maxStringLength is not set a BufferOverflowException is thrown later, because the buffer size for the stack trace is the same as the buffer size for the whole JSON object and that space is filled by the (truncated) stack trace.

I understand now why we can't have a growing buffer, but I think the default value for maxStringLength should be reconsidered. Stack trace is the field most likely to fill up the buffer, so if the default was, e.g., 12288 (i.e., 3/4 of the buffer) you would keep the message under normal conditions. Sure, the user is smart ... but (IMHO) is also inclined to think that the defaults are safe and will probably not look out for buffer sizes when the logging does not include large fields apart from stack traces.

[vy]

Thanks for the prompt feedback @waldeinburg. I am closing the issue. Once you comment on the 2 other issues you have submitted (#55 and #56), I will make a new release for you.

[waldeinburg]

You're welcome! Thanks for the super quick feedback to you too!

[vy]

I've just released v1.0.2 including this fix.

[waldeinburg]

Thanks! I would wish all libraries replied this fast to issues!