Repository navigation
MINOR: Check source range in ArrowBuf.setBytes for direct ByteBuffers - #1380
Merged
jbonofre merged 1 commit intoNov 6, 2026
Merged
Conversation
Arawoof06
requested review from
jbonofre,
laurentgo,
lidavidm and
wgtmac
as code owners
October 21, 2026 10:35
jbonofre
approved these changes
Oct 21, 2026
jbonofre
left a comment
Member
There was a problem hiding this comment.
LGTM! Thanks for the fix @Arawoof06!
The direct-buffer path of setBytes(long, ByteBuffer, int, int) only checked the destination, so an out-of-range srcIndex/length silently copied adjacent native memory.
Adding the source check up front brings it in line with the byte[] and ArrowBuf overloads, and valid ranges behave as before.
The regression test covers oversize length, oversize range and negative srcIndex.
A couple of non-blocking suggestions:
- Add a test for the heap
ByteBufferpath and a valid boundary copy (e.g.srcIndex=4, length=4on a capacity-8 buffer), so an over-strict check would be caught. - Consider naming the source in the exception message (e.g. "source ByteBuffer index:..., length:... (expected: range(0, capacity))"). As written it looks identical to a destination-bounds failure from
checkIndex.
jbonofre
force-pushed
the
arrowbuf-setbytes-direct-src-check
branch
from
November 5, 2026 19:57
4d14315 to
6553487
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's Changed
ArrowBuf.setBytes(long index, ByteBuffer src, int srcIndex, int length)bounds-checks only the destination. Whensrcis a direct buffer it copieslengthbytes fromgetByteBufferAddress(src) + srcIndexwithMemoryUtil.copyMemory, so a source range pastsrc.capacity()(or a negativesrcIndex) silently copies adjacent native memory into the ArrowBuf, even with bounds checking enabled. For example,buf.setBytes(0, ByteBuffer.allocateDirect(8), 0, 64)succeeds. The heap path already rejects this throughByteBuffer.position/limit, and thebyte[]andArrowBufoverloads (andNettyArrowBuf.setBytes(ByteBuf)) check the source withisOutOfBounds. This adds the same source check before either path, so theset/setSafe(int, ByteBuffer, int, int)methods on the variable-width vectors fail withIndexOutOfBoundsExceptioninstead of reading out of bounds. Valid ranges behave as before. Added a regression test toTestArrowBuf.