Escape tabs and newlines in keys for the WAL/SSTable line format#114
Escape tabs and newlines in keys for the WAL/SSTable line format#114mweiden wants to merge 1 commit into
Conversation
Records are stored as 'key\tbase64(value)\n'. Values are base64-encoded and safe, but keys were written raw, and keys are built from user-supplied partition/clustering values. A value like 'a\tb' or a string containing a newline split the record at the wrong byte on WAL replay and SSTable scans, silently corrupting that record and shifting parsing for the rest of the file. Keys are now backslash-escaped on disk (\t, \n, \\) and unescaped on read. SSTables sort, index, and probe by the escaped form so binary search and early-exit scans stay consistent; bloom filters and zone maps keep operating on raw keys. Keys without special characters are byte-identical to the previous format, so existing data files remain readable. https://claude.ai/code/session_01CJXWjB9ERGgV6B9mRqrdM9
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a889bb1d48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| // The file and index store escaped keys, so probe with the escaped | ||
| // form. | ||
| let escaped = crate::util::escape_key(key); |
There was a problem hiding this comment.
When opening SSTables written before this commit, the on-disk key bytes and persisted sparse index are still raw. Escaping every lookup key here makes any pre-upgrade row whose key contains a backslash, such as user\1 or path\to, get probed as user\\1; the bloom/zone-map checks can pass, but seek_offset/scan_from compare against the raw legacy bytes and return None, making those rows unreadable after upgrade. Please detect/migrate legacy SSTables or fall back to raw-key probing when the escaped probe misses.
Useful? React with 👍 / 👎.
| .map_err(std::io::Error::other)? | ||
| .to_string(); | ||
| .map_err(std::io::Error::other)?; | ||
| let key = crate::util::unescape_key(key).into_owned(); |
There was a problem hiding this comment.
This unconditionally decodes keys from any existing WAL, but WAL records produced before this change stored backslashes literally. If an upgrade happens with an unflushed record whose key is a\t, a\n, or a\\b, replay inserts it under a different key (a<TAB>, a<NEWLINE>, or a\b), so reads by the original key fail or collide with another row. The WAL needs a version/marker or a legacy fallback before applying escape decoding to old records.
Useful? React with 👍 / 👎.
2 participants
Problem
WAL and SSTable records are stored as
key\tbase64(value)\n. Values are base64-encoded and safe, but keys were written raw — and keys are built from user-supplied partition/clustering values. Inserting a value like'a\tb'(or anything containing a newline) split the record at the wrong byte on WAL replay and on every SSTable scan, silently corrupting that record and the parse of everything after it.Fix
util::escape_key/util::unescape_key(backslash escaping:\t,\n,\\),Cow-based so plain keys don't allocate.lib.rs::insert_internal) and unescape on replay (wal.rs::parse_entries).scan_nsunescapes parsed keys.Compatibility: keys without special characters are byte-identical to the old format, and unknown escape sequences are preserved verbatim on decode, so existing data files remain readable.
Tests
wal_recovery_preserves_keys_with_delimiters: hostile keys (tab/newline/backslash mixes) survive restart, neighbouring records intact, no phantom rows.sstable_roundtrip_preserves_keys_with_delimiters: lookups work after create and after a meta-less rebuild.flush_and_query_preserve_partition_keys_with_delimiters: end-to-end through the SQL engine with flush.Found by codebase audit (high severity: silent data corruption from ordinary inputs).
https://claude.ai/code/session_01CJXWjB9ERGgV6B9mRqrdM9
Generated by Claude Code