From 8a2475922c3b9969d6ed2ff6038ebc614166164e Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sat, 24 Oct 2026 23:10:08 +0000 Subject: [PATCH 1/2] fix(storage-postgres): store attribute names containing U+0000 in the catalog and validate table names before authorization PostgreSQL jsonb rejects the \u0000 escape, so CreateTable with U+0000 in a key schema or attribute definition name failed at the catalog insert with 500. The catalog's key_schema, attribute_definitions and projection columns, the backup copies of them and the backup definition are now written through an order-preserving escape (U+0000 as U+0001 U+0001, U+0001 as U+0001 U+0002) and read back through its inverse, so DescribeTable, DescribeStream, DescribeBackup and a restore return the names byte-identical. Earlier releases stored a name containing U+0001 raw, which the decoder would read as an escape pair. Catalog migration 004_escape_catalog_control_chars.sql re-encodes those documents once, one transaction per table with a progress marker committed with the rewrite so an interrupted run resumes without visiting a table twice. It refuses to run while another session holds the catalog (the escape is not repeatable: an escaped U+0001 and a raw pair are the same bytes), skipping sessions named extenddb-migrate, and moves the catalog version to 0.0.5, so this release refuses an unmigrated catalog and an earlier release refuses a migrated one. A table whose key attribute, index key or projected attribute names contain U+0000 or U+0001 on PostgreSQL is not readable by an earlier release. A table name containing U+0000 reached the authorization key info and resource tag lookups, where PostgreSQL refuses the byte as a TEXT parameter, and the request was answered 500 on every backend that stores tags there. Every table name and ARN a request carries is now checked before those lookups, with the service's sentence: the member pattern (and the length clause when that fails too) at tableName, targetTableName, sourceTableName, tableCreationParameters.tableName, exclusiveStartTableName, indexName and transactItems..member..tableName; the map-key constraint list at requestItems; the "Table name of ARN" sentence for a plain table ARN with a bad table segment, the "Valid ARN format is" frame (or "Invalid subscriber ID", "Invalid AWS region") for any other arn: value, including an index or stream ARN of an existing table; "Invalid TableArn: Invalid Table ARN" for SourceTableArn and TableArn; "Invalid StreamArn", "Invalid Backup ARN" and "Invalid TableArn: Invalid ResourceArn provided as input " for the stream, backup and tag ARN fields; and ListStreams's own length sentence. Every sentence was measured on the service. Closes #367 --- .github/workflows/integration.yml | 1 + crates/app/src/cmd_migrate.rs | 14 +- crates/app/src/lib.rs | 1 + crates/app/src/migrate_connection.rs | 86 +++ crates/engine/src/update_table.rs | 34 +- crates/server/src/arn_gate.rs | 332 ++++++++++ crates/server/src/lib.rs | 2 + crates/server/src/request_helpers.rs | 6 + crates/server/src/table_name_gate.rs | 491 ++++++++++++++ .../004_escape_catalog_control_chars.sql | 27 + crates/storage-postgres/src/backup_engine.rs | 37 +- crates/storage-postgres/src/bootstrapper.rs | 22 +- crates/storage-postgres/src/catalog_escape.rs | 316 +++++++++ crates/storage-postgres/src/catalog_json.rs | 235 +++++++ crates/storage-postgres/src/create_table.rs | 19 +- crates/storage-postgres/src/data/ddl.rs | 19 +- crates/storage-postgres/src/data/gsi_build.rs | 11 +- crates/storage-postgres/src/data/index.rs | 10 +- .../storage-postgres/src/data/vector_index.rs | 8 +- crates/storage-postgres/src/lib.rs | 10 +- crates/storage-postgres/src/migrations.rs | 21 +- crates/storage-postgres/src/stream_engine.rs | 4 +- crates/storage-postgres/src/table_helpers.rs | 13 +- crates/storage-postgres/src/update_table.rs | 41 +- .../tests/catalog_converge.rs | 28 +- .../tests/catalog_escape_migration.rs | 550 ++++++++++++++++ .../tests/catalog_nul_names.rs | 615 ++++++++++++++++++ crates/storage/src/bootstrapper.rs | 11 + docs/differences-from-dynamodb.md | 7 +- docs/getting-started.md | 6 +- docs/manuals/01-architecture-guide.md | 2 +- docs/manuals/04-quickstart-setup-guide.md | 4 +- docs/manuals/05-admin-guide.md | 2 +- docs/manuals/07-upgrade-manual.md | 28 +- docs/manuals/08-install-linux.md | 2 +- docs/manuals/09-install-macos.md | 2 +- tests/python/conftest.py | 21 +- tests/python/helpers.py | 43 ++ tests/python/test_nul_arn_shapes.py | 164 +++++ tests/python/test_nul_metadata_names.py | 403 ++++++++++++ tests/python/test_nul_table_name_fields.py | 267 ++++++++ 41 files changed, 3783 insertions(+), 132 deletions(-) create mode 100644 crates/app/src/migrate_connection.rs create mode 100644 crates/server/src/arn_gate.rs create mode 100644 crates/server/src/table_name_gate.rs create mode 100644 crates/storage-postgres/migrations/004_escape_catalog_control_chars.sql create mode 100644 crates/storage-postgres/src/catalog_escape.rs create mode 100644 crates/storage-postgres/src/catalog_json.rs create mode 100644 crates/storage-postgres/tests/catalog_escape_migration.rs create mode 100644 crates/storage-postgres/tests/catalog_nul_names.rs create mode 100644 tests/python/test_nul_arn_shapes.py create mode 100644 tests/python/test_nul_metadata_names.py create mode 100644 tests/python/test_nul_table_name_fields.py diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 8a4818f75..cd64866db 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -312,6 +312,7 @@ jobs: --test vector_control_plane --test put_create_race --test twi_conflict --test stream_shard_retention --test backup_restore --test gsi_online_build --test gsi_stale_writer + --test catalog_nul_names --test catalog_escape_migration # The daemonized server logs to syslog; dump it so server-side failures # are diagnosable from the job log. diff --git a/crates/app/src/cmd_migrate.rs b/crates/app/src/cmd_migrate.rs index 2c9ea1be3..717bfe2ad 100755 --- a/crates/app/src/cmd_migrate.rs +++ b/crates/app/src/cmd_migrate.rs @@ -16,6 +16,8 @@ use extenddb_config as config; use extenddb_storage::error::StorageError; use extenddb_storage::management_store::OpError; +use crate::migrate_connection::with_migrate_application_name; + /// Exit status of a preview (`migrate` without `--yes`) that found work to do. /// Distinct from 1 so a script can tell "pending" from "failed". pub const EXIT_PENDING: u8 = 3; @@ -114,7 +116,8 @@ pub async fn run(args: MigrateArgs) -> anyhow::Result { // Serialize concurrent migrators, such as several replicas starting at // once, so they don't race each other applying schema changes. bootstrap.acquire_migration_lock().await.map_err(op_err)?; - let result = match CatalogResourceTagStep::open(app_config.storage.connection_config()).await { + let catalog_connection = with_migrate_application_name(app_config.storage.connection_config()); + let result = match CatalogResourceTagStep::open(&catalog_connection).await { Ok(rtag) => apply_migrations(bootstrap.as_ref(), &args, &rtag).await, Err(e) => Err(e), }; @@ -302,6 +305,15 @@ async fn apply_migrations( return Ok(EXIT_PENDING); } + // A migration that rewrites rows in place needs every other session off + // the databases it touches. Check before the first write, so a refusal + // leaves the catalog version where it was and an older binary can still be + // restarted. + bootstrap + .check_exclusive_access_for_pending_migrations() + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + if catalog_pending { bootstrap.run_catalog_migrations().await.map_err(op_err)?; } diff --git a/crates/app/src/lib.rs b/crates/app/src/lib.rs index 3857465bf..aa51d615d 100644 --- a/crates/app/src/lib.rs +++ b/crates/app/src/lib.rs @@ -36,6 +36,7 @@ mod cmd_verify; mod init_helpers; mod manage_http; mod manage_types; +mod migrate_connection; mod serve_helpers; mod util; diff --git a/crates/app/src/migrate_connection.rs b/crates/app/src/migrate_connection.rs new file mode 100644 index 000000000..a80d9a231 --- /dev/null +++ b/crates/app/src/migrate_connection.rs @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! The `application_name` the migrate command sets on the PostgreSQL sessions +//! it opens through the configured connection string. + +/// `application_name` the migrate command carries on every PostgreSQL session +/// it opens. The exclusive-access check for `004_escape_control_chars` +/// (`extenddb-storage-postgres`, `migrations::MIGRATE_APPLICATION_NAME`) counts +/// every session on the catalog and data databases except those with this +/// name. The bootstrapper sets it on its own pools; the catalog stores opened +/// by `CatalogResourceTagStep` connect through the configured connection +/// string and would otherwise be counted as foreign sessions, which makes +/// `migrate --yes` refuse every upgrade from a release without that migration. +const MIGRATE_APPLICATION_NAME: &str = "extenddb-migrate"; + +/// `connection` with its `application_name` set to `extenddb-migrate` when it is +/// a PostgreSQL URL. An `application_name` already in the query string is +/// replaced: the exclusive-access check skips only sessions with this exact +/// name, so a URL that keeps an operator's name would make `migrate --yes` +/// count its own sessions as foreign and refuse. Other backends' connection +/// strings are returned unchanged. +pub(crate) fn with_migrate_application_name(connection: &str) -> String { + let is_postgres = + connection.starts_with("postgres://") || connection.starts_with("postgresql://"); + if !is_postgres { + return connection.to_owned(); + } + let (base, query) = connection.split_once('?').unwrap_or((connection, "")); + let mut params: Vec<&str> = query + .split('&') + .filter(|p| !p.is_empty() && !p.starts_with("application_name=")) + .collect(); + let ours = format!("application_name={MIGRATE_APPLICATION_NAME}"); + params.push(&ours); + format!("{base}?{}", params.join("&")) +} + +#[cfg(test)] +mod application_name_tests { + use super::with_migrate_application_name; + + #[test] + fn a_postgres_url_without_a_query_string_gets_the_name() { + assert_eq!( + with_migrate_application_name("postgresql://app:pw@db:5432/extenddb_catalog"), + "postgresql://app:pw@db:5432/extenddb_catalog?application_name=extenddb-migrate" + ); + assert_eq!( + with_migrate_application_name("postgres://db/cat"), + "postgres://db/cat?application_name=extenddb-migrate" + ); + } + + #[test] + fn a_postgres_url_with_a_query_string_gets_the_name_appended() { + assert_eq!( + with_migrate_application_name("postgresql://db/cat?sslmode=require"), + "postgresql://db/cat?sslmode=require&application_name=extenddb-migrate" + ); + } + + #[test] + fn an_existing_application_name_is_replaced() { + assert_eq!( + with_migrate_application_name("postgresql://db/cat?application_name=ops"), + "postgresql://db/cat?application_name=extenddb-migrate" + ); + assert_eq!( + with_migrate_application_name( + "postgresql://db/cat?sslmode=require&application_name=ops&connect_timeout=5" + ), + "postgresql://db/cat?sslmode=require&connect_timeout=5&application_name=extenddb-migrate" + ); + } + + #[test] + fn other_backends_are_unchanged() { + for given in [ + "/var/lib/extenddb/extenddb.sqlite", + "mongodb://127.0.0.1:27017/?replicaSet=rs0", + "cassandra://127.0.0.1:9042", + ] { + assert_eq!(with_migrate_application_name(given), given); + } + } +} diff --git a/crates/engine/src/update_table.rs b/crates/engine/src/update_table.rs index 6db763f82..e6f57aabe 100755 --- a/crates/engine/src/update_table.rs +++ b/crates/engine/src/update_table.rs @@ -10,6 +10,30 @@ use serde_json::Value; use crate::OperationContext; use crate::serialize_output; +/// `validate_index_name` with the pattern clause located at the update's +/// member path, as the service reports it. The length clauses keep the +/// bare `indexName` location, which is not measured for UpdateTable. +fn validate_update_index_name( + name: &str, + position: usize, + action: &str, +) -> Result<(), DynamoDbError> { + match extenddb_core::validation::validate_index_name(name) { + Err(DynamoDbError::ValidationException(m)) + if m.contains("' at 'indexName' ") && m.contains("regular expression pattern") => + { + Err(DynamoDbError::ValidationException(m.replace( + "' at 'indexName' ", + &format!( + "' at 'globalSecondaryIndexUpdates.{}.member.{action}.indexName' ", + position + 1 + ), + ))) + } + other => other, + } +} + /// Handle `UpdateTable` — modify billing mode, throughput, deletion protection, /// or GSI configuration. /// @@ -209,7 +233,7 @@ pub async fn handle_update_table( /// (when given) a throughput of at least 1 unit each. fn validate_gsi_updates(input: &UpdateTableInput) -> Result<(), DynamoDbError> { if let Some(updates) = &input.global_secondary_index_updates { - for update in updates { + for (position, update) in updates.iter().enumerate() { if update.create.is_some() && update.delete.is_some() { return Err(DynamoDbError::ValidationException( "One or more parameter values were invalid: Only one of Create or Delete can be specified per GlobalSecondaryIndexUpdate".to_owned(), @@ -227,9 +251,11 @@ fn validate_gsi_updates(input: &UpdateTableInput) -> Result<(), DynamoDbError> { "One or more parameter values were invalid: GlobalSecondaryIndexUpdate must contain Create, Update, or Delete".to_owned(), )); } - // M3: Validate index names the same way CreateTable does. + // Index names are validated the way CreateTable does, except that the + // pattern clause carries the member's path (measured 2026-10-24: + // `globalSecondaryIndexUpdates..member.create.indexName`, 1-based). if let Some(create) = &update.create { - extenddb_core::validation::validate_index_name(&create.index_name)?; + validate_update_index_name(&create.index_name, position, "create")?; extenddb_core::validation::validate_index_throughput_minimum( &create.index_name, create.provisioned_throughput.as_ref(), @@ -260,7 +286,7 @@ fn validate_gsi_updates(input: &UpdateTableInput) -> Result<(), DynamoDbError> { } } if let Some(delete) = &update.delete { - extenddb_core::validation::validate_index_name(&delete.index_name)?; + validate_update_index_name(&delete.index_name, position, "delete")?; } } } diff --git a/crates/server/src/arn_gate.rs b/crates/server/src/arn_gate.rs new file mode 100644 index 000000000..22bbcf101 --- /dev/null +++ b/crates/server/src/arn_gate.rs @@ -0,0 +1,332 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The ARN shapes `table_name_gate` refuses before any catalog lookup, with the +//! service's sentences (measured 2026-10-24 unless marked derived). +//! +//! A `TableName` may be a table ARN. The service checks the ARN's shape and +//! each of its segments before it resolves the table: +//! +//! | `TableName` | sentence at `'tableName'` | +//! |---|---| +//! | a table ARN whose table segment fails the pattern | `Table name of ARN must satisfy regular expression pattern: ...` | +//! | a dynamodb ARN with a bad region segment | `Invalid AWS region` | +//! | a dynamodb ARN with a bad account segment | `Invalid subscriber ID` | +//! | anything else that is not a plain table ARN (another service, an index or stream suffix, too few fields) | the `Valid ARN format is ...` frame | +//! +//! `DescribeTable` with an ARN of another service is `AccessDeniedException` +//! on the service; that answer is left to the engine, so the frame is not +//! applied to it there. The frame for a well-formed index or stream ARN in +//! `TableName` is derived from the measured index ARN with a bad segment. +//! +//! The other ARN-bearing fields: `SourceTableArn` (`RestoreTableToPointInTime`) +//! and `TableArn` (`ExportTableToPointInTime`) with a bad table segment, +//! `StreamArn` (`DescribeStream`, `GetShardIterator`) and a tag operation's +//! stream `ResourceArn` with a bad table segment or U+0000 in the label, +//! `BackupArn` (`DescribeBackup`, `DeleteBackup`, `RestoreTableFromBackup`) +//! with a bad table segment, and a tag operation's table `ResourceArn` with a +//! bad table segment. Each has its own sentence below. + +use extenddb_core::arn::{SubResource, fields, parse}; +use extenddb_core::error::DynamoDbError; + +pub(crate) const PATTERN: &str = "[a-zA-Z0-9_.-]+"; + +const FRAME: &str = "Valid ARN format is \ + 'arn:::::resourceType/resourceName', \ + where Resource name must have length less than or equal to 255, \ + Resource name must have length greater than or equal to 3, \ + Resource name must satisfy regular expression pattern: [a-zA-Z0-9_.-]+"; + +/// The `RequestItems` map-key constraint list for a key that is an ARN. +pub(crate) const MAP_KEYS_ARN: &str = "Map keys must satisfy constraint: \ + [Valid resource name or resource arn must be given. Valid resource arn format is \ + 'arn:::::resourceType/resourceName', \ + where Resource name must have length less than or equal to 255, \ + Resource name must have length greater than or equal to 3, \ + Resource name must satisfy regular expression pattern: [a-zA-Z0-9_.-]+]"; + +pub(crate) fn in_pattern(name: &str) -> bool { + name.chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.' || c == '-') +} + +fn validation(message: String) -> DynamoDbError { + DynamoDbError::ValidationException(message) +} + +fn one_error(value: &str, location: &str, constraint: &str) -> DynamoDbError { + validation(format!( + "1 validation error detected: Value '{value}' at '{location}' failed to satisfy \ + constraint: {constraint}" + )) +} + +/// What is wrong with an `arn:` value read as a table ARN. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum ArnFault { + /// A plain table ARN whose table segment fails the pattern. + TableSegment, + /// A dynamodb ARN whose region segment carries a character outside `[a-z0-9-]`. + Region, + /// A dynamodb ARN whose account segment carries a character that is not a digit. + Account, + /// Not a plain table ARN of this service: too few fields, another service, + /// an index or stream or backup suffix, or a shape `arn::parse` rejects. + NotPlainTable, +} + +/// Inspect an `arn:` value the way the service checks a `TableName` ARN. +/// `None` means a well-formed plain table ARN whose segments all pass. +pub(crate) fn table_arn_fault(arn: &str) -> Option { + let Some(f) = fields(arn) else { + return Some(ArnFault::NotPlainTable); + }; + if !f + .region + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-') + { + return Some(ArnFault::Region); + } + if !f.account.chars().all(|c| c.is_ascii_digit()) { + return Some(ArnFault::Account); + } + if f.service != "dynamodb" { + return Some(ArnFault::NotPlainTable); + } + match parse(arn) { + Some(res) if res.sub.is_none() => { + (!in_pattern(&res.table)).then_some(ArnFault::TableSegment) + } + _ => Some(ArnFault::NotPlainTable), + } +} + +fn arn_constraint(fault: ArnFault) -> &'static str { + match fault { + ArnFault::TableSegment => { + "Table name of ARN must satisfy regular expression pattern: [a-zA-Z0-9_.-]+" + } + ArnFault::Region => "Invalid AWS region", + ArnFault::Account => "Invalid subscriber ID", + ArnFault::NotPlainTable => FRAME, + } +} + +/// A `TableName` that is an ARN, for `operation`. `Ok` passes the value on to +/// the handlers, which resolve a well-formed table ARN. +pub(crate) fn check_table_name_arn(operation: &str, arn: &str) -> Result<(), DynamoDbError> { + let Some(fault) = table_arn_fault(arn) else { + return Ok(()); + }; + // DescribeTable answers a foreign-service ARN with AccessDeniedException on + // the service; the engine's own answer stands for that one shape. + if operation == "DescribeTable" + && fault == ArnFault::NotPlainTable + && fields(arn).is_some_and(|f| f.service != "dynamodb") + { + return Ok(()); + } + Err(one_error(arn, "tableName", arn_constraint(fault))) +} + +/// A `TransactItems` member's `TableName` that is an ARN, at `location` +/// (`transactItems..member..tableName`). A plain table ARN with a bad +/// table segment is measured; the other faults take the same constraints as a +/// top-level `TableName` (derived). +pub(crate) fn check_transact_member_arn(arn: &str, location: &str) -> Result<(), DynamoDbError> { + match table_arn_fault(arn) { + None => Ok(()), + Some(fault) => Err(one_error(arn, location, arn_constraint(fault))), + } +} + +/// `SourceTableArn` of `RestoreTableToPointInTime` and `TableArn` of +/// `ExportTableToPointInTime` with a table segment that fails the pattern. +pub(crate) fn check_table_arn_field(operation: &str, arn: &str) -> Result<(), DynamoDbError> { + let bad = matches!(parse(arn), Some(res) if res.sub.is_none() && !in_pattern(&res.table)); + if !bad { + return Ok(()); + } + Err(validation( + match operation { + "RestoreTableToPointInTime" => "Invalid Request: Invalid TableArn: Invalid Table ARN", + _ => "Invalid TableArn: Invalid Table ARN", + } + .to_owned(), + )) +} + +/// A stream ARN whose table segment fails the pattern or whose label carries +/// U+0000: `Invalid StreamArn` (measured for `DescribeStream`, +/// `GetShardIterator` and `TagResource`). +pub(crate) fn check_stream_arn(arn: &str) -> Result<(), DynamoDbError> { + match parse(arn) { + Some(res) + if matches!(&res.sub, Some(SubResource::Stream(label)) + if !in_pattern(&res.table) || label.contains('\u{0}')) => + { + Err(validation("Invalid StreamArn".to_owned())) + } + _ => Ok(()), + } +} + +/// A backup ARN whose table segment fails the pattern: `Invalid Backup ARN` +/// (measured for `DescribeBackup`, `DeleteBackup` and +/// `RestoreTableFromBackup`). U+0000 in the backup id keeps the engine's +/// `Invalid Request: BackupArn is not valid`, as measured. +pub(crate) fn check_backup_arn(arn: &str) -> Result<(), DynamoDbError> { + match parse(arn.trim_matches(|c: char| c.is_ascii_whitespace())) { + Some(res) if matches!(res.sub, Some(SubResource::Backup(_))) && !in_pattern(&res.table) => { + Err(validation("Invalid Backup ARN".to_owned())) + } + _ => Ok(()), + } +} + +/// A tag operation's `ResourceArn`: a table ARN with a bad table segment is +/// `Invalid TableArn: Invalid ResourceArn provided as input ` (measured +/// for `TagResource`; `UntagResource` and `ListTagsOfResource` the same), a +/// stream ARN with a bad table segment or U+0000 in the label is +/// `Invalid StreamArn`. An index or backup ARN keeps the shape check's verdict. +pub(crate) fn check_tag_resource_arn(arn: &str) -> Result<(), DynamoDbError> { + check_stream_arn(arn)?; + match parse(arn) { + Some(res) if res.sub.is_none() && !in_pattern(&res.table) => Err(validation(format!( + "Invalid TableArn: Invalid ResourceArn provided as input {arn}" + ))), + _ => Ok(()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const ACCT: &str = "964157134968"; + fn table(name: &str) -> String { + format!("arn:aws:dynamodb:us-east-1:{ACCT}:table/{name}") + } + fn message(r: Result<(), DynamoDbError>) -> Option { + match r { + Ok(()) => None, + Err(DynamoDbError::ValidationException(m)) => Some(m), + Err(other) => panic!("{other:?}"), + } + } + + #[test] + fn a_plain_table_arn_with_a_bad_segment_gets_the_table_name_of_arn_sentence() { + let arn = table("t367\u{0}name"); + let m = message(check_table_name_arn("PutItem", &arn)).expect("refused"); + assert_eq!( + m, + format!( + "1 validation error detected: Value '{arn}' at 'tableName' failed to satisfy \ + constraint: Table name of ARN must satisfy regular expression pattern: \ + [a-zA-Z0-9_.-]+" + ) + ); + assert!(check_table_name_arn("PutItem", &table("t367name")).is_ok()); + } + + #[test] + fn region_and_account_faults_name_their_segment() { + let region = format!("arn:aws:dynamodb:us-ea\u{0}st-1:{ACCT}:table/t367name"); + assert!( + message(check_table_name_arn("PutItem", ®ion)) + .unwrap() + .ends_with("failed to satisfy constraint: Invalid AWS region") + ); + let account = "arn:aws:dynamodb:us-east-1:1234\u{0}56789012:table/t367name"; + assert!( + message(check_table_name_arn("PutItem", account)) + .unwrap() + .ends_with("failed to satisfy constraint: Invalid subscriber ID") + ); + } + + #[test] + fn other_shapes_get_the_valid_arn_format_frame() { + for arn in [ + "arn:aws:s3:::t367\u{0}name".to_owned(), + format!("{}/index/i\u{0}x", table("t367name")), + format!("{}/index/ix", table("t367\u{0}name")), + format!("{}/stream/2026-10-24T00:00:00.000", table("t367\u{0}name")), + format!("{}/index/ix", table("t367name")), + "arn:\u{0}".to_owned(), + format!("{}/", table("t367\u{0}name")), + ] { + let m = + message(check_table_name_arn("Query", &arn)).unwrap_or_else(|| panic!("{arn:?}")); + assert_eq!( + m, + format!( + "1 validation error detected: Value '{arn}' at 'tableName' failed to satisfy \ + constraint: {FRAME}" + ) + ); + } + let m = message(check_transact_member_arn( + &table("t367\u{0}name"), + "transactItems.1.member.put.tableName", + )) + .unwrap(); + assert!(m.contains("at 'transactItems.1.member.put.tableName' failed to satisfy constraint: Table name of ARN"), "{m}"); + } + + #[test] + fn describe_table_leaves_a_foreign_service_arn_to_the_engine() { + assert!(check_table_name_arn("DescribeTable", "arn:aws:s3:::t367\u{0}name").is_ok()); + assert!( + check_table_name_arn( + "DescribeTable", + &format!("{}/index/ix", table("t367\u{0}name")) + ) + .is_err() + ); + } + + #[test] + fn table_arn_fields_stream_backup_and_tag_arns() { + let bad = table("t367\u{0}name"); + assert_eq!( + message(check_table_arn_field("RestoreTableToPointInTime", &bad)).unwrap(), + "Invalid Request: Invalid TableArn: Invalid Table ARN" + ); + assert_eq!( + message(check_table_arn_field("ExportTableToPointInTime", &bad)).unwrap(), + "Invalid TableArn: Invalid Table ARN" + ); + assert!(check_table_arn_field("ExportTableToPointInTime", &table("ok")).is_ok()); + let stream = format!("{bad}/stream/2026-10-24T00:00:00.000"); + assert_eq!( + message(check_stream_arn(&stream)).unwrap(), + "Invalid StreamArn" + ); + let label = format!("{}/stream/2026\u{0}", table("ok")); + assert_eq!( + message(check_stream_arn(&label)).unwrap(), + "Invalid StreamArn" + ); + assert!(check_stream_arn(&format!("{}/stream/2026", table("ok"))).is_ok()); + let backup = format!("{bad}/backup/01700000000000-abcdef12"); + assert_eq!( + message(check_backup_arn(&backup)).unwrap(), + "Invalid Backup ARN" + ); + let backup_id = format!("{}/backup/0170\u{0}", table("ok")); + assert!(check_backup_arn(&backup_id).is_ok(), "left to the engine"); + assert_eq!( + message(check_tag_resource_arn(&bad)).unwrap(), + format!("Invalid TableArn: Invalid ResourceArn provided as input {bad}") + ); + assert_eq!( + message(check_tag_resource_arn(&stream)).unwrap(), + "Invalid StreamArn" + ); + assert!(check_tag_resource_arn(&format!("{bad}/index/i")).is_ok()); + } +} diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index 69dc9e0a4..74f4d2ac6 100755 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -8,6 +8,7 @@ //! with correct headers (x-amzn-RequestId, x-amz-crc32, Content-Type). //! Supports TLS via `axum-server` with rustls when configured. +pub(crate) mod arn_gate; pub(crate) mod authorization; pub mod authz_cache; pub(crate) mod authz_resource; @@ -23,6 +24,7 @@ pub mod rate_limit; mod request_helpers; mod response; mod serve; +mod table_name_gate; mod throttle_helpers; mod workers; diff --git a/crates/server/src/request_helpers.rs b/crates/server/src/request_helpers.rs index e7d12abf9..98604f0ed 100755 --- a/crates/server/src/request_helpers.rs +++ b/crates/server/src/request_helpers.rs @@ -108,6 +108,12 @@ pub(crate) async fn authorize_request( source_ip: Option<&str>, user_agent: Option<&str>, ) -> Result { + // A table name outside the service's character pattern, in TableName or + // nested in RequestItems or TransactItems, is refused here, before the key + // info and resource tag lookups below bind it as a database parameter (see + // `table_name_gate`). + crate::table_name_gate::check_table_name_chars(operation, input)?; + let table_name = extract_table_name(input); let mut authorized_key_info = extenddb_engine::AuthorizedKeyInfo::new(); diff --git a/crates/server/src/table_name_gate.rs b/crates/server/src/table_name_gate.rs new file mode 100644 index 000000000..e2fa0b89a --- /dev/null +++ b/crates/server/src/table_name_gate.rs @@ -0,0 +1,491 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The table name checks that run before any catalog lookup. +//! +//! A request's table names reach storage before the engine validates them: +//! authorization looks up each table's key schema (for `dynamodb:LeadingKeys`) +//! and the resource tags for the table ARN it builds from the name. On +//! PostgreSQL a name containing U+0000 cannot be bound as a `TEXT` parameter +//! (`invalid byte sequence for encoding "UTF8": 0x00`), so those lookups failed +//! and the request was answered 500 `Internal error during authorization`. +//! The service answers 400 `ValidationException`, and it does so before the +//! IAM decision: a principal with no DynamoDB permission gets the same +//! `ValidationException` for a bad name and `AccessDeniedException` for a real +//! one (measured 2026-10-24). This module runs first in `authorize_request`. +//! +//! Sentences (measured 2026-09-18 and 2026-10-24; derived cases are marked): +//! +//! - A bare name outside `[a-zA-Z0-9_.-]+`: `Value '' at '' +//! failed to satisfy constraint: Member must satisfy regular expression +//! pattern: ...`. When the name is also shorter than 3 or longer than 255 +//! characters the message lists both clauses, the length first, joined by +//! `; ` under `2 validation errors detected:` (measured at `tableName`; +//! derived for the other locations). Locations: `tableName` for `TableName`, +//! `targetTableName`, `sourceTableName`, `tableCreationParameters.tableName`, +//! `exclusiveStartTableName`, `indexName` for `Query` and `Scan`, and +//! `transactItems..member..tableName` (1-based, `put`, `get`, +//! `delete`, `update`, `conditionCheck`). A name inside the pattern but of a +//! bad length is left to the engine handlers, whose answer the service's +//! matches already. +//! - `ListStreams` has its own validator: `TableName must be at least 3 +//! characters long and at most 255 characters long` for a U+0000 name +//! (measured); applied to any character outside the pattern (derived). +//! - A `RequestItems` key outside the pattern: `at 'requestItems'` with the +//! map-key constraint list. The service renders its own dump of the whole +//! map inside `Value '...'` and names no key first; the first offending key +//! in request order (`serde_json` keeps it) is rendered alone here. A key +//! that is an ARN takes the ARN constraint list (`arn_gate::MAP_KEYS_ARN`). +//! - ARN-shaped values: see `arn_gate`. + +use extenddb_core::error::DynamoDbError; +use serde_json::Value; + +use crate::arn_gate::{self, PATTERN, in_pattern}; + +const MAP_KEYS_NAME: &str = "Map keys must satisfy constraint: \ + [Member must have length less than or equal to 255, \ + Member must have length greater than or equal to 3, \ + Member must satisfy regular expression pattern: [a-zA-Z0-9_.-]+]"; + +/// Refuse a table name outside the service's character pattern, in any field +/// the operation carries one, before the name is used in any lookup. A +/// missing, non-string or empty name passes. +pub(crate) fn check_table_name_chars(operation: &str, input: &Value) -> Result<(), DynamoDbError> { + match operation { + "BatchGetItem" | "BatchWriteItem" => check_request_items_keys(input), + "TransactGetItems" => check_transact_items(input, &["Get"]), + "TransactWriteItems" => { + check_transact_items(input, &["Put", "Delete", "Update", "ConditionCheck"]) + } + "TagResource" | "UntagResource" | "ListTagsOfResource" => { + with_str(input, "ResourceArn", arn_gate::check_tag_resource_arn) + } + "DescribeStream" | "GetShardIterator" => { + with_str(input, "StreamArn", arn_gate::check_stream_arn) + } + "DescribeBackup" | "DeleteBackup" => { + with_str(input, "BackupArn", arn_gate::check_backup_arn) + } + "RestoreTableFromBackup" => { + with_str(input, "BackupArn", arn_gate::check_backup_arn)?; + check_name(input.get("TargetTableName"), "targetTableName", operation) + } + "RestoreTableToPointInTime" => { + check_name(input.get("SourceTableName"), "sourceTableName", operation)?; + check_name(input.get("TargetTableName"), "targetTableName", operation)?; + with_str(input, "SourceTableArn", |a| { + arn_gate::check_table_arn_field(operation, a) + }) + } + "ExportTableToPointInTime" => with_str(input, "TableArn", |a| { + arn_gate::check_table_arn_field(operation, a) + }), + "ImportTable" => check_name( + input + .get("TableCreationParameters") + .and_then(|p| p.get("TableName")), + "tableCreationParameters.tableName", + operation, + ), + "ListTables" => check_name( + input.get("ExclusiveStartTableName"), + "exclusiveStartTableName", + operation, + ), + "ListStreams" => match input.get("TableName").and_then(Value::as_str) { + Some(name) if name.starts_with("arn:") => { + arn_gate::check_table_name_arn(operation, name) + } + Some(name) if !name.is_empty() && !in_pattern(name) => { + Err(DynamoDbError::ValidationException( + "TableName must be at least 3 characters long and at most 255 characters long" + .to_owned(), + )) + } + _ => Ok(()), + }, + "Query" | "Scan" => { + check_name(input.get("TableName"), "tableName", operation)?; + match input.get("IndexName").and_then(Value::as_str) { + // Pattern only: the length clauses for an index name are unmeasured. + Some(index) if !index.is_empty() && !in_pattern(index) => { + Err(one_error(index, "indexName", &pattern_clause())) + } + _ => Ok(()), + } + } + _ => check_name(input.get("TableName"), "tableName", operation), + } +} + +fn with_str( + input: &Value, + field: &str, + check: impl Fn(&str) -> Result<(), DynamoDbError>, +) -> Result<(), DynamoDbError> { + match input.get(field).and_then(Value::as_str) { + Some(s) if !s.is_empty() => check(s), + _ => Ok(()), + } +} + +fn pattern_clause() -> String { + format!("Member must satisfy regular expression pattern: {PATTERN}") +} + +fn one_error(value: &str, location: &str, constraint: &str) -> DynamoDbError { + DynamoDbError::ValidationException(format!( + "1 validation error detected: Value '{value}' at '{location}' failed to satisfy \ + constraint: {constraint}" + )) +} + +/// A table name field: a bare name outside the pattern is refused with the +/// pattern clause (and the length clause when that fails too); an `arn:` value +/// goes through the ARN rules. +fn check_name(value: Option<&Value>, location: &str, operation: &str) -> Result<(), DynamoDbError> { + let Some(name) = value.and_then(Value::as_str) else { + return Ok(()); + }; + if name.starts_with("arn:") { + return arn_gate::check_table_name_arn(operation, name); + } + if name.is_empty() || in_pattern(name) { + return Ok(()); + } + Err(bare_name_error(name, location)) +} + +/// The service lists every failed constraint of a member in one message, the +/// length clause before the pattern clause. +fn bare_name_error(name: &str, location: &str) -> DynamoDbError { + let length = name.chars().count(); + let mut clauses: Vec = Vec::with_capacity(2); + if length < 3 { + clauses.push("Member must have length greater than or equal to 3".to_owned()); + } else if length > 255 { + clauses.push("Member must have length less than or equal to 255".to_owned()); + } + clauses.push(pattern_clause()); + let n = clauses.len(); + DynamoDbError::ValidationException(format!( + "{n} validation error{} detected: {}", + if n == 1 { "" } else { "s" }, + clauses + .iter() + .map(|c| format!("Value '{name}' at '{location}' failed to satisfy constraint: {c}")) + .collect::>() + .join("; ") + )) +} + +/// A `RequestItems` key outside the pattern, or an ARN key that is not a +/// well-formed plain table ARN. The sentence from `at 'requestItems'` onward +/// is the service's; the offending key alone is rendered where the service +/// renders the whole map. +fn check_request_items_keys(input: &Value) -> Result<(), DynamoDbError> { + let Some(map) = input.get("RequestItems").and_then(Value::as_object) else { + return Ok(()); + }; + for key in map.keys() { + if key.is_empty() { + continue; + } + if key.starts_with("arn:") { + if arn_gate::table_arn_fault(key).is_some() { + return Err(one_error(key, "requestItems", arn_gate::MAP_KEYS_ARN)); + } + } else if !in_pattern(key) { + return Err(one_error(key, "requestItems", MAP_KEYS_NAME)); + } + } + Ok(()) +} + +/// A `TransactItems` member whose sub-operation names a table outside the +/// pattern, or by an ARN that is not a well-formed plain table ARN. +fn check_transact_items(input: &Value, subs: &[&str]) -> Result<(), DynamoDbError> { + let Some(items) = input.get("TransactItems").and_then(Value::as_array) else { + return Ok(()); + }; + for (i, item) in items.iter().enumerate() { + for sub in subs { + let Some(name) = item + .get(sub) + .and_then(|o| o.get("TableName")) + .and_then(Value::as_str) + else { + continue; + }; + if name.is_empty() { + continue; + } + let location = format!( + "transactItems.{}.member.{}.tableName", + i + 1, + lower_camel(sub) + ); + if name.starts_with("arn:") { + arn_gate::check_transact_member_arn(name, &location)?; + } else if !in_pattern(name) { + return Err(bare_name_error(name, &location)); + } + } + } + Ok(()) +} + +fn lower_camel(s: &str) -> String { + let mut c = s.chars(); + match c.next() { + Some(f) => f.to_ascii_lowercase().to_string() + c.as_str(), + None => String::new(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + const NUL: &str = "t367\u{0}name"; + const ARN: &str = "arn:aws:dynamodb:us-east-1:964157134968:table/t367\u{0}name"; + const PAT: &str = "Member must satisfy regular expression pattern: [a-zA-Z0-9_.-]+"; + + fn message(operation: &str, input: Value) -> Option { + match check_table_name_chars(operation, &input) { + Ok(()) => None, + Err(DynamoDbError::ValidationException(m)) => Some(m), + Err(other) => panic!("unexpected error {other:?}"), + } + } + + fn one(value: &str, location: &str, constraint: &str) -> String { + format!( + "1 validation error detected: Value '{value}' at '{location}' failed to satisfy \ + constraint: {constraint}" + ) + } + + #[test] + fn nul_in_table_name_is_refused_with_the_pattern_message() { + let m = message("DescribeTable", json!({ "TableName": NUL })).expect("refused"); + assert_eq!(m, one(NUL, "tableName", PAT)); + } + + #[test] + fn other_characters_outside_the_pattern_are_refused_and_plain_names_pass() { + for name in [ + "a b", + "tab\tle", + "t/name", + "t:name", + "caf\u{e9}", + "\u{0}t367name", + ] { + assert!( + message("PutItem", json!({ "TableName": name })).is_some(), + "{name:?}" + ); + } + for name in ["abc", "Table_1.a-b", &"x".repeat(300), "ab"] { + assert!( + message("Query", json!({ "TableName": name })).is_none(), + "{name:?}" + ); + } + for input in [ + json!({}), + json!({ "TableName": "" }), + json!({ "TableName": null }), + json!({ "TableName": 7 }), + json!({ "TableName": "arn:aws:dynamodb:us-east-1:123456789012:table/t" }), + ] { + assert!(message("GetItem", input.clone()).is_none(), "{input}"); + } + } + + #[test] + fn a_name_failing_two_constraints_lists_the_length_clause_first() { + let m = message("DescribeTable", json!({ "TableName": "\u{0}" })).expect("refused"); + assert_eq!( + m, + format!( + "2 validation errors detected: Value '\u{0}' at 'tableName' failed to satisfy \ + constraint: Member must have length greater than or equal to 3; \ + Value '\u{0}' at 'tableName' failed to satisfy constraint: {PAT}" + ) + ); + let long = format!("{}\u{0}", "x".repeat(300)); + let m = message("CreateTable", json!({ "TableName": long })).expect("refused"); + assert!(m.starts_with("2 validation errors detected: "), "{m}"); + assert!( + m.contains("Member must have length less than or equal to 255; "), + "{m}" + ); + } + + #[test] + fn the_other_name_fields_report_their_own_location() { + let cases = [ + ( + "RestoreTableFromBackup", + json!({ "TargetTableName": NUL }), + "targetTableName", + ), + ( + "RestoreTableToPointInTime", + json!({ "SourceTableName": NUL }), + "sourceTableName", + ), + ( + "RestoreTableToPointInTime", + json!({ "TargetTableName": NUL }), + "targetTableName", + ), + ( + "ImportTable", + json!({ "TableCreationParameters": { "TableName": NUL } }), + "tableCreationParameters.tableName", + ), + ( + "ListTables", + json!({ "ExclusiveStartTableName": NUL }), + "exclusiveStartTableName", + ), + ("ListBackups", json!({ "TableName": NUL }), "tableName"), + ("UpdateTimeToLive", json!({ "TableName": NUL }), "tableName"), + ]; + for (op, input, location) in cases { + assert_eq!( + message(op, input).as_deref(), + Some(one(NUL, location, PAT).as_str()), + "{op}" + ); + } + assert_eq!( + message( + "Query", + json!({ "TableName": "ok", "IndexName": "g\u{0}2" }) + ) + .unwrap(), + one("g\u{0}2", "indexName", PAT) + ); + assert!(message("Scan", json!({ "TableName": "ok", "IndexName": "gsi1" })).is_none()); + assert_eq!( + message("ListStreams", json!({ "TableName": NUL })).unwrap(), + "TableName must be at least 3 characters long and at most 255 characters long" + ); + } + + #[test] + fn arn_bearing_fields_are_checked_before_authorization() { + let m = message("DescribeTable", json!({ "TableName": ARN })).expect("refused"); + assert!(m.contains("Table name of ARN must satisfy"), "{m}"); + assert_eq!( + message( + "RestoreTableToPointInTime", + json!({ "SourceTableArn": ARN }) + ) + .unwrap(), + "Invalid Request: Invalid TableArn: Invalid Table ARN" + ); + assert_eq!( + message("ExportTableToPointInTime", json!({ "TableArn": ARN })).unwrap(), + "Invalid TableArn: Invalid Table ARN" + ); + let stream = format!("{ARN}/stream/2026-10-24T00:00:00.000"); + for op in ["DescribeStream", "GetShardIterator"] { + assert_eq!( + message(op, json!({ "StreamArn": stream })).unwrap(), + "Invalid StreamArn" + ); + } + let backup = format!("{ARN}/backup/01700000000000-abcdef12"); + for op in ["DescribeBackup", "DeleteBackup", "RestoreTableFromBackup"] { + assert_eq!( + message(op, json!({ "BackupArn": backup })).unwrap(), + "Invalid Backup ARN" + ); + } + for op in ["TagResource", "UntagResource", "ListTagsOfResource"] { + assert_eq!( + message(op, json!({ "ResourceArn": ARN })).unwrap(), + format!("Invalid TableArn: Invalid ResourceArn provided as input {ARN}") + ); + assert_eq!( + message(op, json!({ "ResourceArn": stream })).unwrap(), + "Invalid StreamArn" + ); + assert!(message(op, json!({ "ResourceArn": format!("{ARN}/index/i") })).is_none()); + assert!(message(op, json!({})).is_none()); + } + } + + #[test] + fn batch_request_items_keys_are_refused_at_request_items() { + for op in ["BatchWriteItem", "BatchGetItem"] { + let m = message(op, json!({ "RequestItems": { "ok": {}, NUL: {} } })).expect("refused"); + assert_eq!(m, one(NUL, "requestItems", MAP_KEYS_NAME)); + let m = message(op, json!({ "RequestItems": { ARN: {} } })).expect("refused"); + assert_eq!(m, one(ARN, "requestItems", arn_gate::MAP_KEYS_ARN)); + assert!(message(op, json!({ "RequestItems": { "ok": {} } })).is_none()); + assert!(message(op, json!({ "RequestItems": [] })).is_none()); + } + } + + #[test] + fn transact_members_are_refused_with_their_member_index_and_operation() { + let m = message( + "TransactWriteItems", + json!({ "TransactItems": [ + { "Put": { "TableName": "ok", "Item": {} } }, + { "Delete": { "TableName": NUL, "Key": {} } }, + ] }), + ) + .expect("refused"); + assert_eq!(m, one(NUL, "transactItems.2.member.delete.tableName", PAT)); + for (sub, member) in [("Update", "update"), ("ConditionCheck", "conditionCheck")] { + let m = message( + "TransactWriteItems", + json!({ "TransactItems": [ { sub: { "TableName": NUL } } ] }), + ) + .expect("refused"); + assert_eq!( + m, + one( + NUL, + &format!("transactItems.1.member.{member}.tableName"), + PAT + ) + ); + } + let m = message( + "TransactGetItems", + json!({ "TransactItems": [ { "Get": { "TableName": NUL, "Key": {} } } ] }), + ) + .expect("refused"); + assert_eq!(m, one(NUL, "transactItems.1.member.get.tableName", PAT)); + let m = message( + "TransactWriteItems", + json!({ "TransactItems": [ { "Put": { "TableName": ARN, "Item": {} } } ] }), + ) + .expect("refused"); + assert_eq!( + m, + one( + ARN, + "transactItems.1.member.put.tableName", + "Table name of ARN must satisfy regular expression pattern: [a-zA-Z0-9_.-]+" + ) + ); + assert!( + message( + "TransactWriteItems", + json!({ "TransactItems": [ { "Delete": { "TableName": "ok" } } ] }) + ) + .is_none() + ); + } +} diff --git a/crates/storage-postgres/migrations/004_escape_catalog_control_chars.sql b/crates/storage-postgres/migrations/004_escape_catalog_control_chars.sql new file mode 100644 index 000000000..8992bf8f0 --- /dev/null +++ b/crates/storage-postgres/migrations/004_escape_catalog_control_chars.sql @@ -0,0 +1,27 @@ +-- Copyright 2026 ExtendDB contributors +-- SPDX-License-Identifier: Apache-2.0 +-- Migration 004: catalog attribute names stored through the control +-- character escape (catalog version 0.0.5). +-- +-- This migration changes no schema. It moves the catalog version because the +-- stored form of seven catalog columns changed: from this release, the +-- attribute names in tables.key_schema, tables.attribute_definitions, +-- indexes.key_schema, indexes.projection, backups.key_schema, +-- backups.attribute_definitions and backup_definitions.definition are +-- written through an order-preserving escape (U+0000 as U+0001 U+0001, +-- U+0001 as U+0001 U+0002), so a key attribute name may contain U+0000 as it +-- may on DynamoDB. Before this file is applied, the runner re-encodes every +-- stored document that holds a raw U+0001 (crates/storage-postgres/src/ +-- catalog_escape.rs), with every other session off the catalog. +-- +-- A binary from an earlier release knows nothing about the escape and would +-- read an escaped name as literal data, silently. The catalog version check at +-- startup is the one check such a binary already performs, so moving the +-- version is what makes it refuse a migrated catalog instead of serving wrong +-- names. + +BEGIN; + +UPDATE settings SET value = '0.0.5' WHERE key = 'catalog_version'; + +COMMIT; diff --git a/crates/storage-postgres/src/backup_engine.rs b/crates/storage-postgres/src/backup_engine.rs index 20baee942..4d28e2b60 100755 --- a/crates/storage-postgres/src/backup_engine.rs +++ b/crates/storage-postgres/src/backup_engine.rs @@ -25,6 +25,7 @@ use futures::future::BoxFuture; use tokio::sync::{OwnedSemaphorePermit, Semaphore}; use crate::PostgresEngine; +use crate::catalog_json::{escape_json, from_catalog_json, to_catalog_json, unescape_json}; use crate::create_table::RestoreRegistration; use crate::data::{ all_sort_key_info, data_table_name, index_table_name, insert_index_rows_multi, @@ -396,9 +397,9 @@ impl PostgresEngine { let mut lsis = Vec::new(); let mut gsi_ids = Vec::new(); for (index_name, index_id, index_type, ks, proj, ipt) in rows { - let key_schema: Vec = serde_json::from_value(ks) + let key_schema: Vec = from_catalog_json(ks) .map_err(|e| StorageError::Internal(format!("index key schema: {e}")))?; - let projection: Projection = serde_json::from_value(proj) + let projection: Projection = from_catalog_json(proj) .map_err(|e| StorageError::Internal(format!("index projection: {e}")))?; if index_type == "LSI" { lsis.push(LsiInput { @@ -490,9 +491,9 @@ impl PostgresEngine { for (index_id, ks, proj) in index_rows { indexes.push(RestoreIndex { table: index_table_name(&index_id), - key_schema: serde_json::from_value(ks) + key_schema: from_catalog_json(ks) .map_err(|e| StorageError::Internal(format!("index key schema: {e}")))?, - projection: serde_json::from_value(proj) + projection: from_catalog_json(proj) .map_err(|e| StorageError::Internal(format!("index projection: {e}")))?, }); } @@ -1129,7 +1130,7 @@ impl BackupEngine for PostgresEngine { // uses, which CreateTable would refuse on the service; drop those. if omitted { let mut used: std::collections::HashSet = - serde_json::from_value::>(key_schema.clone()) + from_catalog_json::>(key_schema.clone()) .map_err(|e| StorageError::Internal(format!("key schema: {e}")))? .into_iter() .map(|k| k.attribute_name) @@ -1147,15 +1148,17 @@ impl BackupEngine for PostgresEngine { { used.insert(k.attribute_name.clone()); } - if let Some(defs) = attr_defs.as_array_mut() { - defs.retain(|d| { - d.get("AttributeName") - .and_then(serde_json::Value::as_str) - .is_some_and(|n| used.contains(n)) - }); - } + // Compare decoded names: the stored column is in escaped form. + let mut defs: Vec = + from_catalog_json(attr_defs).map_err(|e| { + StorageError::Internal(format!("attribute definitions: {e}")) + })?; + defs.retain(|d| used.contains(&d.attribute_name)); + attr_defs = to_catalog_json(&defs)?; } - let definition = definition.to_json()?; + // The definition carries attribute names, so it is stored in the + // same form as the catalog's key_schema and projection columns. + let definition = escape_json(definition.to_json()?); // All catalog-side writes are one transaction, so a crash cannot // leave a backup AVAILABLE with partial items. The items are read @@ -1310,7 +1313,7 @@ impl BackupEngine for PostgresEngine { ) = row; let key_schema: Vec = - serde_json::from_value(ks_json) + from_catalog_json(ks_json) .map_err(|e| StorageError::Internal(format!("Parse key schema: {e}")))?; Ok(BackupDescription { @@ -1562,7 +1565,7 @@ impl BackupEngine for PostgresEngine { }; let definition = definition - .map(|d| BackupTableDefinition::from_json(d, &backup_arn)) + .map(|d| BackupTableDefinition::from_json(unescape_json(d), &backup_arn)) .transpose()?; // A backup taken before catalog 0.0.4 has no definition; its // vector indexes are known only from the `backups.vector_indexes` @@ -1598,9 +1601,9 @@ impl BackupEngine for PostgresEngine { source_table_name, source_table_arn, source_table_id, - key_schema: serde_json::from_value(ks_json) + key_schema: from_catalog_json(ks_json) .map_err(|e| StorageError::Internal(format!("Parse key schema: {e}")))?, - attribute_definitions: serde_json::from_value(ad_json) + attribute_definitions: from_catalog_json(ad_json) .map_err(|e| StorageError::Internal(format!("Parse attr defs: {e}")))?, billing_mode, provisioned_throughput, diff --git a/crates/storage-postgres/src/bootstrapper.rs b/crates/storage-postgres/src/bootstrapper.rs index 5eea7b26d..6485e6371 100755 --- a/crates/storage-postgres/src/bootstrapper.rs +++ b/crates/storage-postgres/src/bootstrapper.rs @@ -25,6 +25,7 @@ use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use tokio::sync::{Mutex, OnceCell}; use crate::CATALOG_VERSION; +use crate::catalog_json::{from_catalog_json, to_catalog_json}; use crate::migrations; /// ExtendDB's advisory-lock namespace. PostgreSQL's two-argument form, @@ -110,6 +111,9 @@ impl PostgresBootstrapper { .username(&self.config.app_user) .password(&self.config.app_password) .database(database) + // Lets a migration tell its own connections apart from a server's + // in pg_stat_activity (see `catalog_escape::MIGRATE_APPLICATION_NAME`). + .application_name(crate::catalog_escape::MIGRATE_APPLICATION_NAME) } /// Build the connection URL for the application user and a named database. @@ -446,6 +450,18 @@ impl Bootstrapper for PostgresBootstrapper { Ok(pending) } + /// `004_escape_catalog_control_chars.sql` rewrites rows in the catalog, so + /// while it is pending the migrate command must be the only client of the + /// catalog. The same check runs again inside the migration; this one runs + /// before anything is written, so a refusal changes nothing. + async fn check_exclusive_access_for_pending_migrations(&self) -> OpResult<()> { + let catalog_pool = self.app_pool(&self.config.catalog_db).await?; + if !crate::catalog_escape::needs_exclusive_access(&catalog_pool).await? { + return Ok(()); + } + crate::catalog_escape::refuse_if_other_clients_connected(&catalog_pool, "catalog").await + } + async fn record_data_connection(&self) -> OpResult<()> { let pool = self.app_pool(&self.config.catalog_db).await?; let data_conn = self.app_connection_url(&self.config.data_db); @@ -627,7 +643,7 @@ impl Bootstrapper for PostgresBootstrapper { .map_err(|e| OpError::Internal(format!("Cannot read tables: {e}")))?; for (account_id, table_name, table_id, ks_json, ad_json) in rows { - let key_schema: Vec = match serde_json::from_value(ks_json) { + let key_schema: Vec = match from_catalog_json(ks_json) { Ok(v) => v, Err(e) => { report @@ -637,7 +653,7 @@ impl Bootstrapper for PostgresBootstrapper { } }; let attr_defs: Vec = - serde_json::from_value(ad_json).unwrap_or_default(); + from_catalog_json(ad_json).unwrap_or_default(); // Nothing to do unless a sort key has lost its definition. // Reported for every table on every run, not only when a sort key is @@ -716,7 +732,7 @@ impl Bootstrapper for PostgresBootstrapper { let mut merged = attr_defs; merged.extend(recovered.iter().cloned()); - let merged_json = serde_json::to_value(&merged) + let merged_json = to_catalog_json(&merged) .map_err(|e| OpError::Internal(format!("Cannot serialize definitions: {e}")))?; if apply { sqlx::query( diff --git a/crates/storage-postgres/src/catalog_escape.rs b/crates/storage-postgres/src/catalog_escape.rs new file mode 100644 index 000000000..ce3b8a083 --- /dev/null +++ b/crates/storage-postgres/src/catalog_escape.rs @@ -0,0 +1,316 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The one-shot re-encode of the catalog columns that carry attribute names, +//! run by the catalog migration `004_escape_catalog_control_chars.sql`. +//! +//! `catalog_json` stores U+0000 as U+0001 U+0001 and U+0001 as U+0001 U+0002 +//! in `tables.key_schema`, `tables.attribute_definitions`, +//! `indexes.key_schema`, `indexes.projection`, `backups.key_schema`, +//! `backups.attribute_definitions` and `backup_definitions.definition`, and +//! its decoder reads a U+0001 followed by U+0001 or U+0002 as such a pair. +//! Earlier releases stored a name containing U+0001 raw (`jsonb` accepts it), +//! so a pre-existing name `az` would read back as +//! `az` after the upgrade and the table's items would no longer be +//! reachable by key. This migration rewrites every stored document that +//! contains a raw U+0001 through the escape once, so that after it every +//! document in these columns is in the escaped form. A legacy document cannot +//! contain U+0000 (`jsonb` refused it), so applying the escape to the raw +//! stored value is exactly the re-encode, and a document without U+0001 +//! encodes to itself and is left alone. +//! +//! The rewrite is not idempotent at the row level: an escaped U+0001 is stored +//! as U+0001 U+0002, the same bytes as a legacy pair, so a row this release +//! wrote must never be rewritten. Three rules follow. Each table is rewritten +//! in one transaction that also inserts a marker (migration name, table name) +//! into the progress table, so a re-run after a crash skips the tables already +//! done; when every table is marked the runner records the migration file in +//! `schema_history` and later runs are no-ops. The migration refuses to run +//! while any other session is connected to the catalog, because a server of +//! this release writing during the rewrite would have its rows rewritten +//! again, and an older server writing after it would leave raw rows behind. +//! And the server refuses to start on a catalog that does not record the +//! migration: the version gate refuses the previous version, and the ledger +//! check refuses a catalog stamped with the current version by another +//! migration file of the same number. +//! +//! Composition with the data-side escape (the `item_data` columns and the +//! key text columns, data migration `004_escape_control_chars`): the alphabet +//! is the same and the tables are disjoint, so the two migrations can run in +//! either order. If that migration later takes these seven columns as more +//! entries of its own table list, a catalog that records this file must be +//! left out of that rewrite, because its documents are already escaped. + +use extenddb_storage::management_store::{OpError, OpResult}; +use sqlx::PgPool; + +use crate::catalog_json::escape_json; + +/// The catalog migration file this re-encode belongs to; the ledger name. +pub(crate) const ESCAPE_CATALOG_MIGRATION: &str = "004_escape_catalog_control_chars.sql"; + +/// `application_name` the init and migrate commands set on their connections, +/// so [`refuse_if_other_clients_connected`] can leave them out of its count. +pub(crate) const MIGRATE_APPLICATION_NAME: &str = "extenddb-migrate"; + +/// Environment variable that skips [`refuse_if_other_clients_connected`], for a +/// database whose only other connections are known to be idle (a connection +/// pooler holding server-side connections open, for example). +pub(crate) const IGNORE_CONNECTIONS_ENV: &str = "EXTENDDB_MIGRATE_IGNORE_CONNECTIONS"; + +/// The progress ledger: one row per (migration, table) rewritten and committed. +const PROGRESS_TABLE: &str = "data_code_migration_progress"; + +/// The catalog tables and `jsonb` columns the escape covers. +const CATALOG_WORK: &[(&str, &[&str])] = &[ + ("tables", &["key_schema", "attribute_definitions"]), + ("indexes", &["key_schema", "projection"]), + ("backups", &["key_schema", "attribute_definitions"]), + ("backup_definitions", &["definition"]), +]; + +/// Refuse to run a row-rewriting migration while anything else is connected +/// to the database `pool` reaches. `which` names that database in the refusal. +/// +/// The migrate command connects as the application role, which is not a +/// superuser. For sessions of other roles, `PostgreSQL` then shows `datname`, +/// `usename`, and `application_name` but hides `backend_type` and +/// `client_addr`, so a filter on `backend_type` alone misses an operator's +/// `psql` or a server running under another role. A session is counted when it +/// has a role (`usename`), which background workers such as autovacuum never +/// have, and its type is either `client backend` or hidden. Connections +/// carrying [`MIGRATE_APPLICATION_NAME`] are this process's own and are +/// ignored. Managed environments where a pooler keeps idle connections open +/// can set [`IGNORE_CONNECTIONS_ENV`] to skip the check. +pub(crate) async fn refuse_if_other_clients_connected(pool: &PgPool, which: &str) -> OpResult<()> { + let others: Vec<(String, Option, Option)> = sqlx::query_as( + "SELECT usename::text, application_name, client_addr::text \ + FROM pg_stat_activity \ + WHERE datname = current_database() \ + AND usename IS NOT NULL \ + AND coalesce(backend_type, 'client backend') = 'client backend' \ + AND pid <> pg_backend_pid() \ + AND coalesce(application_name, '') <> $1", + ) + .bind(MIGRATE_APPLICATION_NAME) + .fetch_all(pool) + .await + .map_err(|e| OpError::Internal(format!("Check for other connections: {e}")))?; + if others.is_empty() { + return Ok(()); + } + let mut described: Vec = others + .iter() + .map(|(user, app, addr)| { + format!( + "{user} from {} ({})", + addr.as_deref().unwrap_or("no client address shown"), + match app.as_deref() { + Some("") | None => "no application name", + Some(a) => a, + } + ) + }) + .collect(); + described.sort(); + described.dedup(); + Err(OpError::Internal(format!( + "{} other connection(s) hold the {which} database: {}. This migration rewrites \ + stored rows and must run with every ExtendDB server stopped and every other \ + session closed. Stop them and run 'extenddb migrate --yes' again. If the only \ + other connections are idle ones held by a connection pooler, set \ + {IGNORE_CONNECTIONS_ENV}=1 to skip this check.", + others.len(), + described.join("; ") + ))) +} + +/// True when [`ESCAPE_CATALOG_MIGRATION`] still has to run on this catalog and +/// the operator has not set [`IGNORE_CONNECTIONS_ENV`], so the migrate +/// command must hold the catalog alone before it writes anything. +pub(crate) async fn needs_exclusive_access(catalog_pool: &PgPool) -> OpResult { + if std::env::var_os(IGNORE_CONNECTIONS_ENV).is_some() { + return Ok(false); + } + Ok(!crate::migrations::is_migration_applied(catalog_pool, ESCAPE_CATALOG_MIGRATION).await?) +} + +async fn ensure_progress_table(pool: &PgPool) -> OpResult<()> { + sqlx::query(&format!( + "CREATE TABLE IF NOT EXISTS {PROGRESS_TABLE} (\ + migration_name TEXT NOT NULL, \ + table_name TEXT NOT NULL, \ + completed_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), \ + PRIMARY KEY (migration_name, table_name)\ + )" + )) + .execute(pool) + .await + .map_err(|e| OpError::Internal(format!("Create migration progress table: {e}")))?; + Ok(()) +} + +async fn is_table_marked(pool: &PgPool, table: &str) -> OpResult { + sqlx::query_scalar(&format!( + "SELECT EXISTS(SELECT 1 FROM {PROGRESS_TABLE} \ + WHERE migration_name = $1 AND table_name = $2)" + )) + .bind(ESCAPE_CATALOG_MIGRATION) + .bind(table) + .fetch_one(pool) + .await + .map_err(|e| OpError::Internal(format!("Check migration progress: {e}"))) +} + +/// The re-encoded form of a stored document, or `None` when the escape does +/// not change it (no raw U+0000 or U+0001 in any key or string). +fn reencoded(value: &serde_json::Value) -> Option { + let escaped = escape_json(value.clone()); + (escaped != *value).then_some(escaped) +} + +/// Re-encode the rows of one column whose stored document contains U+0001 or +/// U+0000, inside the caller's per-table transaction. The candidate ctids are +/// read once and locked before any row is touched, so no row is visited twice. +async fn escape_column( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + table: &str, + col: &str, +) -> OpResult { + // `jsonb::text` renders a raw U+0001 inside a string or key as the six + // characters backslash-u-0001 (U+0000 likewise, though no legacy row holds + // one). The backslash is spelled chr(92) so the predicate reads the same + // under either value of standard_conforming_strings. A stored string that + // contains that literal text also matches, re-encodes to itself and is + // skipped by `reencoded`. + let select = format!( + "SELECT ctid::text FROM \"{table}\" \ + WHERE position((chr(92) || 'u0001') IN \"{col}\"::text) > 0 \ + OR position((chr(92) || 'u0000') IN \"{col}\"::text) > 0 \ + FOR UPDATE" + ); + let ctids: Vec = sqlx::query_scalar(&select) + .fetch_all(&mut **tx) + .await + .map_err(|e| OpError::Internal(format!("Find rows to re-encode in {table}.{col}: {e}")))?; + let fetch = + format!("SELECT ctid::text, \"{col}\" FROM \"{table}\" WHERE ctid = ANY($1::tid[])"); + let update = format!("UPDATE \"{table}\" SET \"{col}\" = $1 WHERE ctid = $2::tid"); + let mut changed: u64 = 0; + for batch in ctids.chunks(500) { + let rows: Vec<(String, serde_json::Value)> = sqlx::query_as(&fetch) + .bind(batch.to_vec()) + .fetch_all(&mut **tx) + .await + .map_err(|e| OpError::Internal(format!("Fetch rows from {table}.{col}: {e}")))?; + for (ctid, value) in rows { + if let Some(escaped) = reencoded(&value) { + sqlx::query(&update) + .bind(escaped) + .bind(&ctid) + .execute(&mut **tx) + .await + .map_err(|e| { + OpError::Internal(format!("Re-encode a row of {table}.{col}: {e}")) + })?; + changed += 1; + } + } + } + Ok(changed) +} + +/// Rewrite one table's columns in one transaction and mark the table done in +/// the progress ledger before committing, so the rewrite and the marker are +/// atomic. Returns the number of column updates (one per column rewritten in +/// a row). +async fn escape_table(pool: &PgPool, table: &str, columns: &[&str]) -> OpResult { + let mut tx = pool + .begin() + .await + .map_err(|e| OpError::Internal(format!("Begin transaction for {table}: {e}")))?; + let mut rewritten = 0; + for col in columns { + rewritten += escape_column(&mut tx, table, col).await?; + } + sqlx::query(&format!( + "INSERT INTO {PROGRESS_TABLE} (migration_name, table_name) \ + VALUES ($1, $2) ON CONFLICT DO NOTHING" + )) + .bind(ESCAPE_CATALOG_MIGRATION) + .bind(table) + .execute(&mut *tx) + .await + .map_err(|e| OpError::Internal(format!("Mark {table} as re-encoded: {e}")))?; + tx.commit() + .await + .map_err(|e| OpError::Internal(format!("Commit re-encode of {table}: {e}")))?; + Ok(rewritten) +} + +/// Re-encode the legacy documents of every catalog column the escape covers +/// (see the module documentation). Refuses while another session holds the +/// catalog unless [`IGNORE_CONNECTIONS_ENV`] is set. Tables absent from this +/// catalog (a `backups` table a deployment never had) are skipped. +pub(crate) async fn escape_legacy_catalog_control_chars(pool: &PgPool) -> OpResult<()> { + if std::env::var_os(IGNORE_CONNECTIONS_ENV).is_none() { + refuse_if_other_clients_connected(pool, "catalog").await?; + } + ensure_progress_table(pool).await?; + for (table, columns) in CATALOG_WORK { + if !crate::migrations::table_exists(pool, table).await? { + continue; + } + if is_table_marked(pool, table).await? { + println!(" {table}: already re-encoded, skipping."); + continue; + } + let rewritten = escape_table(pool, table, columns).await?; + println!(" {table}: {rewritten} column update(s)"); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn reencoded_changes_exactly_the_documents_holding_raw_control_chars() { + assert_eq!( + reencoded(&json!([{"AttributeName": "pk", "KeyType": "HASH"}])), + None + ); + // The six-character text backslash-u-0001 is literal data, not U+0001. + assert_eq!(reencoded(&json!({"AttributeName": "a\\u0001z"})), None); + assert_eq!( + reencoded(&json!([{"AttributeName": "a\u{1}\u{1}z", "KeyType": "HASH"}])), + Some(json!([{"AttributeName": "a\u{1}\u{2}\u{1}\u{2}z", "KeyType": "HASH"}])) + ); + assert_eq!( + reencoded(&json!({"NonKeyAttributes": ["n\u{1}k"], "k\u{1}": 1})), + Some(json!({"NonKeyAttributes": ["n\u{1}\u{2}k"], "k\u{1}\u{2}": 1})) + ); + } + + #[test] + fn the_work_list_names_the_seven_columns_the_decoder_reads() { + let columns: Vec = CATALOG_WORK + .iter() + .flat_map(|(t, cs)| cs.iter().map(move |c| format!("{t}.{c}"))) + .collect(); + assert_eq!( + columns, + [ + "tables.key_schema", + "tables.attribute_definitions", + "indexes.key_schema", + "indexes.projection", + "backups.key_schema", + "backups.attribute_definitions", + "backup_definitions.definition", + ] + ); + } +} diff --git a/crates/storage-postgres/src/catalog_json.rs b/crates/storage-postgres/src/catalog_json.rs new file mode 100644 index 000000000..44f7d0d05 --- /dev/null +++ b/crates/storage-postgres/src/catalog_json.rs @@ -0,0 +1,235 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Serialization for the catalog's `jsonb` columns that carry attribute names: +//! `key_schema`, `attribute_definitions` and index `projection`. +//! +//! DynamoDB accepts U+0000 in an attribute name, including a key attribute +//! name, and `DescribeTable` returns it byte-identical. PostgreSQL `jsonb` +//! rejects the `\u0000` escape (`unsupported Unicode escape sequence`), so a +//! `CreateTable` whose key schema carried the character failed at the catalog +//! insert. Every write of these columns goes through [`to_catalog_json`], which +//! maps the two affected characters to sequences `jsonb` accepts, and every +//! read goes through [`from_catalog_json`], which reverses the mapping: +//! +//! | input | stored | +//! |--------|---------------| +//! | U+0000 | U+0001 U+0001 | +//! | U+0001 | U+0001 U+0002 | +//! +//! Every other character is stored as itself, so a name without either +//! character is stored exactly as before. The mapping is the one the item +//! data path uses for `item_data` columns, so one decoder reads both once the +//! two paths share a helper. +//! +//! Decoding is lenient about a U+0001 not followed by U+0001 or U+0002: it is +//! returned as a literal U+0001, which is how a name written before this +//! module existed reads back unchanged. No catalog migration accompanies this +//! module: a row written before it cannot contain U+0000 (the write failed), +//! and a stored name containing U+0001 immediately followed by U+0001 or +//! U+0002 is the one legacy shape this decoder would misread. + +use extenddb_storage::error::StorageError; +use serde::Serialize; +use serde::de::DeserializeOwned; +use serde_json::Value; + +const ESC: char = '\u{1}'; +const NUL_TAIL: char = '\u{1}'; +const ESC_TAIL: char = '\u{2}'; + +/// Serialize `value` for a catalog `jsonb` column that may carry attribute +/// names. +pub(crate) fn to_catalog_json(value: &T) -> Result { + serde_json::to_value(value) + .map(escape_json) + .map_err(|e| StorageError::Internal(e.to_string())) +} + +/// Deserialize a value written by [`to_catalog_json`]. +pub(crate) fn from_catalog_json(v: Value) -> Result { + serde_json::from_value(unescape_json(v)).map_err(|e| StorageError::Internal(e.to_string())) +} + +/// Reverse the escape on a stored JSON tree without deserializing it, for a +/// caller that reads fields of the tree directly. +pub(crate) fn unescape_json(v: Value) -> Value { + map_strings(v, &unescape_str) +} + +/// Apply the escape to a JSON tree that is already serialized, for a caller +/// that builds the stored document itself. +pub(crate) fn escape_json(v: Value) -> Value { + map_strings(v, &escape_str) +} + +fn needs_escape(s: &str) -> bool { + s.bytes().any(|b| b == 0 || b == 1) +} + +fn escape_str(s: &str) -> Option { + if !needs_escape(s) { + return None; + } + let mut out = String::with_capacity(s.len() + 8); + for ch in s.chars() { + match ch { + '\u{0}' => { + out.push(ESC); + out.push(NUL_TAIL); + } + '\u{1}' => { + out.push(ESC); + out.push(ESC_TAIL); + } + other => out.push(other), + } + } + Some(out) +} + +fn unescape_str(s: &str) -> Option { + if !s.bytes().any(|b| b == 1) { + return None; + } + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); + while let Some(ch) = chars.next() { + if ch == ESC { + match chars.peek() { + Some(&NUL_TAIL) => { + chars.next(); + out.push('\u{0}'); + } + Some(&ESC_TAIL) => { + chars.next(); + out.push('\u{1}'); + } + _ => out.push('\u{1}'), + } + } else { + out.push(ch); + } + } + Some(out) +} + +/// Apply `f` to every object key and string value in the tree. `f` returns +/// `None` when the string is unchanged, so an ordinary tree is rebuilt without +/// copying any string. +fn map_strings(v: Value, f: &dyn Fn(&str) -> Option) -> Value { + match v { + Value::String(s) => match f(&s) { + Some(t) => Value::String(t), + None => Value::String(s), + }, + Value::Array(items) => { + Value::Array(items.into_iter().map(|item| map_strings(item, f)).collect()) + } + Value::Object(map) => { + let mut out = serde_json::Map::with_capacity(map.len()); + for (k, val) in map { + let key = f(&k).unwrap_or(k); + out.insert(key, map_strings(val, f)); + } + Value::Object(out) + } + other => other, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use extenddb_core::types::{ + AttributeDefinition, KeySchemaElement, KeyType, ScalarAttributeType, + }; + use serde_json::json; + + fn key_schema(name: &str) -> Vec { + vec![KeySchemaElement { + attribute_name: name.to_owned(), + key_type: KeyType::Hash, + }] + } + + #[test] + fn plain_names_are_stored_as_before() { + let ks = key_schema("pk"); + let stored = to_catalog_json(&ks).unwrap(); + assert_eq!(stored, serde_json::to_value(&ks).unwrap()); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back, ks); + } + + #[test] + fn nul_in_key_attribute_name_round_trips_and_is_stored_without_nul() { + let ks = key_schema("p\u{0}k"); + let stored = to_catalog_json(&ks).unwrap(); + let text = stored.to_string(); + assert!( + !text.contains("\\u0000"), + "stored form carries no NUL: {text}" + ); + assert_eq!(stored[0]["AttributeName"], json!("p\u{1}\u{1}k")); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back, ks); + } + + #[test] + fn u0001_in_attribute_definition_round_trips() { + let ad = vec![AttributeDefinition { + attribute_name: "a\u{1}b\u{1}\u{1}c\u{0}".to_owned(), + attribute_type: ScalarAttributeType::S, + }]; + let stored = to_catalog_json(&ad).unwrap(); + assert_eq!( + stored[0]["AttributeName"], + json!("a\u{1}\u{2}b\u{1}\u{2}\u{1}\u{2}c\u{1}\u{1}") + ); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back, ad); + } + + #[test] + fn legacy_u0001_without_tail_reads_back_unchanged() { + // A name written before the escape existed: a lone U+0001 is a literal. + let legacy = json!([{"AttributeName": "a\u{1}z", "KeyType": "HASH"}]); + let back: Vec = from_catalog_json(legacy).unwrap(); + assert_eq!(back[0].attribute_name, "a\u{1}z"); + } + + #[test] + fn object_keys_and_nested_values_are_escaped() { + let v = json!({"k\u{0}": {"NonKeyAttributes": ["n\u{0}", "plain"]}, "n": 1}); + let stored = escape_json(v.clone()); + assert_eq!( + stored, + json!({"k\u{1}\u{1}": {"NonKeyAttributes": ["n\u{1}\u{1}", "plain"]}, "n": 1}) + ); + assert_eq!(unescape_json(stored), v); + } + + #[test] + fn exhaustive_round_trip_over_the_affected_alphabet() { + let alphabet = ['\u{0}', '\u{1}', '\u{2}', 'a']; + let mut frontier = vec![String::new()]; + for _ in 0..5 { + let mut next = Vec::new(); + for s in &frontier { + for ch in alphabet { + let mut t = s.clone(); + t.push(ch); + next.push(t); + } + } + for s in &next { + let stored = escape_str(s).unwrap_or_else(|| s.clone()); + assert!(!stored.contains('\u{0}'), "{s:?}"); + let back = unescape_str(&stored).unwrap_or_else(|| stored.clone()); + assert_eq!(&back, s, "round trip of {s:?} via {stored:?}"); + } + frontier = next; + } + } +} diff --git a/crates/storage-postgres/src/create_table.rs b/crates/storage-postgres/src/create_table.rs index fc41c8c0b..84a4e7298 100755 --- a/crates/storage-postgres/src/create_table.rs +++ b/crates/storage-postgres/src/create_table.rs @@ -11,6 +11,7 @@ use extenddb_storage::error::StorageError; use extenddb_storage::util::{index_arn, stream_arn, table_arn}; use crate::PostgresEngine; +use crate::catalog_json::to_catalog_json; /// What a restore target records about where it came from, written in the /// same catalog transaction that creates it. @@ -110,10 +111,8 @@ impl PostgresEngine { let table_id = uuid::Uuid::new_v4().to_string(); let table_arn = table_arn(&self.region, account_id, &input.table_name); let billing_mode = input.billing_mode.unwrap_or(BillingMode::Provisioned); - let key_schema_json = serde_json::to_value(&input.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let attr_defs_json = serde_json::to_value(&input.attribute_definitions) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema_json = to_catalog_json(&input.key_schema)?; + let attr_defs_json = to_catalog_json(&input.attribute_definitions)?; let billing_str = match billing_mode { BillingMode::Provisioned => "PROVISIONED", BillingMode::PayPerRequest => "PAY_PER_REQUEST", @@ -262,10 +261,8 @@ impl PostgresEngine { let mut gsi_index_ids: Vec = Vec::new(); if let Some(gsis) = &input.global_secondary_indexes { for gsi in gsis { - let gsi_ks = serde_json::to_value(&gsi.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let gsi_proj = serde_json::to_value(&gsi.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let gsi_ks = to_catalog_json(&gsi.key_schema)?; + let gsi_proj = to_catalog_json(&gsi.projection)?; let gsi_pt = gsi .provisioned_throughput .as_ref() @@ -305,10 +302,8 @@ impl PostgresEngine { let mut lsi_index_ids: Vec = Vec::new(); if let Some(lsis) = &input.local_secondary_indexes { for lsi in lsis { - let lsi_ks = serde_json::to_value(&lsi.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let lsi_proj = serde_json::to_value(&lsi.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let lsi_ks = to_catalog_json(&lsi.key_schema)?; + let lsi_proj = to_catalog_json(&lsi.projection)?; let index_id = uuid::Uuid::new_v4().to_string(); sqlx::query( diff --git a/crates/storage-postgres/src/data/ddl.rs b/crates/storage-postgres/src/data/ddl.rs index f608c89e8..cb6196837 100755 --- a/crates/storage-postgres/src/data/ddl.rs +++ b/crates/storage-postgres/src/data/ddl.rs @@ -3,6 +3,7 @@ //! DDL helpers for creating and dropping per-DynamoDB-table data tables in `PostgreSQL`. +use crate::catalog_json::from_catalog_json; use extenddb_core::types::{ AttributeDefinition, IndexInfo, IndexType, KeySchemaElement, Projection, StreamSpecification, TableKeyInfo, @@ -431,10 +432,8 @@ impl PostgresEngine { _ => return Err(StorageError::TableNotActive(table_name.to_owned())), } - let key_schema: Vec = - serde_json::from_value(ks_json).map_err(|e| StorageError::Internal(e.to_string()))?; - let attribute_definitions: Vec = - serde_json::from_value(ad_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let attribute_definitions: Vec = from_catalog_json(ad_json)?; let stream_specification: Option = stream_spec_json .map(serde_json::from_value) @@ -531,10 +530,8 @@ impl PostgresEngine { ))); } }; - let key_schema: Vec = serde_json::from_value(ks_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let projection: Projection = serde_json::from_value(proj_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let projection: Projection = from_catalog_json(proj_json)?; let info = IndexInfo { index_name, index_id, @@ -627,10 +624,8 @@ impl PostgresEngine { } }; - let key_schema: Vec = - serde_json::from_value(ks_json).map_err(|e| StorageError::Internal(e.to_string()))?; - let projection: Projection = - serde_json::from_value(proj_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let projection: Projection = from_catalog_json(proj_json)?; Ok(IndexInfo { index_name: index_name.to_owned(), diff --git a/crates/storage-postgres/src/data/gsi_build.rs b/crates/storage-postgres/src/data/gsi_build.rs index 22ee5bf6f..27965f5c2 100644 --- a/crates/storage-postgres/src/data/gsi_build.rs +++ b/crates/storage-postgres/src/data/gsi_build.rs @@ -66,6 +66,7 @@ use super::index::{ project_item_for_index, }; use super::{all_sort_key_info, data_table_name, index_table_name, json_to_item}; +use crate::catalog_json::from_catalog_json; use crate::gsi_queue::{GsiApplyContext, GsiIndexDef, PendingApplyContext, enqueue_gsi_pending}; /// How long a write may take, from just before its catalog read to just before @@ -1235,12 +1236,12 @@ async fn reconcile_pass( continue; } } - let parse = || -> Result<_, serde_json::Error> { + let parse = || -> Result<_, StorageError> { Ok(( - serde_json::from_value::>(idx_ks.clone())?, - serde_json::from_value::(idx_proj.clone())?, - serde_json::from_value::>(base_ks.clone())?, - serde_json::from_value::>(attr_defs.clone())?, + from_catalog_json::>(idx_ks.clone())?, + from_catalog_json::(idx_proj.clone())?, + from_catalog_json::>(base_ks.clone())?, + from_catalog_json::>(attr_defs.clone())?, )) }; let (key_schema, projection, base_ks, attr_defs) = match parse() { diff --git a/crates/storage-postgres/src/data/index.rs b/crates/storage-postgres/src/data/index.rs index 09c5627d7..072d921bc 100644 --- a/crates/storage-postgres/src/data/index.rs +++ b/crates/storage-postgres/src/data/index.rs @@ -7,6 +7,7 @@ //! index updates within transactions, and async index enqueue for deferred //! propagation. +use crate::catalog_json::from_catalog_json; use extenddb_core::types::{ AttributeDefinition, Item, KeySchemaElement, Projection, ProjectionType, ScalarAttributeType, TableKeyInfo, @@ -137,16 +138,13 @@ where // Every row carries the same table definitions; parse them once. let attribute_definitions: Vec = match rows.first() { - Some(row) => serde_json::from_value(row.6.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?, + Some(row) => from_catalog_json(row.6.clone())?, None => Vec::new(), }; rows.into_iter() .map(|(id, index_name, idx_type, ks_json, proj_json, delay, _)| { - let key_schema: Vec = serde_json::from_value(ks_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let projection: Projection = serde_json::from_value(proj_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let projection: Projection = from_catalog_json(proj_json)?; Ok(IndexMeta { index_id: id, index_name, diff --git a/crates/storage-postgres/src/data/vector_index.rs b/crates/storage-postgres/src/data/vector_index.rs index 4a8c70bef..5577dc0a0 100644 --- a/crates/storage-postgres/src/data/vector_index.rs +++ b/crates/storage-postgres/src/data/vector_index.rs @@ -42,6 +42,8 @@ use pgvector::Vector; use super::{all_sort_key_info, vector_table_name}; +use crate::catalog_json::from_catalog_json; + /// Read the vector index metadata for a table from the catalog. /// /// Every field the write path needs, including the index id that names the data @@ -1093,10 +1095,8 @@ pub async fn rebuild_stuck_vector_indexes( ); continue; }; - let base_key_schema: Vec = - serde_json::from_value(ks_json).map_err(|e| StorageError::Internal(e.to_string()))?; - let attribute_definitions: Vec = - serde_json::from_value(ad_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let base_key_schema: Vec = from_catalog_json(ks_json)?; + let attribute_definitions: Vec = from_catalog_json(ad_json)?; let dimensions = u32::try_from(dimensions) .map_err(|_| StorageError::Internal("vector dimensions out of range".to_owned()))?; diff --git a/crates/storage-postgres/src/lib.rs b/crates/storage-postgres/src/lib.rs index e9f1ae2e9..c742f44de 100755 --- a/crates/storage-postgres/src/lib.rs +++ b/crates/storage-postgres/src/lib.rs @@ -12,6 +12,8 @@ mod admin_store; mod authorization_store; mod backup_engine; mod bootstrapper; +mod catalog_escape; +mod catalog_json; mod catalog_store; pub mod config; mod create_table; @@ -158,7 +160,13 @@ use sqlx::postgres::PgPoolOptions; /// /// The tuple is the single source of truth. Use `CATALOG_VERSION.to_string()` /// wherever a string representation is needed. -pub const CATALOG_VERSION: CatalogVersion = CatalogVersion::new(0, 0, 4); +/// +/// 0.0.5 changed no schema: it marks the stored form of the catalog columns +/// that carry attribute names (`catalog_json`), introduced with catalog +/// migration `004_escape_catalog_control_chars.sql`, so that a binary from an +/// earlier release refuses a migrated catalog instead of reading escaped names +/// as literal data. +pub const CATALOG_VERSION: CatalogVersion = CatalogVersion::new(0, 0, 5); /// Minimum number of connections allowed per pool. /// diff --git a/crates/storage-postgres/src/migrations.rs b/crates/storage-postgres/src/migrations.rs index 2873bbc77..1d85ab715 100755 --- a/crates/storage-postgres/src/migrations.rs +++ b/crates/storage-postgres/src/migrations.rs @@ -20,6 +20,10 @@ pub(crate) const CATALOG_MIGRATIONS: &[(&str, &str)] = &[ "003_backup_definitions.sql", include_str!("../../storage-postgres/migrations/003_backup_definitions.sql"), ), + ( + crate::catalog_escape::ESCAPE_CATALOG_MIGRATION, + include_str!("../../storage-postgres/migrations/004_escape_catalog_control_chars.sql"), + ), ]; /// The runner's convergence write, executed after the walk over @@ -37,6 +41,12 @@ pub(crate) async fn run_catalog_migrations(pool: &PgPool) -> OpResult<()> { continue; } println!(" Applying {filename}..."); + // 004 moves the version for a change of stored form; the rows are + // re-encoded first, each table in its own committed transaction with a + // progress marker, so a crash before the version write resumes here. + if *filename == crate::catalog_escape::ESCAPE_CATALOG_MIGRATION { + crate::catalog_escape::escape_legacy_catalog_control_chars(pool).await?; + } sqlx::raw_sql(sql) .execute(pool) .await @@ -382,6 +392,7 @@ pub(crate) async fn run_data_code_migrations( /// created before the index existed. `CREATE INDEX CONCURRENTLY IF NOT EXISTS` /// is idempotent and does not block concurrent writes. async fn ensure_gsi_base_key_indexes(catalog_pool: &PgPool, data_pool: &PgPool) -> OpResult<()> { + use crate::catalog_json::from_catalog_json; use extenddb_core::types::{AttributeDefinition, KeySchemaElement}; use extenddb_storage::util::{sk_column, sk_column_n}; @@ -397,9 +408,9 @@ async fn ensure_gsi_base_key_indexes(catalog_pool: &PgPool, data_pool: &PgPool) for (index_id, ks_json, ad_json) in rows { let base_ks: Vec = - serde_json::from_value(ks_json).map_err(|e| OpError::Internal(e.to_string()))?; + from_catalog_json(ks_json).map_err(|e| OpError::Internal(e.to_string()))?; let attr_defs: Vec = - serde_json::from_value(ad_json).map_err(|e| OpError::Internal(e.to_string()))?; + from_catalog_json(ad_json).map_err(|e| OpError::Internal(e.to_string()))?; let base_sks = crate::data::all_sort_key_info(&base_ks, &attr_defs); let idx_table = crate::data::index_table_name(&index_id); @@ -477,7 +488,7 @@ pub(crate) async fn pending_data_migrations(pool: &PgPool) -> OpResult OpResult { +pub(crate) async fn is_migration_applied(pool: &PgPool, filename: &str) -> OpResult { if table_exists(pool, "schema_history").await? { let applied: (bool,) = sqlx::query_as("SELECT EXISTS(SELECT 1 FROM schema_history WHERE filename = $1)") @@ -522,10 +533,10 @@ mod tests { fn the_migration_count_and_the_catalog_version_agree() { assert_eq!( CATALOG_MIGRATIONS.len(), - 3, + 4, "a catalog migration was added or removed; update CATALOG_VERSION and this count" ); - assert_eq!(CATALOG_VERSION.to_string(), "0.0.4"); + assert_eq!(CATALOG_VERSION.to_string(), "0.0.5"); } /// The version the binary expects must be the version the schema writes. diff --git a/crates/storage-postgres/src/stream_engine.rs b/crates/storage-postgres/src/stream_engine.rs index 6eee09658..3baf60992 100755 --- a/crates/storage-postgres/src/stream_engine.rs +++ b/crates/storage-postgres/src/stream_engine.rs @@ -3,6 +3,7 @@ //! `StreamEngine` trait implementation for `PostgresEngine`. +use crate::catalog_json::from_catalog_json; use extenddb_core::types::{ SequenceNumberRange, Shard, StreamDescription, StreamRecord, StreamStatus, StreamSummary, StreamViewType, @@ -308,8 +309,7 @@ impl StreamEngine for PostgresEngine { )) })?; - let key_schema = serde_json::from_value(ks_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema = from_catalog_json(ks_json)?; let stream_view_type = stream_spec_json .and_then(|v| { diff --git a/crates/storage-postgres/src/table_helpers.rs b/crates/storage-postgres/src/table_helpers.rs index 4dfb7f18f..41727cb1d 100755 --- a/crates/storage-postgres/src/table_helpers.rs +++ b/crates/storage-postgres/src/table_helpers.rs @@ -12,6 +12,7 @@ use extenddb_storage::util::{index_arn, stream_arn}; use extenddb_storage::vector_catalog::{VectorIndexCatalogRow, vector_index_descriptions}; use crate::PostgresEngine; +use crate::catalog_json::from_catalog_json; use crate::data; /// Row type for table metadata queries. @@ -165,10 +166,8 @@ impl PostgresEngine { let mut lsis: Vec = Vec::new(); for idx in index_rows { - let ks = serde_json::from_value(idx.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let proj = serde_json::from_value(idx.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let ks = from_catalog_json(idx.key_schema)?; + let proj = from_catalog_json(idx.projection)?; if idx.index_type == "GSI" { // F-1 fallback: old data may be stored as ProvisionedThroughput @@ -231,10 +230,8 @@ impl PostgresEngine { } } - let key_schema = serde_json::from_value(row.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let attr_defs = serde_json::from_value(row.attribute_definitions) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema = from_catalog_json(row.key_schema)?; + let attr_defs = from_catalog_json(row.attribute_definitions)?; let stream_spec = row .stream_specification .map(serde_json::from_value) diff --git a/crates/storage-postgres/src/update_table.rs b/crates/storage-postgres/src/update_table.rs index dec99a93b..9cf615d3c 100755 --- a/crates/storage-postgres/src/update_table.rs +++ b/crates/storage-postgres/src/update_table.rs @@ -10,6 +10,7 @@ use extenddb_storage::error::StorageError; use extenddb_storage::util::{building_refuses, effective_attribute_definitions}; use crate::PostgresEngine; +use crate::catalog_json::{from_catalog_json, to_catalog_json}; use crate::data::gsi_build::{ GsiBuild, build_gsis, drop_gsi_tables, is_transient, remove_failed_build, }; @@ -454,10 +455,8 @@ impl PostgresEngine { // `ensure_index_name_free`. ensure_index_name_free(&mut tx, &table_id, &create.index_name).await?; - let gsi_ks = serde_json::to_value(&create.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let gsi_proj = serde_json::to_value(&create.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let gsi_ks = to_catalog_json(&create.key_schema)?; + let gsi_proj = to_catalog_json(&create.projection)?; let gsi_pt = create .provisioned_throughput .as_ref() @@ -528,12 +527,8 @@ impl PostgresEngine { // transaction, so `indexes` already holds exactly the surviving set; // the stored definitions were read under the FOR UPDATE lock, so the // read-modify-write is atomic against a concurrent UpdateTable. - let stored_attr_defs: Vec = - serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let table_key_schema: Vec = - serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let stored_attr_defs: Vec = from_catalog_json(ad_json.clone())?; + let table_key_schema: Vec = from_catalog_json(ks_json.clone())?; let surviving_rows: Vec<(serde_json::Value,)> = sqlx::query_as("SELECT key_schema FROM indexes WHERE table_id = $1") @@ -544,10 +539,7 @@ impl PostgresEngine { let mut surviving_index_key_schemas: Vec> = Vec::with_capacity(surviving_rows.len()); for (ks_value,) in surviving_rows { - surviving_index_key_schemas.push( - serde_json::from_value(ks_value) - .map_err(|e| StorageError::Internal(e.to_string()))?, - ); + surviving_index_key_schemas.push(from_catalog_json(ks_value)?); } let effective = effective_attribute_definitions( @@ -558,8 +550,7 @@ impl PostgresEngine { ); if effective != stored_attr_defs { - let effective_json = serde_json::to_value(&effective) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let effective_json = to_catalog_json(&effective)?; sqlx::query( "UPDATE tables SET attribute_definitions = $1 \ WHERE account_id = $2 AND table_name = $3", @@ -645,11 +636,9 @@ impl PostgresEngine { // naming both shapes, with the vector as type L and its // dimension count. let base_key_schema: Vec = - serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + from_catalog_json(ks_json.clone())?; let stored_attr_defs: Vec = - serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + from_catalog_json(ad_json.clone())?; let vec_attr_name = &create.vector_attribute.attribute_name; if let Some(ks) = base_key_schema .iter() @@ -792,10 +781,8 @@ impl PostgresEngine { // recovery sweep never sees a CREATING row with nobody holding it. It is an // efficiency measure only: a second, concurrent build is correct, because a // full backfill pass is enough on its own to publish the index. - let base_key_schema: Vec = serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let base_attr_defs: Vec = serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let base_key_schema: Vec = from_catalog_json(ks_json.clone())?; + let base_attr_defs: Vec = from_catalog_json(ad_json.clone())?; // Built from the merged and pruned set rather than the request's subset: a // new index may key on an attribute the base table already defined, and on // a conflicting redeclaration the stored type is the one that counts. @@ -926,10 +913,8 @@ impl PostgresEngine { // leaves a CREATING row for the reconciler rather than an ACTIVE index over // a table that was never populated. for (index_id, create) in &created_vector_indexes { - let base_key_schema: Vec = serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let base_attr_defs: Vec = serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let base_key_schema: Vec = from_catalog_json(ks_json.clone())?; + let base_attr_defs: Vec = from_catalog_json(ad_json.clone())?; let started = self .start_vector_index_build( diff --git a/crates/storage-postgres/tests/catalog_converge.rs b/crates/storage-postgres/tests/catalog_converge.rs index feb761aef..4a7235e63 100644 --- a/crates/storage-postgres/tests/catalog_converge.rs +++ b/crates/storage-postgres/tests/catalog_converge.rs @@ -8,9 +8,9 @@ //! (`PostgresEngine::prepare_catalog_for_serve`), waiting at most a few //! seconds for the table lock, and keeps the default; `extenddb migrate` //! reports and applies both steps. -//! - A catalog stamped 0.0.4 by a different `003_*` file: `serve` refuses it -//! naming this binary's 003; `extenddb migrate` reports that file as -//! pending and applies it. +//! - A catalog stamped with the current version by a different `003_*` file: +//! `serve` refuses it naming this binary's 003; `extenddb migrate` reports +//! that file as pending and applies it. //! //! Requires `EXTENDDB_TEST_PG_CONNECTION_STRING` (a base URL //! `postgres://user:pass@host:port`, no database); without it every test @@ -108,8 +108,9 @@ impl Drop for DbGuard { impl Scratch { /// A catalog with 001 and 002 applied and recorded, then `third` - /// applied and recorded under `third_name`; and a data database with - /// every data migration applied and recorded. + /// applied and recorded under `third_name`, then 004 applied and + /// recorded (so the stored version is the current one); and a data + /// database with every data migration applied and recorded. async fn new(third_name: &str, third: &str) -> Self { let base = base_conn().expect("caller checks base_conn() first"); let stem = format!("eddb_conv_{}", uuid::Uuid::new_v4().simple())[..24].to_owned(); @@ -144,6 +145,10 @@ impl Scratch { include_str!("../migrations/002_vector_indexes.sql"), ), (third_name, third), + ( + "004_escape_catalog_control_chars.sql", + include_str!("../migrations/004_escape_catalog_control_chars.sql"), + ), ] { sqlx::raw_sql(sql).execute(&catalog).await.expect(file); sqlx::query("INSERT INTO schema_history (filename) VALUES ($1)") @@ -305,7 +310,7 @@ async fn an_earlier_0_0_4_catalog_converges_at_serve_and_describe_table_works() return; } let s = Scratch::new("003_backup_definitions.sql", EARLIER_003).await; - assert_eq!(s.catalog_version().await, "0.0.4"); + assert_eq!(s.catalog_version().await, "0.0.5"); assert_eq!(s.column_default("source_table_arn").await, None); assert!( matches!(s.column_default("restore_date_time").await, Some(Some(_))), @@ -510,7 +515,8 @@ async fn migrate_converges_an_earlier_0_0_4_catalog() { } /// A catalog stamped 0.0.4 by another branch's `003_*` (recorded under its -/// own filename). The version matches, but this binary's 003 is unrecorded: +/// own filename) and then brought to 0.0.5 by 004. The version matches, but +/// this binary's 003 is unrecorded: /// `serve` refuses to start, naming the file and `extenddb migrate --yes`, /// and changes nothing; `migrate` reports it and applies it; afterwards /// nothing is pending, serve starts, and DescribeTable works. @@ -525,7 +531,7 @@ async fn an_unrecorded_catalog_migration_is_pending_even_at_the_current_version( "UPDATE settings SET value = '0.0.4' WHERE key = 'catalog_version';", ) .await; - assert_eq!(s.catalog_version().await, "0.0.4"); + assert_eq!(s.catalog_version().await, "0.0.5"); let b = s.bootstrapper(); assert_eq!( b.read_catalog_version().await.expect("version").as_deref(), @@ -596,6 +602,12 @@ async fn serve_refuses_an_unreadable_catalog_version_with_the_shared_refusal() { "UPDATE settings SET value = 'garbage' WHERE key = 'catalog_version';", ) .await; + // 004 wrote the current version after the tampered 003; tamper again so the + // stored version is the unreadable one. + sqlx::query("UPDATE settings SET value = 'garbage' WHERE key = 'catalog_version'") + .execute(&s.catalog) + .await + .expect("tamper"); assert_eq!(s.catalog_version().await, "garbage"); let expected = extenddb_storage::util::check_stored_catalog_version( Some("garbage"), diff --git a/crates/storage-postgres/tests/catalog_escape_migration.rs b/crates/storage-postgres/tests/catalog_escape_migration.rs new file mode 100644 index 000000000..8aaac708b --- /dev/null +++ b/crates/storage-postgres/tests/catalog_escape_migration.rs @@ -0,0 +1,550 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 +//! The catalog migration `004_escape_catalog_control_chars.sql` against a +//! catalog an earlier release wrote. +//! +//! Earlier releases stored attribute names raw in the catalog's `jsonb` +//! columns; this release stores them through the `catalog_json` escape, whose +//! decoder reads U+0001 followed by U+0001 or U+0002 as an escape pair. The +//! migration re-encodes every stored document holding a raw U+0001 once, so a +//! legacy name `az` still reads back as itself. Pinned here: +//! +//! - a 0.0.4 catalog holding such names in all seven columns is refused by +//! `serve` (version gate), migrated by `migrate --yes`, and then returns the +//! names byte-identical through `DescribeTable` and `DescribeBackup`; +//! - a second `migrate --yes` changes nothing; +//! - a catalog stamped 0.0.5 by hand without the record is refused by `serve` +//! naming the file; +//! - the migration refuses to run while another session holds the catalog. +//! +//! Requires `EXTENDDB_TEST_PG_CONNECTION_STRING` (a base URL +//! `postgres://user:pass@host:port`, no database); without it every test +//! reports a skip and passes. Each test creates and drops its own databases. + +use extenddb_core::types::{ + AttributeDefinition, BillingMode, CreateTableInput, DescribeTableInput, GsiInput, + KeySchemaElement, KeyType, Projection, ProjectionType, ScalarAttributeType, +}; +use extenddb_storage::bootstrapper::{BootstrapConfig, Bootstrapper}; +use extenddb_storage::server_components::BackendError; +use extenddb_storage::{BackupEngine, TableEngine}; +use extenddb_storage_postgres::{PostgresBootstrapper, PostgresConfig, PostgresEngine}; +use sqlx::PgPool; +use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; + +const ACCOUNT: &str = "123456789012"; +/// A name an earlier release could store raw and this release's decoder +/// would misread without the migration. +const LEGACY: &str = "a\u{1}\u{1}z"; +const LEGACY_GSI_KEY: &str = "g\u{1}\u{2}k"; +const LEGACY_PROJECTED: &str = "n\u{1}\u{1}"; + +fn base_conn() -> Option { + let conn = std::env::var("EXTENDDB_TEST_PG_CONNECTION_STRING").ok()?; + (!conn.trim().is_empty()).then(|| conn.trim_end_matches('/').to_owned()) +} + +fn bootstrap_config(base: &str, catalog: &str, data: &str) -> BootstrapConfig { + let rest = base + .strip_prefix("postgres://") + .or_else(|| base.strip_prefix("postgresql://")) + .expect("a postgres:// URL"); + let (creds, addr) = rest.split_once('@').expect("user:pass@host:port"); + let (user, pass) = creds.split_once(':').expect("user:pass"); + let (host, port) = addr.split_once(':').unwrap_or((addr, "5432")); + BootstrapConfig { + host: host.to_owned(), + port: port.parse().expect("port"), + admin_user: user.to_owned(), + admin_password: Some(pass.to_owned()), + app_user: user.to_owned(), + app_password: pass.to_owned(), + catalog_db: catalog.to_owned(), + data_db: data.to_owned(), + } +} + +/// Drops the scratch databases on every exit, a panic included. +struct Scratch { + base: String, + names: [String; 2], + catalog: Option, +} + +impl Drop for Scratch { + fn drop(&mut self) { + let base = self.base.clone(); + let names = self.names.clone(); + let pool = self.catalog.take(); + let _ = std::thread::spawn(move || { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("runtime"); + rt.block_on(async move { + if let Some(p) = pool { + p.close().await; + } + if let Ok(admin) = PgPoolOptions::new() + .max_connections(1) + .connect(&format!("{base}/postgres")) + .await + { + for n in &names { + let _ = + sqlx::query(&format!("DROP DATABASE IF EXISTS \"{n}\" WITH (FORCE)")) + .execute(&admin) + .await; + } + admin.close().await; + } + }); + }) + .join(); + } +} + +fn s_attr(name: &str) -> AttributeDefinition { + AttributeDefinition { + attribute_name: name.to_owned(), + attribute_type: ScalarAttributeType::S, + } +} + +fn key(name: &str, key_type: KeyType) -> KeySchemaElement { + KeySchemaElement { + attribute_name: name.to_owned(), + key_type, + } +} + +impl Scratch { + fn pool(&self) -> &PgPool { + self.catalog.as_ref().expect("catalog pool") + } + + /// A 0.0.4 catalog and data database the way the previous release's + /// `init` left them (001 to 003 applied and recorded, every data migration + /// recorded), holding one table with a GSI and one backup of it, with the + /// legacy names written raw into all seven columns the way that release + /// wrote them. + async fn legacy() -> Self { + let base = base_conn().expect("caller checks base_conn() first"); + let stem = format!("eddb_esc_{}", uuid::Uuid::new_v4().simple())[..24].to_owned(); + let names = [format!("{stem}_c"), format!("{stem}_d")]; + let admin = PgPoolOptions::new() + .max_connections(1) + .connect(&format!("{base}/postgres")) + .await + .expect("admin"); + for n in &names { + sqlx::query(&format!("CREATE DATABASE \"{n}\"")) + .execute(&admin) + .await + .expect("create scratch database"); + } + admin.close().await; + // The scratch pool carries the migrate command's application name so + // the exclusive-access check does not count it; a real operator session + // is simulated separately with a plain connection. + let opts: PgConnectOptions = format!("{base}/{}", names[0]).parse().expect("catalog url"); + let catalog = PgPoolOptions::new() + .max_connections(2) + .connect_with(opts.application_name("extenddb-migrate")) + .await + .expect("catalog"); + let s = Self { + base, + names, + catalog: Some(catalog), + }; + for (file, sql) in [ + ( + "001_schema.sql", + include_str!("../migrations/001_schema.sql"), + ), + ( + "002_vector_indexes.sql", + include_str!("../migrations/002_vector_indexes.sql"), + ), + ( + "003_backup_definitions.sql", + include_str!("../migrations/003_backup_definitions.sql"), + ), + ] { + sqlx::raw_sql(sql).execute(s.pool()).await.expect(file); + sqlx::query("INSERT INTO schema_history (filename) VALUES ($1)") + .bind(file) + .execute(s.pool()) + .await + .expect("record"); + } + let data = PgPoolOptions::new() + .max_connections(1) + .connect(&format!("{}/{}", s.base, s.names[1])) + .await + .expect("data"); + sqlx::query( + "CREATE TABLE schema_history (filename TEXT PRIMARY KEY, \ + applied_at TIMESTAMPTZ NOT NULL DEFAULT NOW())", + ) + .execute(&data) + .await + .expect("data ledger"); + for (file, sql) in [ + ( + "001_data_schema.sql", + include_str!("../data_migrations/001_data_schema.sql"), + ), + ( + "002_gsi_pending.sql", + include_str!("../data_migrations/002_gsi_pending.sql"), + ), + ( + "003_idempotency_account_scope.sql", + include_str!("../data_migrations/003_idempotency_account_scope.sql"), + ), + ( + "004_vector_index_state.sql", + include_str!("../data_migrations/004_vector_index_state.sql"), + ), + ("003_gsi_base_key_index", ""), + ] { + if !sql.is_empty() { + sqlx::raw_sql(sql).execute(&data).await.expect(file); + } + sqlx::query("INSERT INTO schema_history (filename) VALUES ($1)") + .bind(file) + .execute(&data) + .await + .expect("record"); + } + data.close().await; + sqlx::query( + "INSERT INTO settings (key, value) VALUES ('data_database_connection_string', $1) \ + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value", + ) + .bind(format!("{}/{}", s.base, s.names[1])) + .execute(s.pool()) + .await + .expect("point the catalog at the data database"); + sqlx::query("UPDATE settings SET value = '0' WHERE key = 'control_plane_delay_seconds'") + .execute(s.pool()) + .await + .expect("no control-plane delay"); + sqlx::query("INSERT INTO accounts (account_id, account_name) VALUES ($1, $2)") + .bind(ACCOUNT) + .bind(format!("acct-{stem}")) + .execute(s.pool()) + .await + .expect("account"); + + // The table and backup rows come from this release's engine (the + // shapes are the same); the seven columns are then set by SQL to the + // raw documents the previous release wrote. + let engine = s.engine().await; + engine + .create_table( + ACCOUNT, + CreateTableInput { + table_name: "legacy".to_owned(), + key_schema: vec![key("pk", KeyType::Hash)], + attribute_definitions: vec![s_attr("pk"), s_attr("gk")], + billing_mode: Some(BillingMode::PayPerRequest), + global_secondary_indexes: Some(vec![GsiInput { + index_name: "gsi1".to_owned(), + key_schema: vec![key("gk", KeyType::Hash)], + projection: Projection { + projection_type: ProjectionType::Include, + non_key_attributes: Some(vec!["nk".to_owned()]), + }, + provisioned_throughput: None, + }]), + ..Default::default() + }, + ) + .await + .expect("create the table"); + engine + .create_backup(ACCOUNT, "legacy", "bk") + .await + .expect("create the backup"); + drop(engine); + + let raw_ks = serde_json::json!([{"AttributeName": LEGACY, "KeyType": "HASH"}]); + let raw_ad = serde_json::json!([ + {"AttributeName": LEGACY, "AttributeType": "S"}, + {"AttributeName": LEGACY_GSI_KEY, "AttributeType": "S"} + ]); + let raw_gsi_ks = serde_json::json!([{"AttributeName": LEGACY_GSI_KEY, "KeyType": "HASH"}]); + let raw_proj = serde_json::json!({ + "ProjectionType": "INCLUDE", + "NonKeyAttributes": [LEGACY_PROJECTED] + }); + for (sql, values) in [ + ( + "UPDATE tables SET key_schema = $1, attribute_definitions = $2", + (&raw_ks, &raw_ad), + ), + ( + "UPDATE indexes SET key_schema = $1, projection = $2", + (&raw_gsi_ks, &raw_proj), + ), + ( + "UPDATE backups SET key_schema = $1, attribute_definitions = $2", + (&raw_ks, &raw_ad), + ), + ] { + sqlx::query(sql) + .bind(values.0) + .bind(values.1) + .execute(s.pool()) + .await + .expect(sql); + } + let mut definition: serde_json::Value = + sqlx::query_scalar("SELECT definition FROM backup_definitions") + .fetch_one(s.pool()) + .await + .expect("the backup definition"); + definition["GlobalSecondaryIndexes"][0]["KeySchema"] = raw_gsi_ks.clone(); + definition["GlobalSecondaryIndexes"][0]["Projection"] = raw_proj.clone(); + sqlx::query("UPDATE backup_definitions SET definition = $1") + .bind(&definition) + .execute(s.pool()) + .await + .expect("write the raw definition"); + assert_eq!(s.catalog_version().await, "0.0.4"); + s + } + + fn bootstrapper(&self) -> PostgresBootstrapper { + PostgresBootstrapper::new(bootstrap_config(&self.base, &self.names[0], &self.names[1])) + } + + async fn engine(&self) -> PostgresEngine { + PostgresEngine::new( + &PostgresConfig { + connection_string: format!("{}/{}", self.base, self.names[0]), + pool_size: 4, + max_item_size_bytes: 400_000, + }, + "us-east-1", + ) + .await + .expect("engine") + } + + async fn catalog_version(&self) -> String { + sqlx::query_scalar("SELECT value FROM settings WHERE key = 'catalog_version'") + .fetch_one(self.pool()) + .await + .expect("catalog_version") + } + + /// The seven columns' stored text, for before-and-after comparisons. + async fn stored_text(&self) -> String { + sqlx::query_scalar( + "SELECT (SELECT string_agg(key_schema::text || attribute_definitions::text, '|' \ + ORDER BY table_name) FROM tables) \ + || ' ' || (SELECT string_agg(key_schema::text || projection::text, '|' \ + ORDER BY index_name) FROM indexes) \ + || ' ' || (SELECT string_agg(key_schema::text || attribute_definitions::text, '|' \ + ORDER BY backup_arn) FROM backups) \ + || ' ' || (SELECT string_agg(definition::text, '|' ORDER BY backup_arn) \ + FROM backup_definitions)", + ) + .fetch_one(self.pool()) + .await + .expect("stored text") + } + + async fn backup_arn(&self) -> String { + sqlx::query_scalar("SELECT backup_arn FROM backups") + .fetch_one(self.pool()) + .await + .expect("backup arn") + } +} + +fn names(ks: &[KeySchemaElement]) -> Vec<&str> { + ks.iter().map(|k| k.attribute_name.as_str()).collect() +} + +#[tokio::test] +async fn legacy_names_with_control_characters_survive_the_upgrade() { + if base_conn().is_none() { + eprintln!("SKIP legacy_names_with_control_characters_survive_the_upgrade: no PostgreSQL"); + return; + } + let s = Scratch::legacy().await; + let before = s.stored_text().await; + // The raw pair is in the stored text (jsonb::text renders it as \u0001). + assert!(before.contains("a\\u0001\\u0001z"), "{before}"); + + // Without the migration this binary's decoder would misread the names, + // so serve refuses the 0.0.4 catalog outright. + let engine = s.engine().await; + match engine.prepare_catalog_for_serve().await { + Err(BackendError::CatalogVersionMismatch { expected, found }) => { + assert_eq!((expected.as_str(), found.as_str()), ("0.0.5", "0.0.4")); + } + other => panic!("serve must refuse the 0.0.4 catalog: {other:?}"), + } + drop(engine); + + let b = s.bootstrapper(); + let pending = b.pending_data_migrations().await.expect("pending"); + assert!( + pending.contains(&"catalog 004_escape_catalog_control_chars.sql".to_owned()), + "{pending:?}" + ); + b.run_data_migrations().await.expect("migrate --yes"); + assert_eq!(s.catalog_version().await, "0.0.5"); + let after = s.stored_text().await; + assert!(!after.contains("a\\u0001\\u0001z"), "re-encoded: {after}"); + assert!(after.contains("a\\u0001\\u0002\\u0001\\u0002z"), "{after}"); + + // A second run changes nothing: the ledger records the file. + assert!( + b.pending_data_migrations() + .await + .expect("pending") + .is_empty() + ); + b.run_data_migrations().await.expect("second migrate --yes"); + assert_eq!( + s.stored_text().await, + after, + "a second run must not rewrite" + ); + + let engine = s.engine().await; + engine + .prepare_catalog_for_serve() + .await + .expect("serve starts on the migrated catalog"); + let desc = engine + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: "legacy".to_owned(), + }, + ) + .await + .expect("DescribeTable"); + assert_eq!(names(&desc.key_schema), [LEGACY]); + let mut defs: Vec<&str> = desc + .attribute_definitions + .iter() + .map(|a| a.attribute_name.as_str()) + .collect(); + defs.sort_unstable(); + assert_eq!(defs, [LEGACY, LEGACY_GSI_KEY]); + let gsi = &desc.global_secondary_indexes.as_deref().expect("GSIs")[0]; + assert_eq!(names(&gsi.key_schema), [LEGACY_GSI_KEY]); + assert_eq!( + gsi.projection.non_key_attributes.as_deref(), + Some(&[LEGACY_PROJECTED.to_owned()][..]) + ); + // backups.key_schema and attribute_definitions (DescribeBackup and the + // restore source) and backup_definitions.definition (the restore source). + let backup = engine + .describe_backup(ACCOUNT, &s.backup_arn().await) + .await + .expect("DescribeBackup"); + assert_eq!(names(&backup.source_table_details.key_schema), [LEGACY]); + let source = engine + .backup_restore_source(ACCOUNT, &s.backup_arn().await) + .await + .expect("restore source"); + assert_eq!(names(&source.key_schema), [LEGACY]); + let mut defs: Vec<&str> = source + .attribute_definitions + .iter() + .map(|a| a.attribute_name.as_str()) + .collect(); + defs.sort_unstable(); + assert_eq!(defs, [LEGACY, LEGACY_GSI_KEY]); + let definition = source.definition.expect("the backup definition"); + let bgsi = &definition.global_secondary_indexes[0]; + assert_eq!(names(&bgsi.key_schema), [LEGACY_GSI_KEY]); + assert_eq!( + bgsi.projection.non_key_attributes.as_deref(), + Some(&[LEGACY_PROJECTED.to_owned()][..]) + ); + drop(engine); +} + +#[tokio::test] +async fn a_catalog_stamped_current_without_the_record_is_refused_at_serve() { + if base_conn().is_none() { + eprintln!("SKIP a_catalog_stamped_current_without_the_record_is_refused_at_serve"); + return; + } + let s = Scratch::legacy().await; + sqlx::query("UPDATE settings SET value = '0.0.5' WHERE key = 'catalog_version'") + .execute(s.pool()) + .await + .expect("stamp"); + let engine = s.engine().await; + match engine.prepare_catalog_for_serve().await { + Err(BackendError::InitializationFailed(m)) => { + assert!(m.contains("004_escape_catalog_control_chars.sql"), "{m}"); + assert!(m.contains("extenddb migrate --yes"), "{m}"); + } + other => panic!("serve must refuse the unrecorded migration: {other:?}"), + } + drop(engine); +} + +#[tokio::test] +async fn the_migration_refuses_to_run_while_another_session_holds_the_catalog() { + if base_conn().is_none() { + eprintln!("SKIP the_migration_refuses_to_run_while_another_session_holds_the_catalog"); + return; + } + let s = Scratch::legacy().await; + let before = s.stored_text().await; + // An operator's psql, or a server still running: a session without the + // migrate command's application name. + let opts: PgConnectOptions = format!("{}/{}", s.base, s.names[0]) + .parse() + .expect("catalog url"); + let other = PgPoolOptions::new() + .max_connections(1) + .min_connections(1) + .connect_with(opts.application_name("psql")) + .await + .expect("other session"); + let _: i32 = sqlx::query_scalar("SELECT 1") + .fetch_one(&other) + .await + .expect("hold the session open"); + let b = s.bootstrapper(); + let err = b + .check_exclusive_access_for_pending_migrations() + .await + .expect_err("another session holds the catalog"); + let text = format!("{err:?}"); + assert!( + text.contains("other connection(s) hold the catalog database"), + "{text}" + ); + assert!(text.contains("psql"), "{text}"); + let err = b + .run_data_migrations() + .await + .expect_err("the migration itself refuses too"); + assert!( + format!("{err:?}").contains("other connection(s) hold the catalog database"), + "{err:?}" + ); + assert_eq!(s.stored_text().await, before, "a refusal changes nothing"); + assert_eq!(s.catalog_version().await, "0.0.4"); + other.close().await; + b.run_data_migrations() + .await + .expect("migrate --yes once the session is gone"); + assert_eq!(s.catalog_version().await, "0.0.5"); +} diff --git a/crates/storage-postgres/tests/catalog_nul_names.rs b/crates/storage-postgres/tests/catalog_nul_names.rs new file mode 100644 index 000000000..6f305b3cb --- /dev/null +++ b/crates/storage-postgres/tests/catalog_nul_names.rs @@ -0,0 +1,615 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 +//! Catalog round trip of attribute names containing U+0000 on PostgreSQL. +//! +//! DynamoDB accepts U+0000 in a key attribute name and `DescribeTable` +//! returns it byte-identical. The catalog stores `key_schema`, +//! `attribute_definitions` and index `projection` as `jsonb`, which rejects +//! the `\u0000` escape, so those columns are written through the escape in +//! `catalog_json` and read back through its inverse. This file pins the +//! round trip at the storage layer: create with the character in a table key +//! name, a GSI key name, an LSI key name and a projected attribute name; +//! describe; read the key info the data plane uses; add and remove a GSI on +//! the attribute; describe the table's stream; back the table up, read the +//! backup's description and restore source, restore it into a second table and +//! describe that; delete. Every decode site the catalog has for these columns +//! (DescribeTable, key info, UpdateTable, DescribeStream, backup capture, +//! DescribeBackup, the restore source and the restore target) returns the names +//! byte for byte. It also checks the stored column holds no U+0000, which is +//! what a PostgreSQL `jsonb` column cannot hold. +//! +//! Requires `EXTENDDB_TEST_PG_CONNECTION_STRING`, a base URL with no database +//! component, pointing at a server whose role may create and drop databases. +//! Without it every test reports a skip and passes. + +use extenddb_core::types::{ + AttributeDefinition, BillingMode, CreateGsiAction, CreateTableInput, DeleteGsiAction, + DeleteTableInput, DescribeStreamInput, DescribeTableInput, GlobalSecondaryIndexUpdate, + GsiInput, KeySchemaElement, KeyType, LsiInput, Projection, ProjectionType, ScalarAttributeType, + StreamSpecification, StreamViewType, UpdateTableInput, +}; +use extenddb_storage::{ + BackupEngine, RestoreRequest, RestoreTableOverrides, StreamEngine, TableEngine, +}; +use extenddb_storage_postgres::{PostgresConfig, PostgresEngine}; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; + +const ACCOUNT: &str = "123456789012"; +const REGION: &str = "us-east-1"; + +const PK: &str = "p\u{0}k"; +const SK: &str = "s\u{0}k"; +const GK: &str = "g\u{0}k"; +const LK: &str = "l\u{0}k"; +const PROJECTED: &str = "n\u{0}k"; + +/// A catalog database and a data database, as `init` creates them. Two +/// databases rather than one: the catalog's `stream_shards` references +/// `tables`, and the data database's copy does not, so a single database +/// would refuse the shard rows the engine writes before the catalog +/// transaction commits. +struct Scratch { + engine: Option, + db: Option, + base: String, + db_name: String, +} + +impl Scratch { + fn engine(&self) -> &PostgresEngine { + self.engine.as_ref().expect("engine") + } + + fn db(&self) -> &PgPool { + self.db.as_ref().expect("db") + } +} + +/// Drops the scratch database on every exit, a failed assertion included, so a +/// failing run leaves nothing behind on a shared server. +impl Drop for Scratch { + fn drop(&mut self) { + let base = self.base.clone(); + let names = [self.db_name.clone(), format!("{}_d", self.db_name)]; + let engine = self.engine.take(); + let db = self.db.take(); + let _ = std::thread::spawn(move || { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("runtime"); + rt.block_on(async move { + drop(engine); + if let Some(db) = db { + db.close().await; + } + if let Ok(admin) = PgPoolOptions::new() + .max_connections(1) + .connect(&format!("{base}/postgres")) + .await + { + for n in &names { + let _ = + sqlx::query(&format!("DROP DATABASE IF EXISTS \"{n}\" WITH (FORCE)")) + .execute(&admin) + .await; + } + admin.close().await; + } + }); + }) + .join(); + } +} + +fn base_conn() -> Option { + let conn = std::env::var("EXTENDDB_TEST_PG_CONNECTION_STRING").ok()?; + (!conn.trim().is_empty()).then(|| conn.trim_end_matches('/').to_owned()) +} + +/// Every `.sql` file under `dir`, in filename order. +fn shipped_migrations(dir: &str) -> Vec { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); + let mut files: Vec<_> = std::fs::read_dir(root.join(dir)) + .expect("read the migrations directory") + .map(|e| e.expect("directory entry").path()) + .filter(|p| p.extension().is_some_and(|e| e == "sql")) + .collect(); + files.sort(); + files + .iter() + .map(|f| std::fs::read_to_string(f).expect("read a migration file")) + .collect() +} + +async fn scratch() -> Scratch { + let base = base_conn().expect("caller checks base_conn() first"); + let db_name = format!("eddb_nul_{}", uuid::Uuid::new_v4().simple())[..24].to_owned(); + let admin = PgPoolOptions::new() + .max_connections(1) + .connect(&format!("{base}/postgres")) + .await + .expect("connect to the postgres maintenance database"); + for n in [db_name.clone(), format!("{db_name}_d")] { + sqlx::query(&format!("CREATE DATABASE \"{n}\"")) + .execute(&admin) + .await + .expect("create a scratch database"); + } + admin.close().await; + let url = format!("{base}/{db_name}"); + let db = PgPoolOptions::new() + .max_connections(2) + .connect(&url) + .await + .expect("connect to the scratch catalog"); + for sql in shipped_migrations("migrations") { + sqlx::raw_sql(&sql) + .execute(&db) + .await + .expect("apply a catalog migration"); + } + let data_url = format!("{base}/{db_name}_d"); + let data = PgPoolOptions::new() + .max_connections(1) + .connect(&data_url) + .await + .expect("connect to the scratch data database"); + for sql in shipped_migrations("data_migrations") { + sqlx::raw_sql(&sql) + .execute(&data) + .await + .expect("apply a data migration"); + } + data.close().await; + sqlx::query( + "INSERT INTO settings (key, value) VALUES ('data_database_connection_string', $1) \ + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value", + ) + .bind(&data_url) + .execute(&db) + .await + .expect("point the catalog at the data database"); + sqlx::query("UPDATE settings SET value = '0' WHERE key = 'control_plane_delay_seconds'") + .execute(&db) + .await + .expect("pin the control-plane delay to zero"); + sqlx::query("INSERT INTO accounts (account_id, account_name) VALUES ($1, $2)") + .bind(ACCOUNT) + .bind(format!("acct-{db_name}")) + .execute(&db) + .await + .expect("seed the account row"); + let engine = PostgresEngine::new( + &PostgresConfig { + connection_string: url, + pool_size: 10, + max_item_size_bytes: 400_000, + }, + REGION, + ) + .await + .expect("open a PostgresEngine on the scratch database"); + Scratch { + engine: Some(engine), + db: Some(db), + base, + db_name, + } +} + +fn s_attr(name: &str) -> AttributeDefinition { + AttributeDefinition { + attribute_name: name.to_owned(), + attribute_type: ScalarAttributeType::S, + } +} + +fn key(name: &str, key_type: KeyType) -> KeySchemaElement { + KeySchemaElement { + attribute_name: name.to_owned(), + key_type, + } +} + +fn include(names: &[&str]) -> Projection { + Projection { + projection_type: ProjectionType::Include, + non_key_attributes: Some(names.iter().map(|n| (*n).to_owned()).collect()), + } +} + +/// A (PK, SK) table with a GSI on GK and an LSI on LK, both projecting PROJECTED. +fn input(table: &str) -> CreateTableInput { + CreateTableInput { + table_name: table.to_owned(), + key_schema: vec![key(PK, KeyType::Hash), key(SK, KeyType::Range)], + attribute_definitions: vec![s_attr(PK), s_attr(SK), s_attr(GK), s_attr(LK)], + billing_mode: Some(BillingMode::PayPerRequest), + global_secondary_indexes: Some(vec![GsiInput { + index_name: "gsi1".to_owned(), + key_schema: vec![key(GK, KeyType::Hash)], + projection: include(&[PROJECTED]), + provisioned_throughput: None, + }]), + local_secondary_indexes: Some(vec![LsiInput { + index_name: "lsi1".to_owned(), + key_schema: vec![key(PK, KeyType::Hash), key(LK, KeyType::Range)], + projection: include(&[PROJECTED]), + }]), + stream_specification: Some(StreamSpecification { + stream_enabled: true, + stream_view_type: Some(StreamViewType::NewAndOldImages), + }), + ..Default::default() + } +} + +/// An `UpdateTableInput` carrying only a GSI update. +fn gsi_update( + table: &str, + attribute_definitions: Option>, + update: GlobalSecondaryIndexUpdate, +) -> UpdateTableInput { + UpdateTableInput { + table_name: table.to_owned(), + billing_mode: None, + table_throughput_mode: None, + provisioned_throughput: None, + deletion_protection_enabled: None, + global_secondary_index_updates: Some(vec![update]), + attribute_definitions, + stream_specification: None, + table_class: None, + on_demand_throughput: None, + vector_index_updates: None, + } +} + +fn names(ks: &[KeySchemaElement]) -> Vec<&str> { + ks.iter().map(|k| k.attribute_name.as_str()).collect() +} + +fn def_names(ad: &[AttributeDefinition]) -> Vec<&str> { + let mut v: Vec<&str> = ad.iter().map(|a| a.attribute_name.as_str()).collect(); + v.sort_unstable(); + v +} + +async fn stored_catalog_text(db: &PgPool, table: &str) -> String { + let (ks, ad, idx): (String, String, String) = sqlx::query_as( + "SELECT t.key_schema::text, t.attribute_definitions::text, \ + COALESCE(string_agg(i.key_schema::text || i.projection::text, ' '), '') \ + FROM tables t LEFT JOIN indexes i ON i.table_id = t.table_id \ + WHERE t.account_id = $1 AND t.table_name = $2 \ + GROUP BY t.key_schema, t.attribute_definitions", + ) + .bind(ACCOUNT) + .bind(table) + .fetch_one(db) + .await + .expect("read the stored catalog columns"); + format!("{ks} {ad} {idx}") +} + +#[tokio::test] +async fn key_attribute_names_with_nul_round_trip_through_the_catalog() { + let Some(_) = base_conn() else { + eprintln!("SKIP: EXTENDDB_TEST_PG_CONNECTION_STRING is not set"); + return; + }; + let sc = scratch().await; + let table = "nul_names"; + + let created = sc + .engine() + .create_table(ACCOUNT, input(table)) + .await + .expect("CreateTable with U+0000 in key attribute names"); + assert_eq!(names(&created.key_schema), [PK, SK]); + + // The stored jsonb holds no U+0000 and no \u0000 escape: that is the + // byte the column cannot hold, so its absence is what makes the insert + // succeed. The names are present in escaped form. + let stored = stored_catalog_text(sc.db(), table).await; + assert!( + !stored.contains('\u{0}'), + "stored catalog text carries a NUL: {stored}" + ); + assert!( + !stored.contains("\\u0000"), + "stored catalog text carries \\u0000: {stored}" + ); + // `jsonb::text` renders control characters as JSON escapes. This pins the + // escape alphabet (U+0000 as U+0001 U+0001) on purpose: it is the mapping + // the item data columns use for `item_data`, so one decoder can read both + // this column and those later; a change here would split the two. + assert!( + stored.contains("p\\u0001\\u0001k"), + "escaped key name absent from: {stored}" + ); + + let desc = sc + .engine() + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: table.to_owned(), + }, + ) + .await + .expect("DescribeTable"); + assert_eq!(names(&desc.key_schema), [PK, SK]); + assert_eq!( + def_names(&desc.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + let gsi = &desc.global_secondary_indexes.as_deref().expect("GSIs")[0]; + assert_eq!(names(&gsi.key_schema), [GK]); + assert_eq!( + gsi.projection.non_key_attributes.as_deref(), + Some(&[PROJECTED.to_owned()][..]) + ); + let lsi = &desc.local_secondary_indexes.as_deref().expect("LSIs")[0]; + assert_eq!(names(&lsi.key_schema), [PK, LK]); + assert_eq!( + lsi.projection.non_key_attributes.as_deref(), + Some(&[PROJECTED.to_owned()][..]) + ); + + // The key info the data plane and authorization read. + let info = sc + .engine() + .table_key_info(ACCOUNT, table) + .await + .expect("table_key_info"); + assert_eq!(names(&info.key_schema), [PK, SK]); + assert_eq!( + def_names(&info.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + assert_eq!(names(&info.global_secondary_indexes[0].key_schema), [GK]); + assert_eq!(names(&info.local_secondary_indexes[0].key_schema), [PK, LK]); + + // DescribeStream reads the key schema from the catalog on its own path. + let stream_arn = desc.latest_stream_arn.clone().expect("the stream ARN"); + let stream = sc + .engine() + .describe_stream( + ACCOUNT, + &DescribeStreamInput { + stream_arn, + limit: None, + exclusive_start_shard_id: None, + }, + ) + .await + .expect("DescribeStream"); + assert_eq!(names(&stream.key_schema), [PK, SK]); + + // The backup copies the key schema, the definitions and every index into + // backups and backup_definitions; DescribeBackup and the restore source + // read them back; the restore target is created from the restore source. + let backup = sc + .engine() + .create_backup(ACCOUNT, table, "bk367") + .await + .expect("CreateBackup"); + let described = sc + .engine() + .describe_backup(ACCOUNT, &backup.backup_arn) + .await + .expect("DescribeBackup"); + assert_eq!(names(&described.source_table_details.key_schema), [PK, SK]); + let source = sc + .engine() + .backup_restore_source(ACCOUNT, &backup.backup_arn) + .await + .expect("restore source"); + assert_eq!(names(&source.key_schema), [PK, SK]); + assert_eq!( + def_names(&source.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + let definition = source.definition.as_ref().expect("the backup definition"); + assert_eq!( + names(&definition.global_secondary_indexes[0].key_schema), + [GK] + ); + assert_eq!( + definition.global_secondary_indexes[0] + .projection + .non_key_attributes + .as_deref(), + Some(&[PROJECTED.to_owned()][..]) + ); + assert_eq!( + names(&definition.local_secondary_indexes[0].key_schema), + [PK, LK] + ); + assert_eq!( + definition.local_secondary_indexes[0] + .projection + .non_key_attributes + .as_deref(), + Some(&[PROJECTED.to_owned()][..]) + ); + let restored_name = "nul_names_restored"; + let restore_input = source.restore_input(restored_name, &RestoreTableOverrides::default()); + extenddb_core::validation::validate_create_table( + &restore_input, + &extenddb_core::limits::LimitsConfig::default(), + ) + .expect("the restore input validates as CreateTable would"); + sc.engine() + .restore_table_from_backup( + ACCOUNT, + RestoreRequest { + backup_arn: backup.backup_arn.clone(), + source_table_arn: source.source_table_arn.clone(), + restore_date_time: source.backup_creation_date_time, + input: restore_input, + }, + ) + .await + .expect("RestoreTableFromBackup"); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(90); + loop { + let status: Option = sqlx::query_scalar( + "SELECT table_status FROM tables WHERE account_id = $1 AND table_name = $2", + ) + .bind(ACCOUNT) + .bind(restored_name) + .fetch_optional(sc.db()) + .await + .expect("restore target status"); + if status.as_deref() == Some("ACTIVE") { + break; + } + assert!( + tokio::time::Instant::now() < deadline, + "restore target still {status:?} after 90 s" + ); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + let restored = sc + .engine() + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: restored_name.to_owned(), + }, + ) + .await + .expect("DescribeTable on the restore target"); + assert_eq!(names(&restored.key_schema), [PK, SK]); + assert_eq!( + def_names(&restored.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + let rgsi = &restored.global_secondary_indexes.as_deref().expect("GSIs")[0]; + assert_eq!(names(&rgsi.key_schema), [GK]); + assert_eq!( + rgsi.projection.non_key_attributes.as_deref(), + Some(&[PROJECTED.to_owned()][..]) + ); + let rlsi = &restored.local_secondary_indexes.as_deref().expect("LSIs")[0]; + assert_eq!(names(&rlsi.key_schema), [PK, LK]); + assert_eq!( + rlsi.projection.non_key_attributes.as_deref(), + Some(&[PROJECTED.to_owned()][..]) + ); + + // Remove the GSI, then add one back on the same U+0000 attribute. + let after_delete = sc + .engine() + .update_table( + ACCOUNT, + gsi_update( + table, + None, + GlobalSecondaryIndexUpdate { + create: None, + update: None, + delete: Some(DeleteGsiAction { + index_name: "gsi1".to_owned(), + }), + }, + ), + ) + .await + .expect("UpdateTable delete GSI"); + assert!( + after_delete + .global_secondary_indexes + .as_deref() + .unwrap_or(&[]) + .is_empty() + ); + // The pruned definitions were rewritten through the escape and read back. + assert_eq!(def_names(&after_delete.attribute_definitions), [LK, PK, SK]); + + let after_create = sc + .engine() + .update_table( + ACCOUNT, + gsi_update( + table, + Some(vec![s_attr(GK)]), + GlobalSecondaryIndexUpdate { + create: Some(CreateGsiAction { + index_name: "gsi2".to_owned(), + key_schema: vec![key(GK, KeyType::Hash)], + projection: include(&[PROJECTED]), + provisioned_throughput: None, + }), + update: None, + delete: None, + }, + ), + ) + .await + .expect("UpdateTable create GSI on a U+0000 attribute"); + let gsi2 = &after_create + .global_secondary_indexes + .as_deref() + .expect("GSIs")[0]; + assert_eq!(gsi2.index_name, "gsi2"); + assert_eq!(names(&gsi2.key_schema), [GK]); + assert_eq!( + def_names(&after_create.attribute_definitions), + [GK, LK, PK, SK] + ); + + let deleted = sc + .engine() + .delete_table( + ACCOUNT, + DeleteTableInput { + table_name: table.to_owned(), + }, + ) + .await + .expect("DeleteTable"); + assert_eq!(names(&deleted.key_schema), [PK, SK]); +} + +#[tokio::test] +async fn plain_names_are_stored_exactly_as_before() { + let Some(_) = base_conn() else { + eprintln!("SKIP: EXTENDDB_TEST_PG_CONNECTION_STRING is not set"); + return; + }; + let sc = scratch().await; + let table = "plain_names"; + let mut plain = input(table); + plain.key_schema = vec![key("pk", KeyType::Hash), key("sk", KeyType::Range)]; + plain.attribute_definitions = vec![s_attr("pk"), s_attr("sk"), s_attr("gk"), s_attr("lk")]; + plain.global_secondary_indexes = Some(vec![GsiInput { + index_name: "gsi1".to_owned(), + key_schema: vec![key("gk", KeyType::Hash)], + projection: include(&["nk"]), + provisioned_throughput: None, + }]); + plain.local_secondary_indexes = Some(vec![LsiInput { + index_name: "lsi1".to_owned(), + key_schema: vec![key("pk", KeyType::Hash), key("lk", KeyType::Range)], + projection: include(&["nk"]), + }]); + sc.engine() + .create_table(ACCOUNT, plain) + .await + .expect("CreateTable"); + + // No U+0001 anywhere: an ordinary name is written as itself. + let stored = stored_catalog_text(sc.db(), table).await; + assert!( + !stored.contains('\u{1}'), + "escape marker in ordinary catalog text: {stored}" + ); + assert!( + stored.contains(r#"{"KeyType": "HASH", "AttributeName": "pk"}"#) + || stored.contains(r#"{"AttributeName": "pk", "KeyType": "HASH"}"#) + ); +} diff --git a/crates/storage/src/bootstrapper.rs b/crates/storage/src/bootstrapper.rs index 5e3ca8dd2..87279e1c3 100755 --- a/crates/storage/src/bootstrapper.rs +++ b/crates/storage/src/bootstrapper.rs @@ -135,6 +135,17 @@ pub trait Bootstrapper: Send + Sync { /// decide whether confirmation is required — without running anything. async fn pending_data_migrations(&self) -> OpResult>; + /// Refuse when a pending migration needs the databases to itself and + /// another session holds one of them. + /// + /// `migrate` calls this after confirmation and before its first write, so + /// a refusal leaves the deployment exactly as it found it (the catalog + /// version in particular). Defaults to a no-op; a backend whose pending + /// migrations never rewrite rows in place has nothing to check. + async fn check_exclusive_access_for_pending_migrations(&self) -> OpResult<()> { + Ok(()) + } + /// Record the data database connection string in the catalog. async fn record_data_connection(&self) -> OpResult<()>; diff --git a/docs/differences-from-dynamodb.md b/docs/differences-from-dynamodb.md index 2d6469b7e..2e12f4b7a 100755 --- a/docs/differences-from-dynamodb.md +++ b/docs/differences-from-dynamodb.md @@ -48,6 +48,7 @@ adaptation when switching between ExtendDB and the real service. |------|----------|------| | Table creation delay | Returns `CREATING` immediately; transitions to `ACTIVE` typically within seconds. Same behavior for on-demand and provisioned | Configurable via `control_plane_delay_seconds` runtime setting (default: 5s) | | DeletionProtectionEnabled | Enforced | Enforced (accepted and stored, DeleteTable rejects when enabled) | +| U+0000 in a key attribute name, an index key attribute name or a projected attribute name | Accepted; `DescribeTable` returns the name byte-identical (measured 2026-09-18 and 2026-10-24) | The same on all four backends. On PostgreSQL the catalog stores those names through an escape (U+0000 as U+0001 U+0001, U+0001 as U+0001 U+0002) because `jsonb` cannot hold U+0000; catalog 0.0.5 re-encodes names an earlier release stored raw. After that upgrade, a table whose key attribute, index key or projected attribute names contain U+0000 or U+0001 on PostgreSQL is not readable by an earlier release (the earlier binary refuses the catalog version; see the Upgrade Manual). A table name or index name with U+0000 is refused with the service's `ValidationException` on every backend | ## Time to Live (TTL) @@ -85,7 +86,7 @@ adaptation when switching between ExtendDB and the real service. | Vector index update propagation | Eventually consistent, the same model as a GSI | Matches DynamoDB. Maintenance is queued on the same propagation queue as async GSIs, so a search immediately after a write may not see it. Governed by the same `index_propagation_delay_ms` setting; unlike a GSI there is no per-index override. A value of 0 applies maintenance inline in the write's own transaction, which is stricter than the service and exists so a test can assert steady state without waiting. Inline additionally requires the table's propagation queue to be empty: while rows queued during an index build are still draining, a new write queues behind them instead of overtaking them, so a brief search-after-write window exists even at 0 until the queue drains. That window is ordering, not data loss; the write is applied, in order, by the queue worker. That zero behaves differently while an index is still building, and the two backends differ: **PostgreSQL** applies inline only to an index that is already `ACTIVE`, and defers a write to a building index whatever the delay says, because the write must not reach the index ahead of the backfill's older snapshot of the same item; **SQLite** does not check the status on this path, so with a zero delay a write during a backfill is applied inline and bypasses the hold that keeps the two ordered. The SQLite behaviour is tracked as a defect (F-20), not intended, and it is reachable only with the zero delay, which is a test setting. | | `SearchVectors` score at extreme magnitudes (PostgreSQL backend only) | Returns the true distance as a number for any vector of finite components | The score is bounded to a finite value instead, and the bound is not a measured service answer. pgvector accumulates distances in single precision, so magnitudes far below `f32::MAX` overflow inside the extension: Euclidean above about 9.2e18, dot product above about 1.8e19, and cosine at both ends, above about 1.8e19 and below about 3.7e-23, which is where a component's single-precision square rounds to zero. A non-finite score cannot be serialised as JSON at all, so the result is bounded in SQL: `1e308` for an overflowed distance, `-1e308` for an overflowed negated inner product, which the score contract negates so a client sees `1e308` in `Score`, and 1.0 for a cosine that comes back NaN. Ranking is unaffected at the overflow end, because each bound sits at the end its metric overflows towards, so the farthest row stays farthest and the most similar stays most similar; two rows that both overflow tie, and the tie breaks on the base key. At the underflow end cosine loses resolution rather than being bounded, and which side is tiny decides how much. For a tiny **query** vector, its norm underflows while the inner product usually does not, so the quotient is an infinity that pgvector clamps and the reported distance collapses to one of 0, 1 or 2 following the sign of the inner product, with the 1.0 substitute firing only when the vectors are exactly orthogonal and the quotient is therefore 0/0 (measured). For a tiny **stored** vector it is worse: the stored norm is computed in single precision and reaches zero, so the zero-vector guard fires and that row reports 1.0 at every angle, parallel included. A corpus of tiny embeddings therefore loses ranking altogether rather than losing resolution. For a tiny query vector, by contrast, ranking still separates nearer-than-orthogonal from farther and loses resolution only within each half. The SQLite backend owns its own arithmetic, computes in double precision, and reports the true value, which is why this row is scoped to PostgreSQL. | | Vector index deletion window | `UpdateTable` Delete leaves the index in `DELETING` long enough to observe, then removes it | No observable `DELETING` window on either backend: the catalog row is removed inside the `UpdateTable` transaction, so a `DescribeTable` immediately afterwards already omits the index. The index's data table is dropped after that commit, in a separate transaction, and the two backends handle a failure there differently. **PostgreSQL** treats it as best effort, because the data table lives in a different database entirely: a failure is logged and skipped rather than failing the request. **SQLite** propagates it, so a failed drop returns an error from an `UpdateTable` whose catalog change has already committed, which is the more surprising outcome of the two: the index is gone from the catalog and the caller saw a failure. So an operator debugging a leftover `_ddb_vec_*` table should look for that warning rather than assume the delete was incomplete. | -| Backing up and restoring a table that has vector indexes | Restores the table with its vector indexes intact: the configuration survives, items keep their vector attributes, and `SearchVectors` works as soon as the table is `ACTIVE` (measured) | Not supported. On PostgreSQL and SQLite, the two backends with vector indexes, `CreateBackup` of such a table is refused with `ValidationException` "table has vector index(es); restoring a table with vector indexes is not supported by this storage backend", and no backup is created, because a restore cannot carry the index data across. A backup of a vector table taken before catalog 0.0.4 is refused at restore on PostgreSQL with the same message naming the backup; on SQLite such a backup restores without its vector indexes. A backup of a table with no vector indexes restores normally. | +| Backing up and restoring a table that has vector indexes | Restores the table with its vector indexes intact: the configuration survives, items keep their vector attributes, and `SearchVectors` works as soon as the table is `ACTIVE` (measured) | Not supported. On PostgreSQL and SQLite, the two backends with vector indexes, `CreateBackup` of such a table is refused with `ValidationException` "table has vector index(es); restoring a table with vector indexes is not supported by this storage backend", and no backup is created, because a restore cannot carry the index data across. A backup of a vector table taken before catalog 0.0.4 is refused at restore on PostgreSQL with the same message naming the backup; on SQLite such a backup restores without its vector indexes. A backup of a table with no vector indexes restores normally. | | `SearchVectors` endpoint | Served only on `search-dynamodb..amazonaws.com`. The standard `dynamodb..amazonaws.com` endpoint answers the same request with HTTP 400 `UnknownOperationException` ("This operation is not supported by this endpoint"); every control-plane and item operation stays on the standard endpoint. Signing is unchanged either way (service name `dynamodb`, target prefix `DynamoDB_20120810`) | Served on the same endpoint as every other operation, so a client is pointed at one ExtendDB endpoint for all of them. Two consequences worth knowing: an SDK that resolves a separate search hostname from its endpoint ruleset needs its endpoint overridden to reach ExtendDB, and ExtendDB does **not** reproduce the service's refusal, so a test asserting `UnknownOperationException` for vector search on the base endpoint passes against Amazon DynamoDB and fails here. | | `SearchVectors` result order for equal distances | Measured unstable: three identical searches returned tied rows in three different orders, and a top-k that truncates a tie group keeps an arbitrary subset of it | Deterministic on both backends, by different means. **PostgreSQL** sorts explicitly on the base table's full primary key after the score, so the order is a property of the query. **SQLite** issues no `ORDER BY` and resolves ties by scan order through a stable top-k, so its order is a property of the plan rather than something the query guarantees. Either way a client re-issuing an identical search sees the same order, which is stricter than the service rather than divergent in outcome. Do not rely on the two backends agreeing on which subset of a truncated tie group they keep. | | Vector indexes on the MongoDB backend | Vector indexes and `SearchVectors` are available | Not supported at all. That backend provides no vector search implementation, so the engine's capability gate refuses `CreateTable` and `UpdateTable` carrying `VectorIndexes` and every `SearchVectors` request, with the same capability message a PostgreSQL deployment without pgvector returns. Every other vector row in this document describes the PostgreSQL and SQLite backends. | @@ -99,7 +100,7 @@ Behaviour stated as measured, and every number below, was observed with release | Area | DynamoDB | ExtendDB | |------|----------|------| -| What a restore carries across | Key schema, attribute definitions, items, secondary indexes, billing mode and provisioned throughput, on-demand throughput (table and GSI level), SSE settings. Table class is not carried: a restore is `STANDARD` whatever the source's class (measured). Stream settings, TTL, tags, auto scaling, IAM policies, and CloudWatch settings must be reapplied by hand | The same set on all four backends, with one gap: the table's on-demand throughput is carried, a GSI's is not (ExtendDB records no per-index on-demand maximum, and `OnDemandThroughput` is not a member of its GSI definition). Table class is not restored: a `STANDARD_INFREQUENT_ACCESS` source restores as `STANDARD`, as on the service. Streams, TTL, tags, and deletion protection are not restored. A backup written before catalog 0.0.4 (PostgreSQL, SQLite, Cassandra) has no recorded table definition and restores with its keys, items, and billing mode but no secondary indexes. A provisioned one restores with 5/5 throughput on PostgreSQL and SQLite and with its recorded throughput on Cassandra | +| What a restore carries across | Key schema, attribute definitions, items, secondary indexes, billing mode and provisioned throughput, on-demand throughput (table and GSI level), SSE settings. Table class is not carried: a restore is `STANDARD` whatever the source's class (measured). Stream settings, TTL, tags, auto scaling, IAM policies, and CloudWatch settings must be reapplied by hand | The same set on all four backends, with one gap: the table's on-demand throughput is carried, a GSI's is not (ExtendDB records no per-index on-demand maximum, and `OnDemandThroughput` is not a member of its GSI definition). Table class is not restored: a `STANDARD_INFREQUENT_ACCESS` source restores as `STANDARD`, as on the service. Streams, TTL, tags, and deletion protection are not restored. A backup written before catalog 0.0.4 (PostgreSQL, SQLite, Cassandra) has no recorded table definition and restores with its keys, items, and billing mode but no secondary indexes. A provisioned one restores with 5/5 throughput on PostgreSQL and SQLite and with its recorded throughput on Cassandra | | How `RestoreTableFromBackup` runs | Returns at once with the target `CREATING`; the service copies in the background | The same on all four backends: the response carries the target `CREATING`, and the items are copied by a background task in the server, so a client that times out or disconnects does not stop it. Once the request has been authenticated and authorized, the rest of it, from reading the backup to registering the restore, runs in a task of its own, so a client that disconnects after that does not cancel it either: the restore runs and the client finds the target with `DescribeTable`. A client that disconnects while the request is still being authenticated or authorized cancels it, and nothing is created *(from the code)*. Measured with a client whose 1 ms read timeout closed its connection before any response arrived: on all four backends the restore was registered and finished. Measured with a 60,000-item backup of a table with no secondary indexes: response in 0.032 s (PostgreSQL), 0.012 s (SQLite), 0.053 s (MongoDB), 0.151 s (Cassandra); `ACTIVE` after 1.5 s, 2.1 s, 3.6 s, and 11.2 s. On Cassandra the response waits for the target table's schema to be created: 0.75 to 1.3 s with five restores running at once *(earlier build)*, which is longer than a client read timeout of 1 s. A restore runs inside the server process; if the process stops, see the abandoned-restore row | | `RestoreSummary` | The restore response and `DescribeTable` while `CREATING` report `SourceBackupArn`, `SourceTableArn`, `RestoreDateTime` (the backup's `BackupCreationDateTime`, not the time the restore ran), and `RestoreInProgress=true`. Once the table is `ACTIVE`, `DescribeTable` omits `RestoreSummary` | The same on all four backends. No response that describes a table (`CreateTable`, `UpdateTable`, `DeleteTable`, `DescribeTable`, `RestoreTableFromBackup`) carries `RestoreSummary` unless the table is `CREATING`; what the service's `UpdateTable` returns for a restored table was not measured | | Reads on a restore target | A `GetItem` or `Scan` on the target while `CREATING` is `ResourceNotFoundException`; the first read after `DescribeTable` reports `ACTIVE` succeeds (measured) | The same on all four backends. The server's key-info cache stores a negative entry (5 s TTL by default) for a name the catalog does not have, and does not store one for a name the catalog has in a state other than `ACTIVE`: on a not-found answer it asks the catalog once more with `DescribeTable`, and authorization records the name as missing in the key-info snapshot it hands the engine, so the engine does not look it up again. A `GetItem`, `PutItem`, `UpdateItem`, `DeleteItem`, `Query`, or `Scan` on the target while it is `CREATING` therefore costs exactly one `table_key_info` and one `describe_table` catalog load, uncached, per request (on SQLite five SQL statements: one for the key info, four for `DescribeTable`; on PostgreSQL `describe_table` is two queries), plus one `describe_table` on the first such request for the throttle registry; the first read after `ACTIVE` loads once more and is then cached. Measured on SQLite with a `GetItem` loop on a `CREATING` restore target of 100,000 items: 40 requests refused, 40 key-info and 41 `DescribeTable` loads in the server's statement log; with the hand-off to the engine removed, 80 and 81. In an earlier run of the fidelity test the first `GetItem` after `ACTIVE` answered in 3 ms after 72 refused reads. | @@ -114,7 +115,7 @@ Behaviour stated as measured, and every number below, was observed with release | Multi-part base table keys (preview) | Not supported | A backup of a table created with `enable_multipart_keys` is refused at restore with `ValidationException`; the item paths address such tables by their first HASH and RANGE attribute only *(from the code and storage-level tests; not checked on a running server)* | | `DeleteBackup` and `DeleteTable` during a restore | `DeleteTable` on the `CREATING` target is `ResourceInUseException`. `DeleteBackup` during a restore was not measured | While the target is `CREATING`, `DeleteTable` on it is `ResourceInUseException` on all four backends, as on the service. `DeleteBackup` on its source backup is `BackupInUseException` on all four: that refusal is ExtendDB's contract, not measured against the service. Both succeed once the target is `ACTIVE`. How the restore and `DeleteBackup` are ordered differs *(from the code)*. PostgreSQL and SQLite register the restore in the transaction that creates the target, holding the backup row, so a concurrent `DeleteBackup` is ordered strictly before (the restore then finds no backup) or after (it finds the restore). MongoDB has no cross-collection transaction: a restore claims the backup document, inserts the target's catalog document, and releases the claim; `DeleteBackup` claims the backup the same way. Cassandra uses two lightweight transactions on different partitions: the target's row is inserted first (`IF NOT EXISTS`, which reserves the name), then the restore is registered on the backup's row (`IF backup_status = 'AVAILABLE'`), which `DeleteBackup` claims on the same partition; a concurrent `DescribeTable` can therefore see a `CREATING` target for a restore that then fails with `BackupNotFoundException`, after which the target is removed. On MongoDB, if `DeleteBackup` returns an internal error or its request is cancelled after the backup's data drop started, the backup stays `AVAILABLE` and in use (`DeleteBackup` and `RestoreTableFromBackup` return `BackupInUseException`) until the heartbeat sweep finishes the delete, about 45 to 80 s later; it is not left restorable without its data. A `DeleteBackup` that stopped while a restore from the same backup is `CREATING` is finished by the sweep after that restore becomes `ACTIVE`. So a `DeleteBackup` that returned an error can still take effect later: besides the case above, one that failed after claiming the backup (for example because it found a restore from it `CREATING`) and whose claim could not then be released, neither in the request nor by the claim guard's release on drop, is finished by the sweep (once no restore from the backup is `CREATING`), although the client was told it failed (not reproduced) *(all from the code and storage-level tests)*. MongoDB reads the catalog from the primary with read concern `local` regardless of the connection string, which the claims rely on; the connection string must use `readPreference=primary` (any other read preference is refused at startup). The write concern follows the connection string and must be `majority` (the server default): a connection string with `w=1` is refused at startup, because a claim acknowledged under `w=1` just before a primary failover can be rolled back | | Concurrent restores of one backup | A second `RestoreTableFromBackup` of a backup while a restore from it is still `CREATING` is `BackupInUseException` (measured) | The same on all four backends: the second restore is refused with `BackupInUseException` and creates nothing; the check runs inside the transaction that registers the restore (PostgreSQL, with the backup row locked `FOR UPDATE`; SQLite), inside the claim on the backup document followed by a check for a `CREATING` target of the backup (MongoDB), or inside the lightweight transaction on the backup row (Cassandra), so two simultaneous requests cannot both pass. Once the first target is `ACTIVE` the backup restores again. A restore of a backup whose `DeleteBackup` is in progress answers differently by backend (the service's answer was not measured): `BackupInUseException` "Backup is being deleted" on MongoDB, where the backup stays `AVAILABLE` under a delete claim until its data is dropped; `BackupNotFoundException` on Cassandra, where the delete marks it `DELETING` first; and on PostgreSQL and SQLite the restore waits for the delete's transaction and then finds no backup (`BackupNotFoundException`). Earlier builds accepted concurrent restores of one backup on PostgreSQL, SQLite, and Cassandra, and on MongoDB after a 2 s retry | -| Restore interrupted by the server stopping | Managed by the service | The copy runs in the server process, so a stopped process leaves its target `CREATING`. Each backend removes such a target without operator action, logs the removal with the table name (at warn; at info on Cassandra), and then lets `DeleteBackup` and a new restore into the name succeed. Measured after `SIGKILL` mid-copy and an immediate restart *(earlier build)*: **PostgreSQL** removes it on a control-plane pass once the target is older than 60 s and its restore lock is free (removed 60 s after the kill). **SQLite** removes it when the next server starts (0.1 s after the restart). **MongoDB** and **Cassandra** remove it once its owner's heartbeat is more than 60 s old, by a sweep that runs at startup and every 15 s, so about 45 to 80 s after the owner stopped (the heartbeat's last refresh can be up to one 15 s interval before the stop; measured: 60 s on MongoDB, 62 s on Cassandra). Until then `DeleteTable` and `DeleteBackup` are refused as during a running restore. Not covered *(from the code)*: on MongoDB, a target whose base copy had finished is not swept but is finished by the index backfill worker after a restart; if that backfill fails on every attempt, the worker retries it every 5 s with a warning and never gives up, so the target stays `CREATING` (and `DeleteTable` and `DeleteBackup` stay refused) until the cause is fixed or the target's catalog document is removed by hand (no failing case was found to reproduce this). On Cassandra, a target left `CREATING` by a build before catalog 0.0.4 carries no heartbeat and is not swept. The MongoDB and Cassandra heartbeats compare wall clocks across servers (MongoDB uses the database server's clock for refreshes), so a server whose clock is far behind can have a live restore swept | +| Restore interrupted by the server stopping | Managed by the service | The copy runs in the server process, so a stopped process leaves its target `CREATING`. Each backend removes such a target without operator action, logs the removal with the table name (at warn; at info on Cassandra), and then lets `DeleteBackup` and a new restore into the name succeed. Measured after `SIGKILL` mid-copy and an immediate restart *(earlier build)*: **PostgreSQL** removes it on a control-plane pass once the target is older than 60 s and its restore lock is free (removed 60 s after the kill). **SQLite** removes it when the next server starts (0.1 s after the restart). **MongoDB** and **Cassandra** remove it once its owner's heartbeat is more than 60 s old, by a sweep that runs at startup and every 15 s, so about 45 to 80 s after the owner stopped (the heartbeat's last refresh can be up to one 15 s interval before the stop; measured: 60 s on MongoDB, 62 s on Cassandra). Until then `DeleteTable` and `DeleteBackup` are refused as during a running restore. Not covered *(from the code)*: on MongoDB, a target whose base copy had finished is not swept but is finished by the index backfill worker after a restart; if that backfill fails on every attempt, the worker retries it every 5 s with a warning and never gives up, so the target stays `CREATING` (and `DeleteTable` and `DeleteBackup` stay refused) until the cause is fixed or the target's catalog document is removed by hand (no failing case was found to reproduce this). On Cassandra, a target left `CREATING` by a build before catalog 0.0.4 carries no heartbeat and is not swept. The MongoDB and Cassandra heartbeats compare wall clocks across servers (MongoDB uses the database server's clock for refreshes), so a server whose clock is far behind can have a live restore swept | | Point-in-time recovery | 35-day window | Not supported. `RestoreTableToPointInTime` is refused with `ValidationException` ("Point-in-time recovery restore is not yet supported"). `UpdateContinuousBackups` is not refused: enabling point-in-time recovery succeeds, and `DescribeContinuousBackups` then reports it `ENABLED` with an `EarliestRestorableDateTime` 35 days back, although nothing is recorded and no restore to any of those times is possible. Same on all four backends | | Where backups live | Managed storage, independent of the table | Inside the database the server already uses, so a lost database loses its backups too: on PostgreSQL in the catalog database (`backups`, `backup_items`, `backup_definitions`, `table_restores`), on SQLite in the one database file, on MongoDB as a `backups` metadata collection in the catalog database and one data collection per backup in the data database, on Cassandra as backup metadata in the catalog keyspace and item payloads in the account's keyspace. Take database-level backups for off-host copies until the backup store program ships | | Backup creation | `CreateBackup` returns while the backup is `CREATING`; `ListBackups` and `DescribeBackup` expose that state until it becomes `AVAILABLE`. `BackupName` and `TableName` are checked as the model says: length 3 to 255 and `[a-zA-Z0-9_.-]+`, with "1 validation error detected: Value '' at 'backupName' failed to satisfy constraint: " texts, joined under "2 validation errors detected:" when both fail (measured). `TableName` may be the table's ARN (measured). A table that does not exist or is still `CREATING` is `TableNotFoundException` "Table not found: " (measured) | `CreateBackup` copies the items before it returns and returns the backup `AVAILABLE` on all four backends. On PostgreSQL, MongoDB, and Cassandra a concurrent `ListBackups` never saw it `CREATING` *(earlier build)*; on SQLite the backup row is visible as `CREATING` to other requests while the copy runs. Measured *(earlier build)*: 1.3 s for 60,000 items on PostgreSQL, 1.9 s for 60,000 on SQLite, 0.2 s for 20,000 on MongoDB, 5.1 s for 20,000 on Cassandra. MongoDB reports `BackupSizeBytes` 0. The name checks are the service's, with its texts (the length clause before the pattern clause when both fail; the service's order was not captured), counting characters, with `TableName` taking the deployment's table-name limits; before them a 200 KB name was stored and returned in every `ListBackups` page (a 500 on Cassandra) and a NUL byte was a 500 on PostgreSQL. `TableName` as an ARN (`arn:aws:dynamodb:::table/`) is accepted on all four backends: another region's resolves, another account's is `AccessDeniedException` (neither measured on the service); an SDK built from the model refuses the ARN form client-side, so only a hand-built request sends it. `TableNotFoundException` "Table not found: " for a missing or `CREATING` table on all four backends, built from the request's name (the backends' own payloads differ) | diff --git a/docs/getting-started.md b/docs/getting-started.md index 2f22f158a..ac0c3c549 100755 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -210,7 +210,7 @@ You should see all checks pass: --- Checking catalog connection... OK: Connected to catalog. --- Checking catalog version... - OK: Catalog version 0.0.4 + OK: Catalog version 0.0.5 --- Checking data database... OK: Connected to data database 'extenddb_catalog_data'. --- Enumerating tables... @@ -226,7 +226,7 @@ extenddb runs as a daemon (background process) and logs to syslog. On startup it ```bash ./target/release/extenddb serve --config extenddb.toml -# extenddb 0.1.14 (catalog 0.0.4) starting on 127.0.0.1:18443 +# extenddb 0.1.14 (catalog 0.0.5) starting on 127.0.0.1:18443 # storage: postgres (postgresql://extenddb:***@localhost:5432/extenddb_catalog) ``` @@ -1305,7 +1305,7 @@ Each runner requires its tools to be installed. The runner checks prerequisites ```bash ./target/release/extenddb version # extenddb 0.1.14 -# catalog 0.0.4 (postgres) +# catalog 0.0.5 (postgres) # commit abc1234 # built 2026-04-17T12:00:00Z ``` diff --git a/docs/manuals/01-architecture-guide.md b/docs/manuals/01-architecture-guide.md index c709f3de5..749dde5f5 100755 --- a/docs/manuals/01-architecture-guide.md +++ b/docs/manuals/01-architecture-guide.md @@ -172,7 +172,7 @@ extenddb uses a dual-database architecture: - **Catalog database** (e.g., `extenddb_catalog`): Stores table metadata, account/user/group/role/policy definitions, access keys, settings, stream metadata, and metrics. Shared across all accounts. - **Data database** (e.g., `extenddb_catalog_data`): Stores user items, GSI/LSI data, and stream records. Each table gets its own PostgreSQL table. -The catalog version is 0.0.4 on PostgreSQL, SQLite, and Cassandra. PostgreSQL and SQLite store it in the `settings` table under the key `catalog_version` and check it at startup; Cassandra has no startup check (see the Upgrade Manual). +The catalog version is 0.0.5 on PostgreSQL and 0.0.4 on SQLite and Cassandra. PostgreSQL and SQLite store it in the `settings` table under the key `catalog_version` and check it at startup; Cassandra has no startup check (see the Upgrade Manual). The MongoDB backend tracks its own catalog version, 0.0.2. A mismatch between the compiled-in version and the stored one prevents a PostgreSQL or SQLite server from starting; run `extenddb migrate` to upgrade. diff --git a/docs/manuals/04-quickstart-setup-guide.md b/docs/manuals/04-quickstart-setup-guide.md index 0f57420d8..2bed0dc6a 100755 --- a/docs/manuals/04-quickstart-setup-guide.md +++ b/docs/manuals/04-quickstart-setup-guide.md @@ -130,7 +130,7 @@ Check the version: ```bash ./target/release/extenddb version # extenddb 0.1.14 -# catalog 0.0.4 (postgres) +# catalog 0.0.5 (postgres) # commit abc1234 # built 2026-04-17T12:00:00Z ``` @@ -171,7 +171,7 @@ Expected output: --- Checking catalog connection... OK: Connected to catalog. --- Checking catalog version... - OK: Catalog version 0.0.4 + OK: Catalog version 0.0.5 --- Checking data database... OK: Connected to data database 'extenddb_catalog_data'. --- Enumerating tables... diff --git a/docs/manuals/05-admin-guide.md b/docs/manuals/05-admin-guide.md index bd74efb12..974aea347 100755 --- a/docs/manuals/05-admin-guide.md +++ b/docs/manuals/05-admin-guide.md @@ -906,7 +906,7 @@ Check that PostgreSQL is running and the connection string in `extenddb.toml` is ``` -Error: catalog version mismatch: found 1.0.0, expected 0.0.4 +Error: catalog version mismatch: found 1.0.0, expected 0.0.5 ``` Run `extenddb migrate --config extenddb.toml` to upgrade the catalog schema. The check is exact equality in both directions, so this also appears when a binary meets a catalog a newer build already migrated; in that case upgrade the binary rather than the catalog. `extenddb migrate` refuses a catalog newer than the binary (`catalog version X is newer than this binary's Y`) instead of stamping the older version onto it. See the Upgrade Manual for the version history and the stop / migrate / start sequence. diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index 23c1cb6f5..329b877b6 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -4,9 +4,10 @@ ## Current Status -Catalog 0.0.4 is current. **Every existing PostgreSQL, SQLite, and Cassandra deployment must run `extenddb migrate`** to reach it. PostgreSQL and SQLite servers refuse to start against a catalog version they were not built for; a Cassandra server starts anyway and serves tables and items, but CreateBackup, RestoreTableFromBackup, and DeleteBackup are refused with a `ValidationException` naming `extenddb migrate --yes` until the catalog is migrated (see [Cassandra](#cassandra)). MongoDB's catalog version does not change in this release. +Catalog 0.0.5 is current on PostgreSQL; 0.0.4 is current on SQLite and Cassandra. **Every existing PostgreSQL deployment must run `extenddb migrate` with every server stopped** to reach 0.0.5 (see [Catalog 0.0.5](#catalog-005-postgresql-current)). **Every existing PostgreSQL, SQLite, and Cassandra deployment must run `extenddb migrate`** to reach 0.0.4. + PostgreSQL and SQLite servers refuse to start against a catalog version they were not built for; a Cassandra server starts anyway and serves tables and items, but CreateBackup, RestoreTableFromBackup, and DeleteBackup are refused with a `ValidationException` naming `extenddb migrate --yes` until the catalog is migrated (see [Cassandra](#cassandra)). MongoDB's catalog version does not change in this release. -See [Catalog 0.0.4](#catalog-004-current) below for what changes and the exact sequence. +See [Catalog 0.0.4](#catalog-004) below for what changes and the exact sequence. ## How Catalog Upgrades Work @@ -202,7 +203,28 @@ To discard data written after the upgrade as well, restore `` from its ## Version History -### Catalog 0.0.4 (Current) +### Catalog 0.0.5 (PostgreSQL, Current) + +Changes no schema. It marks a change in the stored form of the seven catalog columns that carry attribute names: `tables.key_schema`, `tables.attribute_definitions`, `indexes.key_schema`, `indexes.projection`, `backups.key_schema`, `backups.attribute_definitions` and `backup_definitions.definition`. From this release those documents are written through an order-preserving escape (U+0000 stored as U+0001 U+0001, U+0001 stored as U+0001 U+0002), so a key attribute name, an index key attribute name or a projected attribute name may contain U+0000, as it may on DynamoDB. PostgreSQL `jsonb` cannot hold U+0000, which is why the escape exists. The migration `004_escape_catalog_control_chars.sql` re-encodes every stored document that holds a raw U+0001 once, before it moves the version, so a name an earlier release stored raw reads back unchanged. A document without U+0001 is left as it is; on most deployments the migration rewrites no rows. + +The re-encode is not repeatable: an escaped U+0001 and a raw U+0001 pair are the same bytes, so `extenddb migrate --yes` refuses to run it while any other session is connected to the catalog database, naming the sessions (`N other connection(s) hold the catalog database: ...`). Stop every ExtendDB server first. If the only other sessions are idle ones held by a connection pooler, set `EXTENDDB_MIGRATE_IGNORE_CONNECTIONS=1` to skip the check. The check leaves out sessions whose `application_name` is `extenddb-migrate`, the name the migrate command sets on its own connections, so a server's configured connection string must not carry that name. Each table is rewritten in its own transaction with a progress marker, so a run interrupted by a crash resumes with the tables not yet done. `extenddb init` records the migration on a fresh catalog. + +```bash +extenddb stop --config extenddb.toml # every server, every host +pg_dump -h -U > catalog_backup_$(date +%Y%m%d).sql +export EXTENDDB_PG_PASSWORD='' +extenddb migrate --config extenddb.toml --pg-user # exit 3, prints the plan +extenddb migrate --config extenddb.toml --pg-user --yes # re-encodes, applies 004 +extenddb serve --config extenddb.toml +``` + +The preview reports `Pending: catalog 0.0.4 -> 0.0.5; data migrations [catalog 004_escape_catalog_control_chars.sql].` and exits 3. + +Downgrade: after this release, a table whose key attribute, index key or projected attribute names contain U+0000 or U+0001 on PostgreSQL is not readable by an earlier release. An earlier binary refuses to start against a 0.0.5 catalog (`Catalog version mismatch: expected 0.0.4, found 0.0.5`); if it were started against the data anyway it would read the escaped form as literal data (`pk` for a name `pk`) and `GetItem` by the real name would answer `Missing the key ...`. Roll back by restoring the catalog saved before the upgrade. Names without those two characters are stored as before and are unaffected. + +SQLite, MongoDB and Cassandra store attribute names without an escape and keep their catalog versions in this release. + +### Catalog 0.0.4 Adds the table definition a backup records and the registration of a running restore: diff --git a/docs/manuals/08-install-linux.md b/docs/manuals/08-install-linux.md index 7414ab006..cff9a63cd 100755 --- a/docs/manuals/08-install-linux.md +++ b/docs/manuals/08-install-linux.md @@ -118,7 +118,7 @@ Expected: ``` === extenddb verify === ... - OK: Catalog version 0.0.4 + OK: Catalog version 0.0.5 ... === HEALTHY: All checks passed === ``` diff --git a/docs/manuals/09-install-macos.md b/docs/manuals/09-install-macos.md index feec0a409..aae4b95c9 100755 --- a/docs/manuals/09-install-macos.md +++ b/docs/manuals/09-install-macos.md @@ -96,7 +96,7 @@ Expected: ``` === extenddb verify === ... - OK: Catalog version 0.0.4 + OK: Catalog version 0.0.5 ... === HEALTHY: All checks passed === ``` diff --git a/tests/python/conftest.py b/tests/python/conftest.py index 956ea3f53..765294ea5 100755 --- a/tests/python/conftest.py +++ b/tests/python/conftest.py @@ -27,7 +27,13 @@ import pytest import urllib3 -from helpers import make_dynamodb_client, unique_name, wait_for_active, wait_for_deleted # noqa: F401 +from helpers import ( # noqa: F401 + make_dynamodb_client, + make_raw_client, + unique_name, + wait_for_active, + wait_for_deleted, +) # Suppress InsecureRequestWarning for self-signed TLS certs. urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) @@ -92,6 +98,19 @@ def dynamodb_resource(endpoint_url: str | None): return boto3.resource(**kwargs) +@pytest.fixture(scope="session") +def raw_client(endpoint_url: str | None): + """Session-scoped client with SDK parameter validation off; see + `helpers.make_raw_client`.""" + return make_raw_client(endpoint_url) + + +@pytest.fixture(scope="session") +def raw_streams_client(endpoint_url: str | None): + """The streams API counterpart of `raw_client`.""" + return make_raw_client(endpoint_url, "dynamodbstreams") + + @pytest.fixture() def table_factory(dynamodb_client): """Factory fixture: creates tables and cleans them up after the test. diff --git a/tests/python/helpers.py b/tests/python/helpers.py index f4ed85479..355967070 100755 --- a/tests/python/helpers.py +++ b/tests/python/helpers.py @@ -71,3 +71,46 @@ def wait_for_deleted(client, table_name: str, timeout: float = 60.0) -> None: def unique_name(prefix: str = "test") -> str: """Generate a unique name for test isolation.""" return f"{prefix}-{uuid.uuid4().hex[:12]}" + + +def make_raw_client(endpoint_url: str | None, service_name: str = "dynamodb"): + """A boto3 client with SDK parameter validation off and retries off, so the + service, not the SDK, decides what a request is answered with. Used by the + tests that send names and ARNs the SDK's model would refuse client-side.""" + kwargs: dict = { + "service_name": service_name, + "region_name": os.environ.get("AWS_DEFAULT_REGION", "us-east-1"), + "config": botocore.config.Config( + parameter_validation=False, retries={"max_attempts": 0} + ), + } + if endpoint_url: + kwargs["endpoint_url"] = endpoint_url + if endpoint_url.startswith("https://"): + ca_cert = os.environ.get("EXTENDDB_CA_CERT", "") + kwargs["verify"] = ca_cert if ca_cert else False + return boto3.client(**kwargs) + + +MEMBER_PATTERN = "Member must satisfy regular expression pattern: [a-zA-Z0-9_.-]+" + + +def assert_validation(exc, message: str) -> None: + """A 400 ValidationException with exactly this message.""" + resp = exc.response + assert resp["ResponseMetadata"]["HTTPStatusCode"] == 400, resp + assert resp["Error"]["Code"] == "ValidationException", resp["Error"] + assert resp["Error"]["Message"] == message, resp["Error"]["Message"] + + +def assert_member_pattern(exc, location: str, value: str) -> None: + """The service's one-constraint member pattern sentence, byte for byte. + + The offending value is rendered inside ``Value '...'`` with a U+0000 as the + raw character (the JSON body carries ``\\u0000``, which the SDK decodes). + """ + assert_validation( + exc, + f"1 validation error detected: Value '{value}' at '{location}' " + f"failed to satisfy constraint: {MEMBER_PATTERN}", + ) diff --git a/tests/python/test_nul_arn_shapes.py b/tests/python/test_nul_arn_shapes.py new file mode 100644 index 000000000..9aaa86128 --- /dev/null +++ b/tests/python/test_nul_arn_shapes.py @@ -0,0 +1,164 @@ +"""ARN-shaped values with U+0000 in `TableName`, in nested table names, and in +the stream, backup and tag ARN fields. + +Measured against the service on 2026-10-24 with SDK parameter validation off. +A plain table ARN whose table segment fails the pattern gets the `Table name +of ARN` sentence; an ARN that is not a plain table ARN gets the `Valid ARN +format is ...` frame, with the constraint replaced by `Invalid subscriber ID` +or `Invalid AWS region` when that segment is the bad one. The ARNs below are +built from a table the test creates, so the account and region are the +caller's on either target. +""" + +import pytest +from botocore.exceptions import ClientError + +from helpers import assert_validation, unique_name, wait_for_active, wait_for_deleted + +NUL = "\u0000" +FRAME = ( + "Valid ARN format is " + "'arn:::::resourceType/resourceName', " + "where Resource name must have length less than or equal to 255, " + "Resource name must have length greater than or equal to 3, " + "Resource name must satisfy regular expression pattern: [a-zA-Z0-9_.-]+" +) +TABLE_OF_ARN = "Table name of ARN must satisfy regular expression pattern: [a-zA-Z0-9_.-]+" +MAP_KEYS_ARN = ( + "Map keys must satisfy constraint: [Valid resource name or resource arn must be given. " + "Valid resource arn format is " + "'arn:::::resourceType/resourceName', " + "where Resource name must have length less than or equal to 255, " + "Resource name must have length greater than or equal to 3, " + "Resource name must satisfy regular expression pattern: [a-zA-Z0-9_.-]+]" +) + + +def _at_table_name(exc: ClientError, value: str, constraint: str) -> None: + assert_validation( + exc, + f"1 validation error detected: Value '{value}' at 'tableName' failed to satisfy " + f"constraint: {constraint}", + ) + + +@pytest.fixture() +def arns(raw_client): + """The created table's name and the parts of its ARN.""" + name = unique_name("nula") + raw_client.create_table( + TableName=name, + BillingMode="PAY_PER_REQUEST", + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}], + ) + wait_for_active(raw_client, name) + arn = raw_client.describe_table(TableName=name)["Table"]["TableArn"] + # arn:aws:dynamodb:::table/ + _, partition, service, region, account, _ = arn.split(":", 5) + yield { + "name": name, + "prefix": f"arn:{partition}:{service}:{region}:{account}:table/", + "region": region, + "account": account, + "bad_table": f"arn:{partition}:{service}:{region}:{account}:table/t367{NUL}name", + } + try: + raw_client.delete_table(TableName=name) + wait_for_deleted(raw_client, name) + except Exception: + pass + + +class TestArnInTableName: + def test_plain_table_arn_with_nul_segment(self, raw_client, arns): + bad = arns["bad_table"] + for call in [ + lambda: raw_client.describe_table(TableName=bad), + lambda: raw_client.put_item(TableName=bad, Item={"pk": {"S": "a"}}), + ]: + with pytest.raises(ClientError) as exc: + call() + _at_table_name(exc.value, bad, TABLE_OF_ARN) + + def test_foreign_service_arn_on_item_operations_gets_the_frame(self, raw_client): + arn = f"arn:aws:s3:::t367{NUL}name" + with pytest.raises(ClientError) as exc: + raw_client.put_item(TableName=arn, Item={"pk": {"S": "a"}}) + _at_table_name(exc.value, arn, FRAME) + with pytest.raises(ClientError) as exc: + raw_client.get_item(TableName=arn, Key={"pk": {"S": "a"}}) + _at_table_name(exc.value, arn, FRAME) + # DescribeTable answers AccessDeniedException on the service and the + # engine's own ValidationException on ExtendDB; neither is a 500, and + # the row is not asserted beyond that. + with pytest.raises(ClientError) as exc: + raw_client.describe_table(TableName=arn) + assert exc.value.response["ResponseMetadata"]["HTTPStatusCode"] == 400 + + def test_bad_account_and_region_segments_name_their_constraint(self, raw_client, arns): + acct = arns["account"] + region = arns["region"] + arn = f"arn:aws:dynamodb:{region}:{acct[:4]}{NUL}{acct[4:]}:table/t367name" + with pytest.raises(ClientError) as exc: + raw_client.put_item(TableName=arn, Item={"pk": {"S": "a"}}) + _at_table_name(exc.value, arn, "Invalid subscriber ID") + arn = f"arn:aws:dynamodb:{region[:5]}{NUL}{region[5:]}:{acct}:table/t367name" + with pytest.raises(ClientError) as exc: + raw_client.put_item(TableName=arn, Item={"pk": {"S": "a"}}) + _at_table_name(exc.value, arn, "Invalid AWS region") + + def test_index_and_stream_arns_get_the_frame(self, raw_client, arns): + index = f"{arns['prefix']}{arns['name']}/index/i{NUL}x" + with pytest.raises(ClientError) as exc: + raw_client.query( + TableName=index, + KeyConditionExpression="pk = :v", + ExpressionAttributeValues={":v": {"S": "a"}}, + ) + _at_table_name(exc.value, index, FRAME) + for arn in [ + f"{arns['bad_table']}/index/ix", + f"{arns['bad_table']}/stream/2026-10-24T00:00:00.000", + ]: + with pytest.raises(ClientError) as exc: + raw_client.describe_table(TableName=arn) + _at_table_name(exc.value, arn, FRAME) + + +class TestNestedArns: + def test_batch_request_items_key_as_arn_with_nul_segment(self, raw_client, arns): + bad = arns["bad_table"] + for call in [ + lambda: raw_client.batch_write_item( + RequestItems={bad: [{"PutRequest": {"Item": {"pk": {"S": "a"}}}}]} + ), + lambda: raw_client.batch_get_item(RequestItems={bad: {"Keys": [{"pk": {"S": "a"}}]}}), + ]: + with pytest.raises(ClientError) as exc: + call() + resp = exc.value.response + assert resp["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert resp["Error"]["Code"] == "ValidationException" + msg = resp["Error"]["Message"] + # The service renders its own dump of the whole map inside + # Value '...'; the sentence from `at 'requestItems'` onward is + # compared byte for byte and the key must appear in the value. + assert msg.startswith("1 validation error detected: Value '"), msg + assert msg.endswith( + f"' at 'requestItems' failed to satisfy constraint: {MAP_KEYS_ARN}" + ), msg + value = msg[len("1 validation error detected: Value '") : msg.rindex("' at 'requestItems'")] + assert bad.replace(NUL, "") in value.replace(NUL, "").replace("\\u0000", ""), msg + + def test_transact_write_items_put_table_name_as_arn(self, raw_client, arns): + bad = arns["bad_table"] + with pytest.raises(ClientError) as exc: + raw_client.transact_write_items( + TransactItems=[{"Put": {"TableName": bad, "Item": {"pk": {"S": "a"}}}}] + ) + assert_validation( + exc.value, + f"1 validation error detected: Value '{bad}' at " + f"'transactItems.1.member.put.tableName' failed to satisfy constraint: {TABLE_OF_ARN}", + ) diff --git a/tests/python/test_nul_metadata_names.py b/tests/python/test_nul_metadata_names.py new file mode 100644 index 000000000..3772a39a3 --- /dev/null +++ b/tests/python/test_nul_metadata_names.py @@ -0,0 +1,403 @@ +"""U+0000 in CreateTable names: key attribute names, index names, table names. + +DynamoDB accepts U+0000 inside a key schema attribute name (the table's or a +secondary index's) and DescribeTable returns the name byte-identical. It +refuses U+0000 inside an index name or a table name with the member pattern +ValidationException. Measured against the service on 2026-09-18 with SDK +parameter validation off, so the service, not the SDK, decided. + +The table-name row is checked on every operation that carries a bare +TableName, since the name is used before any handler runs. + +Order against authorization (measured 2026-10-24, not encoded as a test): a +principal with no DynamoDB permission gets the same tableName pattern +ValidationException for DescribeTable, PutItem and Scan with a U+0000 name +(and the requestItems sentence for BatchWriteItem), and AccessDeniedException +for DescribeTable on a real table. The pattern check precedes the IAM decision. +This suite has no denied-principal fixture against the service, so the row is +recorded here as a comment; the server places the check before authorization +(crates/server/src/table_name_gate.rs). +""" + +import pytest +from botocore.exceptions import ClientError + +from helpers import assert_member_pattern, unique_name, wait_for_active, wait_for_deleted + +NUL = "\u0000" +PATTERN = "Member must satisfy regular expression pattern: [a-zA-Z0-9_.-]+" +MAP_KEYS = ( + "Map keys must satisfy constraint: [Member must have length less than or equal to 255, " + "Member must have length greater than or equal to 3, " + "Member must satisfy regular expression pattern: [a-zA-Z0-9_.-]+]" +) + + +@pytest.fixture() +def raw_table_factory(raw_client): + created: list[str] = [] + + def _create(**kwargs) -> str: + name = kwargs.pop("TableName", None) or unique_name("nul") + raw_client.create_table(TableName=name, BillingMode="PAY_PER_REQUEST", **kwargs) + created.append(name) + wait_for_active(raw_client, name) + return name + + yield _create + + for name in created: + try: + raw_client.delete_table(TableName=name) + wait_for_deleted(raw_client, name) + except Exception: + pass + + +def _validation_error(exc: ClientError, location: str, value: str) -> None: + assert_member_pattern(exc, location, value) + + +class TestNulInKeyAttributeNames: + def test_partition_key_attribute_name_with_nul_is_stored_and_returned( + self, raw_client, raw_table_factory + ): + pk = f"p{NUL}k" + name = raw_table_factory( + KeySchema=[{"AttributeName": pk, "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": pk, "AttributeType": "S"}], + ) + table = raw_client.describe_table(TableName=name)["Table"] + assert table["TableStatus"] == "ACTIVE" + assert table["KeySchema"] == [{"AttributeName": pk, "KeyType": "HASH"}] + assert table["AttributeDefinitions"] == [{"AttributeName": pk, "AttributeType": "S"}] + + def test_gsi_key_attribute_name_with_nul_is_stored_and_returned( + self, raw_client, raw_table_factory + ): + gk = f"g{NUL}k" + name = raw_table_factory( + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": gk, "AttributeType": "S"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) + table = raw_client.describe_table(TableName=name)["Table"] + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["IndexName"] == "gsi1" + assert gsi["KeySchema"] == [{"AttributeName": gk, "KeyType": "HASH"}] + defs = {d["AttributeName"]: d["AttributeType"] for d in table["AttributeDefinitions"]} + assert defs == {"pk": "S", gk: "S"} + + def test_gsi_on_nul_attribute_can_be_removed_and_added_again( + self, raw_client, raw_table_factory + ): + gk = f"g{NUL}k" + name = raw_table_factory( + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": gk, "AttributeType": "S"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) + raw_client.update_table( + TableName=name, + GlobalSecondaryIndexUpdates=[{"Delete": {"IndexName": "gsi1"}}], + ) + wait_for_active(raw_client, name) + table = raw_client.describe_table(TableName=name)["Table"] + assert not table.get("GlobalSecondaryIndexes") + raw_client.update_table( + TableName=name, + AttributeDefinitions=[{"AttributeName": gk, "AttributeType": "S"}], + GlobalSecondaryIndexUpdates=[ + { + "Create": { + "IndexName": "gsi2", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + } + ], + ) + wait_for_active(raw_client, name) + table = raw_client.describe_table(TableName=name)["Table"] + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["IndexName"] == "gsi2" + assert gsi["KeySchema"] == [{"AttributeName": gk, "KeyType": "HASH"}] + defs = {d["AttributeName"]: d["AttributeType"] for d in table["AttributeDefinitions"]} + assert defs == {"pk": "S", gk: "S"} + + def test_lsi_key_and_gsi_projected_attribute_names_with_nul_are_stored_and_returned( + self, raw_client, raw_table_factory + ): + """An LSI keyed on a U+0000 attribute and a GSI whose NonKeyAttributes + entry contains U+0000: 200, both returned byte-identical (measured + 2026-10-24).""" + lk = f"l{NUL}k" + nk = f"n{NUL}k" + name = raw_table_factory( + KeySchema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "sk", "AttributeType": "S"}, + {"AttributeName": "gk", "AttributeType": "S"}, + {"AttributeName": lk, "AttributeType": "S"}, + ], + LocalSecondaryIndexes=[ + { + "IndexName": "lsi1", + "KeySchema": [ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": lk, "KeyType": "RANGE"}, + ], + "Projection": {"ProjectionType": "ALL"}, + } + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": "gk", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": [nk]}, + } + ], + ) + table = raw_client.describe_table(TableName=name)["Table"] + lsi = table["LocalSecondaryIndexes"][0] + assert lsi["KeySchema"] == [ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": lk, "KeyType": "RANGE"}, + ] + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["Projection"] == {"ProjectionType": "INCLUDE", "NonKeyAttributes": [nk]} + defs = {d["AttributeName"]: d["AttributeType"] for d in table["AttributeDefinitions"]} + assert defs == {"pk": "S", "sk": "S", "gk": "S", lk: "S"} + + +class TestNulInNames: + def test_gsi_name_with_nul_is_refused(self, raw_client): + gsi_name = f"gsi{NUL}1" + with pytest.raises(ClientError) as exc: + raw_client.create_table( + TableName=unique_name("nul"), + BillingMode="PAY_PER_REQUEST", + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "gk", "AttributeType": "S"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": gsi_name, + "KeySchema": [{"AttributeName": "gk", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) + resp = exc.value.response + assert resp["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert resp["Error"]["Code"] == "ValidationException" + assert "indexName" in resp["Error"]["Message"] + assert PATTERN in resp["Error"]["Message"] + + def test_create_table_with_nul_in_table_name_is_refused(self, raw_client): + bad = f"nul{NUL}table" + with pytest.raises(ClientError) as exc: + raw_client.create_table( + TableName=bad, + BillingMode="PAY_PER_REQUEST", + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}], + ) + _validation_error(exc.value, "tableName", bad) + + @pytest.mark.parametrize( + "call", + [ + pytest.param(lambda c, t: c.describe_table(TableName=t), id="DescribeTable"), + pytest.param(lambda c, t: c.delete_table(TableName=t), id="DeleteTable"), + pytest.param( + lambda c, t: c.update_table(TableName=t, BillingMode="PAY_PER_REQUEST"), + id="UpdateTable", + ), + pytest.param( + lambda c, t: c.put_item(TableName=t, Item={"pk": {"S": "a"}}), id="PutItem" + ), + pytest.param( + lambda c, t: c.get_item(TableName=t, Key={"pk": {"S": "a"}}), id="GetItem" + ), + pytest.param( + lambda c, t: c.update_item( + TableName=t, + Key={"pk": {"S": "a"}}, + UpdateExpression="SET v = :v", + ExpressionAttributeValues={":v": {"S": "a"}}, + ), + id="UpdateItem", + ), + pytest.param( + lambda c, t: c.delete_item(TableName=t, Key={"pk": {"S": "a"}}), + id="DeleteItem", + ), + pytest.param( + lambda c, t: c.query( + TableName=t, + KeyConditionExpression="pk = :v", + ExpressionAttributeValues={":v": {"S": "a"}}, + ), + id="Query", + ), + pytest.param(lambda c, t: c.scan(TableName=t), id="Scan"), + ], + ) + def test_operations_with_nul_in_table_name_are_refused(self, raw_client, call): + bad = f"nul{NUL}table" + with pytest.raises(ClientError) as exc: + call(raw_client, bad) + _validation_error(exc.value, "tableName", bad) + + @pytest.mark.parametrize("name", ["a b"], ids=["space"]) + def test_query_and_scan_with_other_bad_characters_are_refused(self, raw_client, name): + """Same sentence for any character outside the pattern (measured with a space).""" + with pytest.raises(ClientError) as exc: + raw_client.scan(TableName=name) + _validation_error(exc.value, "tableName", name) + with pytest.raises(ClientError) as exc: + raw_client.query( + TableName=name, + KeyConditionExpression="pk = :v", + ExpressionAttributeValues={":v": {"S": "a"}}, + ) + _validation_error(exc.value, "tableName", name) + + +class TestNulInNestedTableNames: + """Table names carried inside RequestItems or TransactItems. + + Measured 2026-10-24. For the batch operations the service renders its own + dump of the whole RequestItems map inside ``Value '...'`` (a Java-style + object dump for BatchWriteItem, the JSON of the map for BatchGetItem), so + the value is not compared byte for byte: the sentence from + ``at 'requestItems'`` onward is, and the offending key must appear in the + value (with the U+0000 normalised, since the BatchGetItem dump carries it as + the six characters ``\\u0000``). ExtendDB renders the offending key alone. + """ + + @staticmethod + def _request_items_error(exc: ClientError, key: str) -> None: + resp = exc.response + assert resp["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert resp["Error"]["Code"] == "ValidationException" + msg = resp["Error"]["Message"] + assert msg.startswith("1 validation error detected: Value '"), msg + assert msg.endswith(f"' at 'requestItems' failed to satisfy constraint: {MAP_KEYS}"), msg + value = msg[len("1 validation error detected: Value '") : msg.rindex("' at 'requestItems'")] + assert key.replace(NUL, "") in value.replace(NUL, "").replace("\\u0000", ""), msg + + def test_batch_write_item_with_nul_request_items_key_is_refused(self, raw_client): + bad = f"nul{NUL}table" + with pytest.raises(ClientError) as exc: + raw_client.batch_write_item( + RequestItems={bad: [{"PutRequest": {"Item": {"pk": {"S": "a"}}}}]} + ) + self._request_items_error(exc.value, bad) + + def test_batch_get_item_with_nul_request_items_key_is_refused(self, raw_client): + bad = f"nul{NUL}table" + with pytest.raises(ClientError) as exc: + raw_client.batch_get_item(RequestItems={bad: {"Keys": [{"pk": {"S": "a"}}]}}) + self._request_items_error(exc.value, bad) + + def test_transact_write_items_with_nul_table_name_is_refused(self, raw_client): + bad = f"nul{NUL}table" + with pytest.raises(ClientError) as exc: + raw_client.transact_write_items( + TransactItems=[{"Put": {"TableName": bad, "Item": {"pk": {"S": "a"}}}}] + ) + _validation_error(exc.value, "transactItems.1.member.put.tableName", bad) + + def test_transact_get_items_with_nul_table_name_is_refused(self, raw_client): + bad = f"nul{NUL}table" + with pytest.raises(ClientError) as exc: + raw_client.transact_get_items( + TransactItems=[{"Get": {"TableName": bad, "Key": {"pk": {"S": "a"}}}}] + ) + _validation_error(exc.value, "transactItems.1.member.get.tableName", bad) + + +class TestNulInTableArn: + """A TableName or ResourceArn that is a table ARN whose table segment + contains U+0000 (measured 2026-10-24). The ARN is built in the caller's + own account and region, read from a table the test creates.""" + + @pytest.fixture() + def nul_arn(self, raw_client, raw_table_factory): + name = raw_table_factory( + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}], + ) + real = raw_client.describe_table(TableName=name)["Table"]["TableArn"] + prefix = real[: real.rindex("table/") + len("table/")] + return f"{prefix}nul{NUL}table" + + @staticmethod + def _arn_table_name_error(exc: ClientError, arn: str) -> None: + resp = exc.response + assert resp["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert resp["Error"]["Code"] == "ValidationException" + assert resp["Error"]["Message"] == ( + f"1 validation error detected: Value '{arn}' at 'tableName' failed to satisfy " + "constraint: Table name of ARN must satisfy regular expression pattern: " + "[a-zA-Z0-9_.-]+" + ) + + def test_describe_table_with_nul_arn_segment_is_refused(self, raw_client, nul_arn): + with pytest.raises(ClientError) as exc: + raw_client.describe_table(TableName=nul_arn) + self._arn_table_name_error(exc.value, nul_arn) + + def test_put_item_with_nul_arn_segment_is_refused(self, raw_client, nul_arn): + with pytest.raises(ClientError) as exc: + raw_client.put_item(TableName=nul_arn, Item={"pk": {"S": "a"}}) + self._arn_table_name_error(exc.value, nul_arn) + + @pytest.mark.parametrize( + "call", + [ + pytest.param( + lambda c, a: c.tag_resource(ResourceArn=a, Tags=[{"Key": "k", "Value": "v"}]), + id="TagResource", + ), + pytest.param(lambda c, a: c.untag_resource(ResourceArn=a, TagKeys=["k"]), id="UntagResource"), + pytest.param(lambda c, a: c.list_tags_of_resource(ResourceArn=a), id="ListTagsOfResource"), + ], + ) + def test_tag_operations_with_nul_arn_segment_are_refused(self, raw_client, nul_arn, call): + """Measured on TagResource; UntagResource and ListTagsOfResource share + the ResourceArn shape check and are given the same sentence.""" + with pytest.raises(ClientError) as exc: + call(raw_client, nul_arn) + resp = exc.value.response + assert resp["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert resp["Error"]["Code"] == "ValidationException" + assert resp["Error"]["Message"] == ( + f"Invalid TableArn: Invalid ResourceArn provided as input {nul_arn}" + ) diff --git a/tests/python/test_nul_table_name_fields.py b/tests/python/test_nul_table_name_fields.py new file mode 100644 index 000000000..c10ef7a4e --- /dev/null +++ b/tests/python/test_nul_table_name_fields.py @@ -0,0 +1,267 @@ +"""Table names outside the pattern in every field that carries one, beyond +`TableName` (covered by test_nul_metadata_names.py), and the stream, backup +and tag ARN fields with U+0000 in a segment. + +Measured against the service on 2026-10-24 with SDK parameter validation off. +Every operation here looks its table up before its handler runs, so the name +has to be refused before any lookup; on PostgreSQL a U+0000 cannot even be +bound as a parameter. These rows pin the service's sentence and location for +each field. +""" + +import time + +import pytest +from botocore.exceptions import ClientError + +from helpers import ( + MEMBER_PATTERN, + assert_member_pattern, + assert_validation, + unique_name, + wait_for_active, + wait_for_deleted, +) + +NUL = "\u0000" +BAD = f"t367{NUL}name" + + +@pytest.fixture() +def nul_table(raw_client): + """A plain table with a GSI, and its ARN prefix up to `table/`.""" + name = unique_name("nulf") + raw_client.create_table( + TableName=name, + BillingMode="PAY_PER_REQUEST", + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "gk", "AttributeType": "S"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": "gk", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) + wait_for_active(raw_client, name) + arn = raw_client.describe_table(TableName=name)["Table"]["TableArn"] + yield name, arn[: arn.rindex("table/") + len("table/")] + try: + raw_client.delete_table(TableName=name) + wait_for_deleted(raw_client, name) + except Exception: + pass + + +class TestTableNameFieldsBeyondTableName: + def test_restore_table_from_backup_target_table_name(self, raw_client, nul_table): + name, _ = nul_table + # The service refuses CreateBackup for a short while after CreateTable + # (ContinuousBackupsUnavailableException); retry as the backup tests do. + deadline = time.monotonic() + 120 + while True: + try: + arn = raw_client.create_backup(TableName=name, BackupName="bk367")["BackupDetails"]["BackupArn"] + break + except ClientError as e: + if e.response["Error"]["Code"] != "ContinuousBackupsUnavailableException" or time.monotonic() >= deadline: + raise + time.sleep(2) + try: + for bad in [BAD, "a b"]: + with pytest.raises(ClientError) as exc: + raw_client.restore_table_from_backup(BackupArn=arn, TargetTableName=bad) + assert_member_pattern(exc.value, "targetTableName", bad) + finally: + try: + raw_client.delete_backup(BackupArn=arn) + except Exception: + pass + + def test_restore_table_to_point_in_time_names(self, raw_client, nul_table): + name, _ = nul_table + with pytest.raises(ClientError) as exc: + raw_client.restore_table_to_point_in_time( + SourceTableName=BAD, TargetTableName=unique_name("tgt"), UseLatestRestorableTime=True + ) + assert_member_pattern(exc.value, "sourceTableName", BAD) + with pytest.raises(ClientError) as exc: + raw_client.restore_table_to_point_in_time( + SourceTableName=name, TargetTableName=BAD, UseLatestRestorableTime=True + ) + assert_member_pattern(exc.value, "targetTableName", BAD) + + def test_import_table_creation_parameters_table_name(self, raw_client): + with pytest.raises(ClientError) as exc: + raw_client.import_table( + S3BucketSource={"S3Bucket": "bucket"}, + InputFormat="DYNAMODB_JSON", + TableCreationParameters={ + "TableName": BAD, + "KeySchema": [{"AttributeName": "pk", "KeyType": "HASH"}], + "AttributeDefinitions": [{"AttributeName": "pk", "AttributeType": "S"}], + "BillingMode": "PAY_PER_REQUEST", + }, + ) + assert_member_pattern(exc.value, "tableCreationParameters.tableName", BAD) + + def test_list_tables_exclusive_start_table_name(self, raw_client): + with pytest.raises(ClientError) as exc: + raw_client.list_tables(ExclusiveStartTableName=BAD) + assert_member_pattern(exc.value, "exclusiveStartTableName", BAD) + + @pytest.mark.parametrize( + "call", + [ + pytest.param(lambda c, t: c.list_backups(TableName=t), id="ListBackups"), + pytest.param( + lambda c, t: c.create_backup(TableName=t, BackupName="bk367"), id="CreateBackup" + ), + pytest.param(lambda c, t: c.describe_time_to_live(TableName=t), id="DescribeTimeToLive"), + pytest.param( + lambda c, t: c.update_time_to_live( + TableName=t, TimeToLiveSpecification={"Enabled": True, "AttributeName": "ttl"} + ), + id="UpdateTimeToLive", + ), + pytest.param( + lambda c, t: c.describe_continuous_backups(TableName=t), + id="DescribeContinuousBackups", + ), + pytest.param( + lambda c, t: c.update_continuous_backups( + TableName=t, + PointInTimeRecoverySpecification={"PointInTimeRecoveryEnabled": True}, + ), + id="UpdateContinuousBackups", + ), + ], + ) + def test_other_operations_with_a_bare_table_name(self, raw_client, call): + with pytest.raises(ClientError) as exc: + call(raw_client, BAD) + assert_member_pattern(exc.value, "tableName", BAD) + + def test_list_streams_has_its_own_sentence(self, raw_streams_client): + with pytest.raises(ClientError) as exc: + raw_streams_client.list_streams(TableName=BAD) + assert_validation( + exc.value, + "TableName must be at least 3 characters long and at most 255 characters long", + ) + + +class TestSeveralConstraintsAtOnce: + """A name that fails the pattern and a length bound lists both clauses, + the length first, under `2 validation errors detected:`.""" + + @pytest.mark.parametrize( + ("name", "length_clause"), + [ + pytest.param(NUL, "Member must have length greater than or equal to 3", id="nul-alone"), + pytest.param(f"a{NUL}", "Member must have length greater than or equal to 3", id="two"), + pytest.param( + "x" * 300 + NUL, "Member must have length less than or equal to 255", id="301" + ), + ], + ) + def test_describe_table(self, raw_client, name, length_clause): + with pytest.raises(ClientError) as exc: + raw_client.describe_table(TableName=name) + assert_validation( + exc.value, + f"2 validation errors detected: Value '{name}' at 'tableName' failed to satisfy " + f"constraint: {length_clause}; Value '{name}' at 'tableName' failed to satisfy " + f"constraint: {MEMBER_PATTERN}", + ) + + +class TestIndexName: + def test_query_and_scan_with_nul_index_name(self, raw_client, nul_table): + name, _ = nul_table + bad = f"g{NUL}2" + with pytest.raises(ClientError) as exc: + raw_client.query( + TableName=name, + IndexName=bad, + KeyConditionExpression="gk = :v", + ExpressionAttributeValues={":v": {"S": "a"}}, + ) + assert_member_pattern(exc.value, "indexName", bad) + with pytest.raises(ClientError) as exc: + raw_client.scan(TableName=name, IndexName=bad) + assert_member_pattern(exc.value, "indexName", bad) + + def test_update_table_create_and_delete_with_nul_index_name(self, raw_client, nul_table): + name, _ = nul_table + bad = f"g{NUL}2" + with pytest.raises(ClientError) as exc: + raw_client.update_table( + TableName=name, + AttributeDefinitions=[{"AttributeName": "gk", "AttributeType": "S"}], + GlobalSecondaryIndexUpdates=[ + { + "Create": { + "IndexName": bad, + "KeySchema": [{"AttributeName": "gk", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + } + ], + ) + assert_member_pattern( + exc.value, "globalSecondaryIndexUpdates.1.member.create.indexName", bad + ) + with pytest.raises(ClientError) as exc: + raw_client.update_table( + TableName=name, GlobalSecondaryIndexUpdates=[{"Delete": {"IndexName": bad}}] + ) + assert_member_pattern( + exc.value, "globalSecondaryIndexUpdates.1.member.delete.indexName", bad + ) + + +class TestStreamBackupAndTagArns: + def test_stream_arns_with_nul(self, raw_client, raw_streams_client, nul_table): + name, prefix = nul_table + bad = f"{prefix}t367{NUL}name" + table_seg = f"{bad}/stream/2026-10-24T00:00:00.000" + # Measured 2026-10-24: a U+0000 inside the label answers Invalid StreamArn; + # a U+0000 after a complete label reads as a label and answers + # ResourceNotFoundException, so the character goes inside. + label = f"{prefix}{name}/stream/2026-10-24T00:00{NUL}:00.000" + for arn in [table_seg, label]: + with pytest.raises(ClientError) as exc: + raw_streams_client.describe_stream(StreamArn=arn) + assert_validation(exc.value, "Invalid StreamArn") + with pytest.raises(ClientError) as exc: + raw_streams_client.get_shard_iterator( + StreamArn=table_seg, ShardId="shardId-00000001", ShardIteratorType="TRIM_HORIZON" + ) + assert_validation(exc.value, "Invalid StreamArn") + with pytest.raises(ClientError) as exc: + raw_client.tag_resource(ResourceArn=table_seg, Tags=[{"Key": "k", "Value": "v"}]) + assert_validation(exc.value, "Invalid StreamArn") + + def test_backup_arns_with_nul(self, raw_client, nul_table): + name, prefix = nul_table + bad = f"{prefix}t367{NUL}name" + table_seg = f"{bad}/backup/01700000000000-abcdef12" + for call in [ + lambda: raw_client.describe_backup(BackupArn=table_seg), + lambda: raw_client.delete_backup(BackupArn=table_seg), + lambda: raw_client.restore_table_from_backup( + BackupArn=table_seg, TargetTableName=unique_name("tgt") + ), + ]: + with pytest.raises(ClientError) as exc: + call() + assert_validation(exc.value, "Invalid Backup ARN") + backup_id = f"{prefix}{name}/backup/0170000000{NUL}0000-abcdef12" + with pytest.raises(ClientError) as exc: + raw_client.describe_backup(BackupArn=backup_id) + assert_validation(exc.value, "Invalid Request: BackupArn is not valid") From 9cd57be9632d73f2f60795398ce59d6be7fa18e4 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sun, 25 Oct 2026 00:22:30 +0000 Subject: [PATCH 2/2] test(cli): the catalog migration test expects 0.0.5 and rolls the escape migration back with the others --- tests/test_cli_vector_catalog_migration.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_cli_vector_catalog_migration.py b/tests/test_cli_vector_catalog_migration.py index e50f9f175..08ea84650 100644 --- a/tests/test_cli_vector_catalog_migration.py +++ b/tests/test_cli_vector_catalog_migration.py @@ -3,8 +3,9 @@ """Catalog migration tests against a live PostgreSQL deployment. -The vector index schema (catalog 0.0.3) and the backup table definitions -(catalog 0.0.4) are exercised through an upgrade from the pre-vector shape. +The vector index schema (catalog 0.0.3), the backup table definitions +(catalog 0.0.4) and the catalog attribute name encoding (catalog 0.0.5) are +exercised through an upgrade from the pre-vector shape. These exercise the migration against a live deployment rather than asserting the SQL text: init one, roll its catalog back to the pre-vector shape, and check that @@ -33,13 +34,17 @@ VECTOR_MIGRATION = "002_vector_indexes.sql" BACKUP_DEFINITIONS_MIGRATION = "003_backup_definitions.sql" +# Moves the version for the catalog attribute name encoding; changes no schema. +# Its code step records one row per catalog table in the progress ledger +# (`data_code_migration_progress`), and a 0.0.2 deployment has none of them. +CATALOG_ESCAPE_MIGRATION = "004_escape_catalog_control_chars.sql" # The version the binary expects, written by the last catalog migration. Every # current-version assertion reads it from here so the next bump is one edit. -CURRENT_CATALOG_VERSION = "0.0.4" +CURRENT_CATALOG_VERSION = "0.0.5" # A version no release has reached, for the symmetric version-gate test. -FUTURE_CATALOG_VERSION = "0.0.5" +FUTURE_CATALOG_VERSION = "0.0.6" def _catalog_conn(cli_env): @@ -108,9 +113,10 @@ def _roll_catalog_back_to_pre_vector(cli_env): cur.execute("DROP TABLE IF EXISTS vector_indexes") cur.execute("ALTER TABLE backups DROP COLUMN IF EXISTS vector_indexes") cur.execute( - "DELETE FROM schema_history WHERE filename IN (%s, %s)", - (VECTOR_MIGRATION, BACKUP_DEFINITIONS_MIGRATION), + "DELETE FROM schema_history WHERE filename IN (%s, %s, %s)", + (VECTOR_MIGRATION, BACKUP_DEFINITIONS_MIGRATION, CATALOG_ESCAPE_MIGRATION), ) + cur.execute("DELETE FROM data_code_migration_progress") cur.execute("UPDATE settings SET value = '0.0.2' WHERE key = 'catalog_version'") finally: conn.close() @@ -145,6 +151,7 @@ def test_init_creates_the_catalog_at_the_current_version(self, cli_env): # so a replay after a crash between applying and recording is harmless. assert VECTOR_MIGRATION in tracked, tracked assert BACKUP_DEFINITIONS_MIGRATION in tracked, tracked + assert CATALOG_ESCAPE_MIGRATION in tracked, tracked def test_migrate_upgrades_a_pre_vector_deployment(self, cli_env): """A 0.0.2 deployment is refused, upgraded by migrate, and then serves."""