Skip to content

fix(ipc): Don't truncate metadata keys and values at NUL bytes - #971

Open
rustyconover wants to merge 1 commit into
apache:mainfrom
rustyconover:fix-ipc-metadata-nul
Open

rustyconover wants to merge 1 commit into
apache:mainfrom
rustyconover:fix-ipc-metadata-nul

Conversation

@rustyconover

Copy link
Copy Markdown
Contributor

Closes #970.

ArrowIpcEncodeMetadata() wrote KeyValue keys and values with flatcc's _create_strn(). That function stops at the first NUL byte (flatcc_builder_create_string(B, s, strnlen(s, max_len))), so a metadata key or value containing a NUL was silently truncated in Schema, Field and Message custom_metadata. NUL (U+0000) is valid UTF-8, and both nanoarrow's metadata format and flatbuffers strings store a length with each string, so the encoder now uses the length-exact _create() to write exactly size_bytes bytes. The decoder already reads keys and values by length. No other IPC encoder code uses _create_strn().

The new NanoarrowIpcEncoderMetadataWithNulBytes test round-trips schema, field and message metadata whose keys and values contain NUL bytes, through the encoder/decoder and through ArrowIpcWriter/ArrowIpcArrayStreamReader, and checks that the bytes match exactly. It fails without the fix: ("field\0key", "\0field\0value") decoded as ("field", ""). ctest passes with and without Arrow C++, also under ASan/UBSan, and pre-commit passes on the changed files.

This PR's code and description were written by AI (Claude Code) and reviewed by a human maintainer.

🤖 Generated with Claude Code

ArrowIpcEncodeMetadata() wrote KeyValue keys and values with flatcc's
_create_strn(), which stops at the first NUL byte (it uses strnlen()).
Metadata keys and values are length-prefixed in both ArrowSchema and the
flatbuffer, and NUL is valid UTF-8, so a key or value containing a NUL
byte was silently truncated in Schema, Field and Message custom_metadata.
Use the length-exact _create() so exactly size_bytes bytes are written.
The decoder already read them length-exactly.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.24%. Comparing base (e5f3d0a) to head (d83a538).

Files with missing lines Patch % Lines
src/nanoarrow/ipc/encoder.c 0.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #971   +/-   ##
=======================================
  Coverage   78.24%   78.24%           
=======================================
  Files         106      106           
  Lines       16901    16901           
  Branches     2012     2012           
=======================================
  Hits        13224    13224           
  Misses       2435     2435           
  Partials     1242     1242           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

IPC encoder truncates metadata keys and values at NUL bytes

2 participants