Skip to content

[common] Fix ArrayIndexOutOfBoundsException in .toArray() methods#2158

Merged
polyzos merged 7 commits into
apache:mainfrom
binary-signal:fix-array-index-out-of-bounds
Apr 27, 2026
Merged

[common] Fix ArrayIndexOutOfBoundsException in .toArray() methods#2158
polyzos merged 7 commits into
apache:mainfrom
binary-signal:fix-array-index-out-of-bounds

Conversation

@binary-signal
Copy link
Copy Markdown
Contributor

@binary-signal binary-signal commented Dec 12, 2025

Linked issue: close #2157

Purpose

Fix incorrect targetOffset usage in BinaryArray’s to*Array() methods.

Previously, these methods passed UNSAFE.arrayBaseOffset(...) constants (e.g., DOUBLE_ARRAY_OFFSET) to BinarySegmentUtils.copyToUnsafe(). These values represent raw memory offsets (typically ~16 bytes on 64-bit JVMs), while copyToUnsafe() expects a standard 0-based Java array index.

This mismatch caused writes to start at an incorrect position in the target array, leading to:

  • ArrayIndexOutOfBoundsException
  • Corrupted results during round-trip conversions

For example, with a 20-element double array:

  • targetOffset ≈ 16
  • Writes occur from target[16] to target[35]
  • Resulting in: `ArrayIndexOutOfBoundsException: Index 20 out of bounds for length 20

Brief change log

  • BinaryArray.java

    • Replaced all UNSAFE.arrayBaseOffset(...) constants with 0 for the targetOffset parameter in:
      • toBooleanArray
      • toByteArray
      • toShortArray
      • toIntArray
      • toLongArray
      • toFloatArray
      • toDoubleArray
  • BinaryArrayTest.java

    • Added round-trip tests for all primitive array types to ensure correctness

Tests

Added the following round-trip tests:

  • testRoundTripToBooleanArray
  • testRoundTripToByteArray
  • testRoundTripToShortArray
  • testRoundTripToIntArray
  • testRoundTripToLongArray
  • testRoundTripToFloatArray
  • testRoundTripToDoubleArray

API & Compatibility

  • No API changes
  • No storage format changes

@binary-signal
Copy link
Copy Markdown
Contributor Author

@wuchong could you please have a look when you have time

@binary-signal binary-signal force-pushed the fix-array-index-out-of-bounds branch from b1c70cc to 73d8682 Compare April 7, 2026 10:41
@binary-signal
Copy link
Copy Markdown
Contributor Author

@fresh-borzoni @luoyuxia PTAL 🙏

Copy link
Copy Markdown
Member

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@binary-signal TY, LGTM 👍 verified the fix locally

nit: worth adding an empty-array test (e.g. new double[0]) for at least one primitive.

@binary-signal
Copy link
Copy Markdown
Contributor Author

Thanks for the review @fresh-borzoni, I will also add the empty array test

@binary-signal
Copy link
Copy Markdown
Contributor Author

addressed comment

@polyzos
Copy link
Copy Markdown
Contributor

polyzos commented Apr 27, 2026

LGTM

@polyzos polyzos merged commit b3c0bad into apache:main Apr 27, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] ArrayIndexOutOfBoundsException when calling toDoubleArray() with arrays larger than a few elements

3 participants