Skip to content

fix(manifest): reject struct children longer than their parent length - #1054

Open
gty404 wants to merge 1 commit into
apache:mainfrom
gty404:fix/manifest-children-length
Open

gty404 wants to merge 1 commit into
apache:mainfrom
gty404:fix/manifest-children-length

Conversation

@gty404

@gty404 gty404 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • nanoarrow FULL validation only requires a struct's children to be at least as long as the parent struct, but ParseManifestList, ParseDataFile, and ParseManifestEntry index containers that are pre-sized to the parent's row count using each child's own length. A spec-legal batch with a longer child drives those loops past the end of the output vector.
  • Add AssertChildrenLength, which asserts every child of a struct view has exactly the same length as the parent, and call it everywhere a struct's children feed a parent-sized container.
  • The kPartitionFieldId case in ParseDataFile previously validated the partition struct's type with a bare AssertViewType (no children-count check at all); it now uses AssertViewTypeAndChildren plus the new length check, matching the other call sites.

Test plan

  • Added AssertChildrenLengthTest.RejectsChildLongerThanParent in manifest_reader_test.cc, hand-building an Arrow StructArray whose declared length is shorter than its child's actual length (legal under nanoarrow FULL validation) and asserting InvalidArgument.
  • Confirmed the new test fails without the fix (temporarily neutered AssertChildrenLength to a no-op) and passes with it restored.
  • Ran the full manifest_test suite: 227 passed, 0 failed.

nanoarrow FULL validation only requires a struct's children to be at
least as long as the parent, but ParseManifestList/ParseDataFile/
ParseManifestEntry index containers sized by the parent's row count
using each child's own length. A spec-legal batch with a longer child
would drive those parsers past the end of the output vector.

Add AssertChildrenLength and call it everywhere a struct's children
feed a parent-sized container, including the partition struct case
which previously skipped the children-count check entirely.

This branch has not been deployed

No deployments
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.

1 participant