Skip to content

fix: reject embedded NUL bytes in documents to prevent FTS5 corruption (#7388) - #7474

Open
rkfshakti wants to merge 1 commit into
chroma-core:mainfrom
rkfshakti:fix/nul-byte-validation-fts5-corruption
Open

fix: reject embedded NUL bytes in documents to prevent FTS5 corruption (#7388)#7474
rkfshakti wants to merge 1 commit into
chroma-core:mainfrom
rkfshakti:fix/nul-byte-validation-fts5-corruption

Conversation

@rkfshakti

Copy link
Copy Markdown

Problem

A document string containing an embedded NUL character (U+0000) passed to Collection.upsert() corrupts the FTS5 inverted index for the entire collection — not just that one document. PRAGMA quick_check on the resulting chroma.sqlite3 reports:

malformed inverted index for FTS5 table main.embedding_fulltext_search

NUL bytes are valid, well-formed Python strings ("a\x00b" is legal UTF-8), so nothing in the Python-level API rejects them before they reach SQLite/FTS5.

Reproduction

import chromadb
client = chromadb.PersistentClient(path="/tmp/chroma_nul_repro")
col = client.get_or_create_collection(name="test_collection")
nul_doc = "before-nuls " + ("\x00" * 10) + " after-nuls"
col.upsert(documents=[nul_doc], ids=["doc-1"])
# Collection's FTS5 index is now corrupted

Fix

Add validation in validate_documents() to reject documents containing NUL bytes before they reach the database, with a clear error message:

if "\x00" in document:
    raise ValueError(
        "Document contains embedded NUL (U+0000) characters, "
        "which corrupt the full-text search index. "
        "Strip or replace NUL bytes before upserting."
    )

This is a strict improvement: malformed documents get a clear error instead of silently corrupting the collection's full-text search index.

Test

Added test_nul_byte_in_document_raises_error — verifies that:

  • Multiple NUL bytes in a document raise ValueError on upsert()
  • A single NUL byte in a document raises ValueError on add()
  • Clean documents still work fine

Fixes #7388

chroma-core#7388)

A document string containing an embedded NUL character (U+0000) passed to
upsert/add corrupts the FTS5 inverted index for the entire collection.
PRAGMA quick_check reports 'malformed inverted index for FTS5 table'.

NUL bytes are valid in Python strings but destructive to SQLite/FTS5.
Add validation in validate_documents() to reject documents containing
NUL bytes before they reach the database, with a clear error message.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@rkfshakti

Copy link
Copy Markdown
Author

Hi maintainers — gentle bump on this fix for #7388. Rejects embedded NUL bytes in documents to prevent FTS5 index corruption for the entire collection. Ready for review. Thanks!

@rkfshakti

Copy link
Copy Markdown
Author

Hi maintainers — circling back on this fix for #7388. Curious if the NUL-byte rejection approach makes sense or if there's a preferred sanitization strategy. Happy to iterate. Keen to contribute! Thanks.

@rkfshakti

Copy link
Copy Markdown
Author

Friendly ping — this PR has been open for a couple of days with CI passing. Would appreciate a review when time allows.

@rkfshakti

Copy link
Copy Markdown
Author

Hi maintainers — following up again on this fix for #7388. Rejects embedded NUL bytes in documents to prevent FTS5 corruption. CI is green. Would appreciate a review when time allows. Thanks!

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]: embedded NUL byte in a document corrupts the FTS5 inverted index for the whole collection

1 participant