GH-3716: Fix data corruption in ByteBufferBackedBinary.getBytes() for non-array-backed buffers

[yimingli-vmware]

Rationale for this change

Fixes #3716.

FixedLenByteArrayPlainValuesReader hands out Binary values that all
share one page-wide ByteBuffer, advancing its live position on every
readBytes() call. Binary.ByteBufferBackedBinary.getBytes() and
toStringUsingUTF8() (non-array-backed branch) called
value.limit(offset + length) directly on that same shared buffer before
capturing value.position(). Since ByteBuffer.limit() clamps position
down whenever position > newLimit, calling getBytes() on an earlier
value after later values have already advanced the buffer permanently
rewinds the buffer's live position -- corrupting every readBytes() call
that follows.

This surfaces as data corruption when reading a repeated (LIST)
FIXED_LEN_BYTE_ARRAY column with 2+ elements per row across 2+ rows:
record assembly stores each Binary and only materializes it once a full
row/group has been built, which is exactly the lazy-after-later-value
pattern that triggers the clamp. Each subsequent row reads back the
previous row's last-written element instead of its own (see #3716 for a
minimal standalone repro).

What changes are included in this PR?

• Binary.ByteBufferBackedBinary.getBytes() and .toStringUsingUTF8() now
  duplicate() the buffer before adjusting position/limit, so the shared
  buffer's own position is never mutated.
• Added regression tests to
  TestFixedLenByteArrayPlainValuesWriterReader that read values out of
  the order they were materialized, matching the lazy-consumption pattern
  from record assembly, and fail against unpatched 1.18.0.

Are these changes tested?

Yes -- two new tests
(testLazyGetBytesDoesNotCorruptSubsequentReadsDirectBuffer,
testLazyToStringUsingUTF8DoesNotCorruptSubsequentReadsDirectBuffer) in
parquet-column/src/test/java/org/apache/parquet/column/values/plain/TestFixedLenByteArrayPlainValuesWriterReader.java
fail on the unpatched code and pass with this fix. Also verified against
the full-file-roundtrip repro from #3716.

Are there any user-facing changes?

No API changes. This fixes a silent data-corruption bug introduced in
1.18.0; no user-facing behavior changes other than correct results.

[dossett]

Nice catch. +1 (non-binding). I confirmed the new test code fails under 1.18.0

[yimingli-vmware]

Nice catch. +1 (non-binding). I confirmed the new test code fails under 1.18.0

Hi @dossett, thanks for the confirmation. I wish we could have a patch release for addressing this issue soon, since we are blocked by the Jackson CVEs which are resolved in 1.18.0.

[jerolba]

Out of curiosity, was this issue latent in the codebase, or was it introduced around the v1.18.0 changes?"

[dossett]

Based on my analysis (with Codex) it was introduced in 1.18.0, specifically in #3565.

I confirmed that 1.18.0 with that PR reverted will pass the new tests.

[jerolba]

@dossett Got it, thanks! So the bug in ByteBufferBackedBinary was exposed by the updates to FixedLenByteArrayPlainValuesReader, right?

[dossett]

That's almost a philosophical question about what constitutes a bug :-) But there's definitely some truth to that.

[Fokko]

Thanks for the quick follow up @yimingli-vmware 🙌

[yimingli-vmware]

Hi @Fokko and @wgtmac, my PR got a failed workflow run since this Fix Maven parent and JUnit test compatibility merged into Master but my PR was in the stale state. I've already rebased my PR off Mater, could @wgtmac help me re-trigger my workflow to make sure this PR does not break anything?

And @dossett created another PR which solves a similar issue, so may be that PR also needs to be added in the 1.18.1 milestone?

Since i've got 2 approvals and it's my first PR to this project, should i merge this PR, or committers/members will merge this PR? Thanks!