MINOR: Bounds check source buffer in BaseFixedWidthVector.copyFrom

[Support-itecz]

What's Changed

BaseFixedWidthVector.copyFrom copies the value with a raw MemoryUtil.copyMemory from from.getDataBuffer().memoryAddress() + fromIndex * typeWidth, so neither the source nor the target buffer is bounds checked, even with bounds checking enabled. A fixed-width vector loaded from an IPC record batch whose field node length is larger than its data buffer (for example 4 int values over a 4-byte buffer) passes isNull. copyFrom / copyFromSafe / TransferPair.copyValueSafe on an index past the buffer then read native memory beyond it, while get() on the same index throws IndexOutOfBoundsException. In the new test the copied value is an int sitting 8 bytes past the end of the data slice.

The copy now goes through ArrowBuf.getBytes(index, dst, dstIndex, length), which checks both buffers, the same way BaseVariableWidthVector.copyFrom already does. Copies of valid data are unchanged. TestCopyFrom.testCopyFromFixedWidthSourceBufferTooShort fails without the change and passes with it, and the arrow-vector and arrow-algorithm test suites pass locally along with spotless and checkstyle.

[jbonofre]

This hardens only BaseFixedWidthVector.copyFrom. The same unchecked pattern, MemoryUtil.copyMemory on raw memoryAddress() plus an offset, remains in other places in vector/src/main. These include DecimalVector, Decimal256Vector, BitVectorHelper, and VectorAppender`.

If the goal is to make malformed IPC input fail with IndexOutOfBoundsException instead of an out-of-bounds read, can we say which of these are in scope?

If they are reachable from loadFieldBuffers style input, I would fix them here.

[jbonofre]

Oh, also, copyFrom is called once per element, and copyFromSafe loops and appenders call it in bulk. ArrowBuf.getBytes(long, ArrowBuf, long, int) adds bounds checks on both buffers plus argument validation, in place of a single raw copy.

Do you have a before/after number, for instance the existing JMH benchmarks for copyFrom? I would like to see the cost here.

Also, this only protects when bounds checking is on. With arrow.enable_unsafe_memory_access=true, getBytes skips validation and the read is unchecked again. That is probably acceptable, since the flag disables checks by design, but please say so in the description.

[jbonofre]

The source is loaded with a zero-length validity buffer (body.slice(0, 0)) and a null count of 0. The first copyFromSafe(0, 0, source) then needs isSet(0) to read that buffer without throwing.

Is that guaranteed, or does it work only because of how isSet handles this case today?
If a later change makes isSet check the buffer length, this test fails for a reason unrelated to the data-buffer bounds check it's meant to cover.

Giving the source a real validity buffer (all bits set) would make the test say what it means.

[github-actions]

Thank you for opening a pull request!

Please label the PR with one or more of:

• bug-fix
• chore
• dependencies
• documentation
• enhancement

Also, add the 'breaking-change' label if appropriate.

See CONTRIBUTING.md for details.