Skip to content

Fix int overflow and ignored offset/count in UnsafeOutput and UnsafeByteBufferOutput#1270

Open
JamesLove03 wants to merge 1 commit intoEsotericSoftware:masterfrom
JamesLove03:fix/unsafe-output-int-overflow
Open

Fix int overflow and ignored offset/count in UnsafeOutput and UnsafeByteBufferOutput#1270
JamesLove03 wants to merge 1 commit intoEsotericSoftware:masterfrom
JamesLove03:fix/unsafe-output-int-overflow

Conversation

@JamesLove03
Copy link
Copy Markdown

Summary

Both UnsafeOutput and UnsafeByteBufferOutput contain two bugs in their
six primitive-array bulk write methods.

Bug 1 — Integer overflow → JVM crash

Byte count was computed by left-shifting array.length as a 32-bit int:

writeBytes(array, longArrayBaseOffset, array.length << 3);

When array.length exceeds the type's overflow threshold the shift wraps to a
negative int. writeBytes widens it to a large positive long and passes it
to unsafe.copyMemory(), causing a fatal JVM crash.

This is reachable from DefaultArraySerializers without any custom code.

Bug 2 — offset and count parameters silently ignored

All six methods accepted offset and count but always serialized the full
array from element 0, this behavior is altered from these method's safe counterparts.

Fix

  • Promote the shift operand to long before the shift so the byte count is
    computed correctly for large arrays.
  • Change writeBytes(Object, long, int count) to accept long count so the
    corrected value is not truncated on the way in.
  • Pass offset and count through to writeBytes in all six methods.

Applied identically to both UnsafeOutput and UnsafeByteBufferOutput.
The unsafe.copyMemory() call that provides the performance benefit is
untouched; the fix has no meaningful impact on throughput.

@theigl
Copy link
Copy Markdown
Collaborator

theigl commented May 7, 2026

Thanks for the PR @JamesLove03. Please provide test-cases that demonstrate these fixes. I will not merge a PR without corresponding regression tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants