Skip to content

Store U+0000 in strings on PostgreSQL and MongoDB - #368

Open
LeeroyHannigan wants to merge 1 commit into
mainfrom
fix/nul-in-strings
Open

LeeroyHannigan wants to merge 1 commit into
mainfrom
fix/nul-in-strings

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

Closes #364.

What changes

DynamoDB accepts the character U+0000 anywhere a string appears. Measured against the service on 2026-09-18 with a scratch table: PutItem accepts it inside a partition key, a sort key, a GSI key, a non-key value, a string in a list, a map key, and a top-level attribute name; GetItem returns each byte-identical; it sorts as the byte 0x00 ahead of every other character; it is a different key from the six-character text \u0000; range and prefix conditions treat it as an ordinary byte; the GSI holds the item; DeleteItem on the key removes it. The service also accepts it in key schema attribute names, and rejects it in table and index names by the usual pattern.

ExtendDB's PostgreSQL backend returned 500 InternalServerError for every one of the item cases (TEXT rejects the byte in key columns; jsonb rejects the \u0000 escape in item_data), and for a Query whose key value contains it. The MongoDB backend returned 500 when the character appeared in an attribute name or map key (BSON field names are C strings). SQLite already matched the service.

Both backends now store the character and return it unchanged.

Shared escape

crates/storage/src/util/control_chars.rs adds an order-preserving escape: U+0000 is stored as U+0001 U+0001, U+0001 as U+0001 U+0002, every other character unchanged. Strings without either character are stored exactly as before, so ordinary data does not change on disk. Unit tests enumerate every string up to length 4 over the alphabet {U+0000, U+0001, U+0002, a} and check that the escape round-trips, preserves UTF-8 byte order, preserves prefixes, and never collides, so COLLATE "C" comparisons, BETWEEN, begins_with, and the row-comparison page cursors from #361 and #362 give the same answers on escaped columns as on the raw text.

PostgreSQL

  • Item documents: item_to_json and json_to_item in data/mod.rs apply and reverse the escape on every string in the tree (names, map keys, values). Every item_data write in the crate goes through it (put, update, transactions, index rows, vector index rows), as do stream records, queued index updates and their contexts in gsi_pending, and the vector search reader.
  • Key columns: data/key_text.rs wraps the shared pk_to_text, composite_pk_to_text, and parse_sk so string key components are escaped before binding; every key-producing site in the crate imports the wrappers, so writes, reads, index maintenance, cursors, and the propagation queue agree on the stored form. Composite partition keys escape each part before netstring joining, matching what the Query builder does with a key condition.
  • SQL that addresses an attribute by name inside item_data uses the escaped name: the TTL index and sweep, and vector search filters (name and value).
  • Migration 004_escape_control_chars: rows written before this change hold their strings raw, and a raw U+0001 followed by U+0001 or U+0002 would read back differently under the new decoder (a lone U+0001 reads back unchanged). The operator-run migration rewrites rows containing U+0001 in every data, index, and vector table (text key columns and item_data), gsi_pending, stream_records, and backup_items, one table per transaction with a per-table progress marker so an interrupted run resumes without rewriting a table twice. Only rows containing U+0001 are touched. It scans each table once, so it takes time proportional to data size. Legacy rows never contain U+0000, since PostgreSQL refused them. The migration cannot tell a row this build wrote from a legacy row (an escaped U+0001 is stored as U+0001 U+0002, which is also a legal legacy sequence), so the server refuses to start on a data database that does not record the migration, with a message naming it and the command to run; extenddb init records it on a fresh deployment. That ordering is what makes the migration safe: it only ever sees rows written by older builds. The candidate predicate spells its backslash as chr(92) so it reads the same under either value of standard_conforming_strings. extenddb migrate --yes refuses to run the migration while any other session is connected to the data database and lists them (a server still writing during the rewrite would leave rows in the wrong encoding); its own connections carry application_name = extenddb-migrate and are excluded, and EXTENDDB_MIGRATE_IGNORE_CONNECTIONS=1 skips the check for idle pooler connections. The upgrade manual documents the procedure, the downgrade hazard (an earlier release started against a migrated database reads escaped rows as literal data), and the MongoDB legacy field-name limitation.

MongoDB

BSON string values are length-prefixed and already held the character; field names could not. item_to_document and its inverse escape object keys only. Conditions and native updates whose attribute names contain U+0000 or U+0001 fall back to the existing in-process paths, the same way names containing . or starting with $ already do. Field paths with no fallback (the TTL index, sweep, and field reads) use the escaped name.

Documentation

docs/design/04-component-storage.md describes the escape beside key and item storage; docs/manuals/07-upgrade-manual.md documents the migration.

Tests

  • tests/test_nul_strings.py: 14 wire tests encoding the measurements above (round trip of an item with the character in every position, partition byte order with U+0000 first, U+0000 distinct from its escape text, distinct partition keys, range and prefix conditions, Limit 1 cursor walk, GSI query on a U+0000 key, filter and condition expressions on a U+0000 attribute name, UpdateItem SET and REMOVE with U+0000 names and values, BatchWriteItem, TransactWriteItems, BatchGetItem, TransactGetItems, Scan count, DeleteItem), plus a stream test (INSERT, MODIFY, REMOVE records for such an item, with Keys, NewImage, and OldImage byte-identical) that runs against ExtendDB servers only, as the existing stream tests do. The 14 pass against DynamoDB itself and all 15 against all three backends on this branch. Before the change: 14 errors on PostgreSQL and MongoDB, 14 passing on SQLite; the stream test fails on both changed backends.
  • Unit tests: 11 on the shared escape (exhaustive order, prefix, collision, round trip, lenient legacy decode, JSON tree variants); 3 on the PostgreSQL key text wrappers; 5 in the MongoDB crate (round trip with the character in a top-level name, a nested map key, and a map key inside a list, with no BSON field name containing a NUL; control-free items store raw names; pushdown and native update refuse such names); the migration has a unit test on its change detector and live tests that seed legacy rows in every affected table, run the migration, check each rewrite, leave a literal-backslash \u0001 text row and plain rows byte-identical, confirm the second run is skipped, confirm jsonb rows are re-encoded with standard_conforming_strings off, confirm the startup gate reports the migration until it is recorded, and confirm the connection guard refuses while an unnamed connection is open and allows once it closes.
  • Gates: fmt, clippy (-D warnings, workspace), 1,230 workspace tests with the live PostgreSQL tests enabled, MSRV 1.88, and the full Python integration suites on servers built from this branch for all three backends.

Not in this change

  • U+0000 in key schema attribute names on PostgreSQL: the service accepts it; PostgreSQL returns 500 because the catalog stores the key schema as jsonb. Table and index names cannot contain it on the service either, but PostgreSQL answers a U+0000 table name with 500 Internal error during authorization where the service and the other backends answer the pattern ValidationException. Both are catalog and authorization paths, separate from item storage: [Bug] PostgreSQL: U+0000 in key schema attribute names returns 500; U+0000 in a table name fails in authorization instead of validation #367.
  • Legacy MongoDB field names containing a raw U+0001 followed by U+0001 or U+0002 are not rewritten; no migration exists for that backend. A field name containing U+0001 at all is the precondition, and a lone U+0001 reads back unchanged; values and key strings are unaffected. Documented in the upgrade manual.
  • Nothing prevents a server from an earlier release from being started against a migrated data database; it reads escaped rows as literal data. The manual says not to downgrade past this release without restoring from a pre-migration backup. A catalog version bump would make old binaries refuse such a database and is a release decision outside this change.

Deploy note

PostgreSQL deployments upgrade in this order: stop every server that uses the data database, run extenddb migrate --yes with the new binary, start the new servers. The new server refuses to start until the migration is recorded, and a server from an earlier release still writing during the migration would leave unescaped rows behind. A rolling deployment mixing the two releases on one data database is not supported for this release. Fresh deployments are unaffected.

@robinnsc robinnsc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One blocking item (the connection guard's coverage, inline on migrations.rs), one strong question on the downgrade hazard, one non-blocking question on the MongoDB asymmetry. The escape design itself checks out: I traced the order/prefix/collision argument, the migration's non-idempotence handling (per-table transaction plus progress marker, with the startup gate guaranteeing only legacy rows are ever scanned), and the backup path — create-backup copies stored rows verbatim from the data tables, so escaped form flows through backup_items transitively and there is no unescaped write path I could find.

// (an older release) or double-escaped (this release); see
// `refuse_if_other_clients_connected`.
if std::env::var_os(IGNORE_CONNECTIONS_ENV).is_none() {
refuse_if_other_clients_connected(data_pool).await?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking: this guard runs against the data pool only, but 004 also rewrites backup_items in the catalog database. When catalog and data are separate databases, a session holding only the catalog DB isn't counted, and a writer there during the rewrite leaves backup_items rows in the wrong encoding. The guard should also run against catalog_pool when the two databases differ — the application_name exclusion already covers the migrate command's own connections on both.

/// migration ran would have its own rows rewritten by it (see
/// [`escape_legacy_control_chars`]). `PostgresEngine::check_data_migrations_applied`
/// refuses to start on a data database missing any of these.
pub(crate) const REQUIRED_DATA_CODE_MIGRATIONS: &[&str] = &["004_escape_control_chars"];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

On the downgrade hazard: an older binary started against a migrated database silently reads escaped rows as literal data — the worst failure mode in this change, and the mitigation is a sentence in the manual. The PR description notes a catalog version bump would make old binaries refuse and defers it as a release decision. Since the version-gate mechanism already exists and the failure is silent corruption rather than an error, I'd like the bump to ride in this PR or the stack — or at minimum a tracked issue with an owner, so it doesn't live only in a doc note.


/// Reverse of [`json_to_item_data`]: BSON read from storage back to JSON with
/// the object-key escape undone.
pub(crate) fn item_data_to_json(value: &bson::Bson) -> Result<serde_json::Value, StorageError> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Non-blocking: this decoder changes how a legacy field name containing U+0001 U+0001 or U+0001 U+0002 reads back — silently, with no migration and no startup gate, disclosed only in the upgrade manual. The exposure is genuinely narrow (attribute names containing U+0001), so skipping the migration is defensible, but Postgres got a startup refusal for the same class of hazard and MongoDB got a paragraph. Is a one-time startup marker (refuse until the operator acknowledges, like the Postgres gate but with nothing to rewrite) cheap enough to close the asymmetry? If not, a sentence here pointing at the manual section would at least make the tradeoff visible at the code site.

jcshepherd
jcshepherd previously approved these changes Sep 21, 2026
@jcshepherd
jcshepherd added this pull request to stack #369 September 21, 2026 21:32
Base automatically changed from fix/hash-only-gsi-reverse-cursor to main September 21, 2026 22:12
DynamoDB accepts the character U+0000 anywhere a string appears: keys, index
keys, values, strings in lists, map keys, attribute names (measured against the
service 2026-09-18; it sorts as the byte 0x00 and is a different key from the
six-character text "\u0000"). PostgreSQL TEXT rejects the byte and jsonb
rejects the escape, so the backend answered 500 for every such item and for a
Query whose key value carried it. BSON field names are C strings, so MongoDB
answered 500 when the character appeared in an attribute name or map key.

Shared escape (crates/storage/src/util/control_chars.rs): U+0000 is stored as
U+0001 U+0001, U+0001 as U+0001 U+0002, everything else unchanged. Exhaustive
tests over every string up to length 4 on {U+0000, U+0001, U+0002, a} pin round
trip, UTF-8 byte order, prefix preservation, and distinctness, so COLLATE "C"
comparisons, BETWEEN, begins_with, and the row-comparison page cursors return
the rows the raw text would.

PostgreSQL: item documents go through item_to_json/json_to_item (every
item_data write, stream records, gsi_pending rows and contexts, vector rows);
key column text goes through data/key_text.rs wrappers at every producing site;
the TTL index and sweep and the vector search filters address attributes by the
escaped name. Migration 004_escape_control_chars re-encodes rows written by
earlier releases that contain U+0001, one table per transaction with a
per-table progress marker, only rows containing U+0001 touched, predicate
independent of standard_conforming_strings. The server refuses to start on a
data database that does not record the migration (rows this build writes are
indistinguishable from legacy rows to the migration), extenddb init records it
on fresh deployments, and extenddb migrate refuses to run it while other
sessions hold the data database.

MongoDB: item_to_document and its inverse escape object keys only (BSON string
values already hold the byte); conditions and native updates on names with
these characters use the existing in-process fallback paths, as names with "."
or "$" already do; field paths with no fallback use the escaped name.

Tests: tests/test_nul_strings.py (14 wire cases passing against DynamoDB and
all three backends, plus a stream record case on ExtendDB servers); unit tests
on the escape, the key wrappers, the MongoDB document conversion and
predicates; live migration tests for re-encoding, the second-run skip, the
standard_conforming_strings setting, the startup gate, and the connection
guard. Docs: storage design, upgrade manual.

Closes #364

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.

3 participants