diff --git a/src/Polecat.Tests/Storage/quoted_identifier_round_trip_tests.cs b/src/Polecat.Tests/Storage/quoted_identifier_round_trip_tests.cs new file mode 100644 index 00000000..179250a5 --- /dev/null +++ b/src/Polecat.Tests/Storage/quoted_identifier_round_trip_tests.cs @@ -0,0 +1,169 @@ +using JasperFx.Events; +using Microsoft.Data.SqlClient; +using Polecat.Linq; +using Polecat.Tests.Harness; +using Shouldly; + +namespace Polecat.Tests.Storage; + +public class QuotedTenantDoc +{ + public Guid Id { get; set; } + public string Name { get; set; } = string.Empty; + public int Rank { get; set; } +} + +public class QuotedNameDoc +{ + public Guid Id { get; set; } + public string Name { get; set; } = string.Empty; +} + +public record QuotedThingHappened(string What); + +/// +/// End-to-end coverage for the polecat#390 audit: values that reach a SQL identifier or +/// string-literal position by way of runtime data or a public-API argument survive a round trip +/// when they contain the characters that terminate those positions — ' for a literal and +/// ] for a bracketed identifier. +/// +/// +/// These are the cases the audit classified as (c) — supplied at runtime or through a public API +/// rather than fixed at compile time. Categories (a) and (b) are covered by the unit tests in +/// . +/// +[Collection("integration")] +public class quoted_identifier_round_trip_tests : IntegrationContext +{ + // Both terminators, in one value, plus a payload that would be visible if escaping failed open. + private const string QuoteTenant = "o'brien'; DROP TABLE pc_events--"; + private const string BracketTenant = "acme]corp"; + + public quoted_identifier_round_trip_tests(DefaultStoreFixture fixture) : base(fixture) + { + } + + [Fact] + public async Task documents_round_trip_for_a_tenant_id_containing_a_quote_or_a_bracket() + { + await DropTableAsync("quoted_tenant_docs", "pc_doc_quotedtenantdoc"); + await StoreOptions(opts => + { + opts.DatabaseSchemaName = "quoted_tenant_docs"; + opts.Events.TenancyStyle = TenancyStyle.Conjoined; // document tenancy is store-wide + }); + + var quoteDoc = new QuotedTenantDoc { Id = Guid.NewGuid(), Name = "Quoted", Rank = 1 }; + var bracketDoc = new QuotedTenantDoc { Id = Guid.NewGuid(), Name = "Bracketed", Rank = 2 }; + + theSession.ForTenant(QuoteTenant).Store(quoteDoc); + theSession.ForTenant(BracketTenant).Store(bracketDoc); + await theSession.SaveChangesAsync(TestContext.Current.CancellationToken); + + // Load: the tenant id reaches the read filter, which composes it into a string literal. + await using var quoteSession = theStore.QuerySession(new SessionOptions { TenantId = QuoteTenant }); + (await quoteSession.LoadAsync(quoteDoc.Id, TestContext.Current.CancellationToken)) + .ShouldNotBeNull() + .Name.ShouldBe("Quoted"); + + await using var bracketSession = theStore.QuerySession(new SessionOptions { TenantId = BracketTenant }); + (await bracketSession.LoadAsync(bracketDoc.Id, TestContext.Current.CancellationToken)) + .ShouldNotBeNull() + .Name.ShouldBe("Bracketed"); + + // And the tenants stay isolated — a broken escape would either error or leak across. + (await quoteSession.Query().ToListAsync(TestContext.Current.CancellationToken)) + .Select(x => x.Name).ShouldBe(["Quoted"]); + (await bracketSession.LoadAsync(quoteDoc.Id, TestContext.Current.CancellationToken)) + .ShouldBeNull(); + } + + [Fact] + public async Task events_round_trip_for_a_tenant_id_containing_a_quote_or_a_bracket() + { + await StoreOptions(opts => + { + opts.DatabaseSchemaName = "quoted_tenant_events"; + opts.Events.TenancyStyle = TenancyStyle.Conjoined; + }); + + var quoteStream = Guid.NewGuid(); + var bracketStream = Guid.NewGuid(); + + theSession.ForTenant(QuoteTenant).Events.StartStream(quoteStream, new QuotedThingHappened("quoted")); + theSession.ForTenant(BracketTenant).Events.StartStream(bracketStream, new QuotedThingHappened("bracketed")); + await theSession.SaveChangesAsync(TestContext.Current.CancellationToken); + + await using var quoteSession = theStore.QuerySession(new SessionOptions { TenantId = QuoteTenant }); + var quoteEvents = await quoteSession.Events.FetchStreamAsync( + quoteStream, token: TestContext.Current.CancellationToken); + quoteEvents.Count.ShouldBe(1); + quoteEvents[0].Data.ShouldBeOfType().What.ShouldBe("quoted"); + + await using var bracketSession = theStore.QuerySession(new SessionOptions { TenantId = BracketTenant }); + var bracketEvents = await bracketSession.Events.FetchStreamAsync( + bracketStream, token: TestContext.Current.CancellationToken); + bracketEvents.Count.ShouldBe(1); + bracketEvents[0].Data.ShouldBeOfType().What.ShouldBe("bracketed"); + + // The pc_events table is still there — proof the `'; DROP TABLE` payload stayed inert data. + (await ScalarAsync("SELECT OBJECT_ID('[quoted_tenant_events].[pc_events]')")).ShouldNotBeNull(); + } + + [Fact] + public async Task an_index_name_containing_a_bracket_or_a_quote_is_created_and_is_idempotent() + { + // IndexName is a public-API argument that lands in BOTH a string literal (the sys.indexes + // existence probe) and a bracketed identifier (CREATE INDEX). Applying the schema twice + // proves the two positions agree: if only one were escaped, the probe would never match its + // own index and the second apply would fail with "index already exists". + const string schema = "quoted_index_names"; + const string indexName = "idx_qu]oted_o'brien"; + + await DropTableAsync(schema, "pc_doc_quotednamedoc"); + + // Document tables (and their indexes) are created lazily on first use by DocumentTableEnsurer, + // so each pass has to actually write. Two passes: the second re-runs the existence probe + // against the index the first one created. + for (var pass = 0; pass < 2; pass++) + { + await StoreOptions(opts => + { + opts.DatabaseSchemaName = schema; + opts.Schema.For().Index(x => x.Name, idx => idx.IndexName = indexName); + }); + + var doc = new QuotedNameDoc { Id = Guid.NewGuid(), Name = $"pass-{pass}" }; + theSession.Store(doc); + await theSession.SaveChangesAsync(TestContext.Current.CancellationToken); + + (await theSession.LoadAsync(doc.Id, TestContext.Current.CancellationToken)) + .ShouldNotBeNull().Name.ShouldBe($"pass-{pass}"); + } + + (await ScalarAsync( + $""" + SELECT COUNT(*) FROM sys.indexes + WHERE name = '{indexName.Replace("'", "''")}' + AND object_id = OBJECT_ID('[{schema}].[pc_doc_quotednamedoc]') + """)).ShouldBe(1); + } + + private async Task ScalarAsync(string sql) + { + await using var conn = await OpenConnectionAsync(); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = sql; + var result = await cmd.ExecuteScalarAsync(TestContext.Current.CancellationToken); + return result == DBNull.Value ? null : result; + } + + private static async Task DropTableAsync(string schema, string table) + { + await using var conn = new SqlConnection(ConnectionSource.ConnectionString); + await conn.OpenAsync(TestContext.Current.CancellationToken); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = $"DROP TABLE IF EXISTS [{schema}].[{table}];"; + await cmd.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); + } +} diff --git a/src/Polecat.Tests/Storage/sql_escaping_tests.cs b/src/Polecat.Tests/Storage/sql_escaping_tests.cs new file mode 100644 index 00000000..896275ce --- /dev/null +++ b/src/Polecat.Tests/Storage/sql_escaping_tests.cs @@ -0,0 +1,64 @@ +using Polecat.Internal; +using Shouldly; + +namespace Polecat.Tests.Storage; + +/// +/// Unit coverage for the shared escaping helper introduced by the polecat#390 audit. These are the +/// two positions T-SQL requires escaping in, and the two mistakes the audit was looking for. +/// +public class sql_escaping_tests +{ + [Fact] + public void quote_identifier_doubles_an_embedded_closing_bracket() + { + // Without the doubling the bracket closes early and everything after it is parsed as SQL. + SqlEscaping.QuoteIdentifier("plain").ShouldBe("[plain]"); + SqlEscaping.QuoteIdentifier("we]ird").ShouldBe("[we]]ird]"); + SqlEscaping.QuoteIdentifier("a]; DROP TABLE x --").ShouldBe("[a]]; DROP TABLE x --]"); + } + + [Fact] + public void quote_identifier_leaves_quotes_and_dots_alone() + { + // A single quote is not special inside a bracketed identifier, and a dot must NOT be treated + // as a separator — [a.b] is one object named "a.b". + SqlEscaping.QuoteIdentifier("o'brien").ShouldBe("[o'brien]"); + SqlEscaping.QuoteIdentifier("a.b").ShouldBe("[a.b]"); + } + + [Fact] + public void qualified_name_escapes_both_halves_independently() + { + SqlEscaping.QualifiedName("dbo", "pc_events").ShouldBe("[dbo].[pc_events]"); + SqlEscaping.QualifiedName("sch]ema", "tab]le").ShouldBe("[sch]]ema].[tab]]le]"); + } + + [Fact] + public void literal_doubles_an_embedded_single_quote() + { + SqlEscaping.Literal("plain").ShouldBe("'plain'"); + SqlEscaping.Literal("o'brien").ShouldBe("'o''brien'"); + SqlEscaping.LiteralBody("o'brien").ShouldBe("o''brien"); + } + + [Fact] + public void literal_has_no_already_quoted_shortcut() + { + // weasel#416's postmortem: an "is this already escaped?" test cannot be made safely from the + // shape of untrusted input — a value that happens to start and end with a quote would skip + // escaping entirely, which is strictly worse than the missing escape it replaces. Formatting + // here is unconditional, so a quote-wrapped input is escaped like any other. + SqlEscaping.Literal("'sneaky'").ShouldBe("'''sneaky'''"); + } + + [Fact] + public void a_name_bound_for_both_positions_composes_the_two_escapes() + { + // The audit's specific trap: the same object name appears bare in `ALTER TABLE [s].[t]` and + // as a string in `OBJECT_ID('[s].[t]')`. Those need different escapes, applied in order. + var qualified = SqlEscaping.QualifiedName("sch'ema", "tab]le"); + qualified.ShouldBe("[sch'ema].[tab]]le]"); + SqlEscaping.Literal(qualified).ShouldBe("'[sch''ema].[tab]]le]'"); + } +} diff --git a/src/Polecat/AdvancedOperations.cs b/src/Polecat/AdvancedOperations.cs index 8697a1d4..3e91baf9 100644 --- a/src/Polecat/AdvancedOperations.cs +++ b/src/Polecat/AdvancedOperations.cs @@ -379,7 +379,10 @@ SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES foreach (var table in tables) { await using var deleteCmd = conn.CreateCommand(); - deleteCmd.CommandText = $"DELETE FROM [{schemaName}].[{table}];"; + // #390: `table` is read back out of INFORMATION_SCHEMA rather than derived from + // configuration, so it is escaped as a bracketed identifier — an embedded `]` in a + // table name would otherwise close the bracket early (stored/second-order). + deleteCmd.CommandText = $"DELETE FROM {SqlEscaping.QualifiedName(schemaName, table)};"; await deleteCmd.ExecuteNonQueryAsync(ct); } @@ -443,7 +446,8 @@ SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES foreach (var table in tables) { await using var dropCmd = conn.CreateCommand(); - dropCmd.CommandText = $"DROP TABLE IF EXISTS [{schemaName}].[{table}];"; + // #390: `table` comes back out of INFORMATION_SCHEMA — escape it as an identifier. + dropCmd.CommandText = $"DROP TABLE IF EXISTS {SqlEscaping.QualifiedName(schemaName, table)};"; await dropCmd.ExecuteNonQueryAsync(ct); } @@ -451,7 +455,8 @@ SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES foreach (var qualified in flatTableNames) { await using var dropCmd = conn.CreateCommand(); - dropCmd.CommandText = $"IF OBJECT_ID('{qualified}', 'U') IS NOT NULL DROP TABLE {qualified};"; + dropCmd.CommandText = + $"IF OBJECT_ID({SqlEscaping.Literal(qualified)}, 'U') IS NOT NULL DROP TABLE {qualified};"; await dropCmd.ExecuteNonQueryAsync(ct); } }, (connStr, schema, flatTables), token); @@ -475,8 +480,11 @@ private static async Task DeleteFlatTablesAsync(SqlConnection conn, string[] qua foreach (var qualified in qualifiedNames) { await using var cmd = conn.CreateCommand(); - // Guard against the table not existing yet (the projection ensures it lazily). - cmd.CommandText = $"IF OBJECT_ID('{qualified}', 'U') IS NOT NULL DELETE FROM {qualified};"; + // Guard against the table not existing yet (the projection ensures it lazily). #390: the + // same name lands in a string-literal position and a bare identifier position, which need + // different escaping — only the literal one gets it. + cmd.CommandText = + $"IF OBJECT_ID({SqlEscaping.Literal(qualified)}, 'U') IS NOT NULL DELETE FROM {qualified};"; await cmd.ExecuteNonQueryAsync(ct); } } @@ -496,7 +504,8 @@ await _resilience.ExecuteAsync(static async (state, ct) => await conn.OpenAsync(ct); await using var cmd = conn.CreateCommand(); - cmd.CommandText = $"IF OBJECT_ID('{qualifiedTableName}', 'U') IS NOT NULL DELETE FROM {qualifiedTableName};"; + cmd.CommandText = + $"IF OBJECT_ID({SqlEscaping.Literal(qualifiedTableName)}, 'U') IS NOT NULL DELETE FROM {qualifiedTableName};"; await cmd.ExecuteNonQueryAsync(ct); }, (connStr, tableName), token); } @@ -633,7 +642,8 @@ SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES foreach (var table in nkTables) { await using var deleteCmd = conn.CreateCommand(); - deleteCmd.CommandText = $"DELETE FROM [{schemaName}].[{table}];"; + // #390: natural-key table names are read out of INFORMATION_SCHEMA — escape. + deleteCmd.CommandText = $"DELETE FROM {SqlEscaping.QualifiedName(schemaName, table)};"; await deleteCmd.ExecuteNonQueryAsync(ct); } } @@ -643,19 +653,22 @@ SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES // store schema has been created (e.g. during bootstrap of a fresh database). await using (var cmd = conn.CreateCommand()) { - cmd.CommandText = $"IF OBJECT_ID('{evtTable}', 'U') IS NOT NULL DELETE FROM {evtTable};"; + cmd.CommandText = + $"IF OBJECT_ID({SqlEscaping.Literal(evtTable)}, 'U') IS NOT NULL DELETE FROM {evtTable};"; await cmd.ExecuteNonQueryAsync(ct); } await using (var cmd = conn.CreateCommand()) { - cmd.CommandText = $"IF OBJECT_ID('{strmTable}', 'U') IS NOT NULL DELETE FROM {strmTable};"; + cmd.CommandText = + $"IF OBJECT_ID({SqlEscaping.Literal(strmTable)}, 'U') IS NOT NULL DELETE FROM {strmTable};"; await cmd.ExecuteNonQueryAsync(ct); } await using (var cmd = conn.CreateCommand()) { - cmd.CommandText = $"IF OBJECT_ID('{progTable}', 'U') IS NOT NULL DELETE FROM {progTable};"; + cmd.CommandText = + $"IF OBJECT_ID({SqlEscaping.Literal(progTable)}, 'U') IS NOT NULL DELETE FROM {progTable};"; await cmd.ExecuteNonQueryAsync(ct); } }, (connStr, schema, eventsTable, streamsTable, progressionTable, flatTables), token); diff --git a/src/Polecat/DocumentStore.EventStore.cs b/src/Polecat/DocumentStore.EventStore.cs index 20777afb..9997dc26 100644 --- a/src/Polecat/DocumentStore.EventStore.cs +++ b/src/Polecat/DocumentStore.EventStore.cs @@ -546,9 +546,12 @@ await Options.ResiliencePipeline.ExecuteAsync(static async (state, ct) => // rebuild it may not exist yet — guard the tenant-scoped delete accordingly. // #234: single-tenant projection tables have no tenant_id column, so the delete // is unscoped there (only the default tenant's rows exist). + // #390: the same name occupies a string-literal position (OBJECT_ID) and a bare + // identifier position, which take different escaping. + var exists = $"IF OBJECT_ID({SqlEscaping.Literal(tableName)}, 'U') IS NOT NULL"; delDocs.CommandText = isConjoined - ? $"IF OBJECT_ID('{tableName}', 'U') IS NOT NULL DELETE FROM {tableName} WHERE tenant_id = @tenant;" - : $"IF OBJECT_ID('{tableName}', 'U') IS NOT NULL DELETE FROM {tableName};"; + ? $"{exists} DELETE FROM {tableName} WHERE tenant_id = @tenant;" + : $"{exists} DELETE FROM {tableName};"; if (isConjoined) delDocs.Parameters.AddVarChar("@tenant", tenant); await delDocs.ExecuteNonQueryAsync(ct); } @@ -611,8 +614,7 @@ private async Task TeardownProjectionStateAsync(IEventDatabase database, string // same as for the doc tables, so it rides the same loop below. if (source is JasperFx.Events.Aggregation.IAggregateProjection { NaturalKeyDefinition: not null } natural) { - var aggregateName = natural.NaturalKeyDefinition.AggregateType.Name.ToLowerInvariant(); - tables.Add($"[{Events.DatabaseSchemaName}].[pc_natural_key_{aggregateName}]"); + tables.Add(Events.NaturalKeyTableName(natural.NaturalKeyDefinition.AggregateType)); } publishedTableNames = tables.ToArray(); diff --git a/src/Polecat/DocumentStore.EventStoreExplorer.cs b/src/Polecat/DocumentStore.EventStoreExplorer.cs index a6054ecf..4d2e1aac 100644 --- a/src/Polecat/DocumentStore.EventStoreExplorer.cs +++ b/src/Polecat/DocumentStore.EventStoreExplorer.cs @@ -210,7 +210,6 @@ async IAsyncEnumerable IEventStore.QueryByTagsAsync( ArgumentNullException.ThrowIfNull(tags); if (tags.Count == 0) yield break; - var schema = Events.DatabaseSchemaName; var options = Events.EventOptions; var registered = Events.TagTypes; @@ -243,7 +242,7 @@ async IAsyncEnumerable IEventStore.QueryByTagsAsync( // value arrives as a string from the explorer, so it's compared case-insensitively via nvarchar — // matching whatever native type (uniqueidentifier, etc.) the tag's value column stores. sb.Append( - $"e.seq_id IN (SELECT seq_id FROM [{schema}].[pc_event_tag_{registration.TableSuffix}] WHERE LOWER(CONVERT(nvarchar(4000), value)) = LOWER(@tag_value_{idx}))"); + $"e.seq_id IN (SELECT seq_id FROM {Events.TagTableName(registration)} WHERE LOWER(CONVERT(nvarchar(4000), value)) = LOWER(@tag_value_{idx}))"); cmd.Parameters.AddWithValue($"@tag_value_{idx}", tagValue); idx++; } diff --git a/src/Polecat/Events/EventGraph.cs b/src/Polecat/Events/EventGraph.cs index 4361befc..1fb5757c 100644 --- a/src/Polecat/Events/EventGraph.cs +++ b/src/Polecat/Events/EventGraph.cs @@ -127,10 +127,51 @@ public TenancyStyle TenancyStyle // and Marten's StoreOptions.Events.EnableSideEffectsOnInlineProjections. public bool EnableSideEffectsOnInlineProjections => _options.Events.EnableSideEffectsOnInlineProjections; - internal string StreamsTableName => $"[{DatabaseSchemaName}].[pc_streams]"; - internal string EventsTableName => $"[{DatabaseSchemaName}].[pc_events]"; - internal string ProgressionTableName => $"[{DatabaseSchemaName}].[pc_event_progression]"; - internal string TenantPartitionsTableName => $"[{DatabaseSchemaName}].[pc_tenant_partitions]"; + internal string StreamsTableName => Polecat.Internal.SqlEscaping.QualifiedName(DatabaseSchemaName, "pc_streams"); + internal string EventsTableName => Polecat.Internal.SqlEscaping.QualifiedName(DatabaseSchemaName, "pc_events"); + + internal string ProgressionTableName => + Polecat.Internal.SqlEscaping.QualifiedName(DatabaseSchemaName, "pc_event_progression"); + + internal string TenantPartitionsTableName => + Polecat.Internal.SqlEscaping.QualifiedName(DatabaseSchemaName, "pc_tenant_partitions"); + + /// + /// The unqualified DCB tag table name for a registered tag type. + /// + /// + /// #390: TableSuffix is a public-API argument + /// (), and a dozen call sites used to compose this + /// name by hand. Two paths building the same object name by different means is both an + /// injection risk and a correctness divergence — they agree right up until quoting is needed. + /// Every caller now goes through this pair, and applies the + /// identifier escaping. + /// + internal static string TagTableNameFor(ITagTypeRegistration registration) + => "pc_event_tag_" + registration.TableSuffix; + + /// + /// The schema-qualified, bracket-escaped DCB tag table name for a registered tag type. + /// + internal string TagTableName(ITagTypeRegistration registration) + => Polecat.Internal.SqlEscaping.QualifiedName(DatabaseSchemaName, TagTableNameFor(registration)); + + /// + /// The unqualified pc_natural_key_* lookup table name for a [NaturalKey] aggregate. + /// + /// + /// Type-derived rather than caller-supplied, so not itself an injection risk — but #390's + /// one-builder-per-object-name rule applies regardless: five sites composed this name + /// independently, which is how two paths quietly stop agreeing. + /// + internal static string NaturalKeyTableNameFor(Type aggregateType) + => "pc_natural_key_" + aggregateType.Name.ToLowerInvariant(); + + /// + /// The schema-qualified, bracket-escaped pc_natural_key_* table name for an aggregate. + /// + internal string NaturalKeyTableName(Type aggregateType) + => Polecat.Internal.SqlEscaping.QualifiedName(DatabaseSchemaName, NaturalKeyTableNameFor(aggregateType)); private ManagedTenantPartitions? _tenantPartitions; diff --git a/src/Polecat/Events/EventOperations.cs b/src/Polecat/Events/EventOperations.cs index 63a14485..32634819 100644 --- a/src/Polecat/Events/EventOperations.cs +++ b/src/Polecat/Events/EventOperations.cs @@ -682,7 +682,7 @@ public async Task> FetchForExclusiveWriting(TId id, Canc var isGuidStream = _events.StreamIdentity == StreamIdentity.AsGuid; var schema = _events.DatabaseSchemaName; - var tableName = $"pc_natural_key_{naturalKey.AggregateType.Name.ToLowerInvariant()}"; + var qualifiedTableName = _events.NaturalKeyTableName(naturalKey.AggregateType); var streamColumn = isGuidStream ? "stream_id" : "stream_key"; // Look up stream id from natural key table (read-only, no locking) @@ -694,7 +694,7 @@ public async Task> FetchForExclusiveWriting(TId id, Canc cmd.CommandText = $""" SELECT nk.{streamColumn} - FROM [{schema}].[{tableName}] nk + FROM {qualifiedTableName} nk WHERE nk.natural_key_value = @naturalKey AND nk.is_archived = 0{tenantFilter}; """; cmd.Parameters.AddWithValue("@naturalKey", unwrapped); // natural_key_value is nvarchar(200) @@ -795,12 +795,12 @@ public async Task EventsExistAsync(EventTagQuery query, CancellationToken var alias = $"t{i}"; if (first) { - sb.Append($"[{schema}].[pc_event_tag_{registration.TableSuffix}] {alias}"); + sb.Append($"{_events.TagTableName(registration)} {alias}"); first = false; } else { - sb.Append($" INNER JOIN [{schema}].[pc_event_tag_{registration.TableSuffix}] {alias} ON t0.seq_id = {alias}.seq_id"); + sb.Append($" INNER JOIN {_events.TagTableName(registration)} {alias} ON t0.seq_id = {alias}.seq_id"); } } @@ -888,7 +888,7 @@ public async Task> QueryByTagsAsync(EventTagQuery query, ?? throw new InvalidOperationException( $"Tag type '{tagType.Name}' is not registered."); - sb.Append($" LEFT JOIN [{schema}].[pc_event_tag_{registration.TableSuffix}] t{i} ON e.seq_id = t{i}.seq_id"); + sb.Append($" LEFT JOIN {_events.TagTableName(registration)} t{i} ON e.seq_id = t{i}.seq_id"); } // WHERE clause @@ -1093,7 +1093,7 @@ internal static void WriteTagQuerySql(ICommandBuilder builder, EventGraph eventG ?? throw new InvalidOperationException( $"Tag type '{tagType.Name}' is not registered."); - builder.Append($" LEFT JOIN [{schema}].[pc_event_tag_{registration.TableSuffix}] t{i} ON e.seq_id = t{i}.seq_id"); + builder.Append($" LEFT JOIN {eventGraph.TagTableName(registration)} t{i} ON e.seq_id = t{i}.seq_id"); } builder.Append(" WHERE ("); diff --git a/src/Polecat/Events/Fetching/NaturalKeyFetchPlanner.cs b/src/Polecat/Events/Fetching/NaturalKeyFetchPlanner.cs index 7055d93c..383386ef 100644 --- a/src/Polecat/Events/Fetching/NaturalKeyFetchPlanner.cs +++ b/src/Polecat/Events/Fetching/NaturalKeyFetchPlanner.cs @@ -24,8 +24,7 @@ public static async Task> FetchForWritingByNaturalKey( CancellationToken cancellation) where T : class where TId : notnull { var isGuidStream = events.StreamIdentity == StreamIdentity.AsGuid; - var schema = events.DatabaseSchemaName; - var tableName = $"pc_natural_key_{naturalKey.AggregateType.Name.ToLowerInvariant()}"; + var qualifiedTableName = events.NaturalKeyTableName(naturalKey.AggregateType); var streamColumn = isGuidStream ? "stream_id" : "stream_key"; // Unwrap strong-typed id to primitive @@ -43,7 +42,7 @@ public static async Task> FetchForWritingByNaturalKey( cmd.CommandText = $""" SELECT s.version, s.id - FROM [{schema}].[{tableName}] nk WITH (NOLOCK) + FROM {qualifiedTableName} nk WITH (NOLOCK) INNER JOIN {events.StreamsTableName} s WITH (UPDLOCK, HOLDLOCK) ON s.id = nk.{streamColumn} WHERE nk.natural_key_value = @naturalKey AND nk.is_archived = 0{tenantFilter} AND s.tenant_id = @tenantId; diff --git a/src/Polecat/Events/Linq/HasTagParser.cs b/src/Polecat/Events/Linq/HasTagParser.cs index 2ae0411d..a472bf9f 100644 --- a/src/Polecat/Events/Linq/HasTagParser.cs +++ b/src/Polecat/Events/Linq/HasTagParser.cs @@ -40,8 +40,7 @@ public ISqlFragment Parse(IMemberResolver memberFactory, MethodCallExpression ex $"Tag type '{tagType.Name}' is not registered. Call RegisterTagType<{tagType.Name}>() first."); var extracted = registration.ExtractValue(value); - var schema = _events.DatabaseSchemaName; - var suffix = registration.TableSuffix; + var tagTable = _events.TagTableName(registration); // Under conjoined tenancy a tag value is only unique per tenant, so the correlated subquery must // also match the outer event row's tenant_id (the outer query is already tenant-scoped). @@ -50,7 +49,7 @@ public ISqlFragment Parse(IMemberResolver memberFactory, MethodCallExpression ex : string.Empty; return new HasTagFilter( - $"seq_id IN (SELECT pt.seq_id FROM [{schema}].[pc_event_tag_{suffix}] pt WHERE pt.value = ", + $"seq_id IN (SELECT pt.seq_id FROM {tagTable} pt WHERE pt.value = ", extracted, $"{correlation})"); } diff --git a/src/Polecat/Events/Operations/AssignTagWhereOperation.cs b/src/Polecat/Events/Operations/AssignTagWhereOperation.cs index bc60cd81..717f3d0f 100644 --- a/src/Polecat/Events/Operations/AssignTagWhereOperation.cs +++ b/src/Polecat/Events/Operations/AssignTagWhereOperation.cs @@ -44,8 +44,9 @@ public AssignTagWhereOperation(string schemaName, ITagTypeRegistration registrat public void ConfigureCommand(ICommandBuilder builder) { - var tagTable = $"[{_schemaName}].[pc_event_tag_{_registration.TableSuffix}]"; - var eventsTable = $"[{_schemaName}].[pc_events]"; + var tagTable = Polecat.Internal.SqlEscaping.QualifiedName( + _schemaName, EventGraph.TagTableNameFor(_registration)); + var eventsTable = Polecat.Internal.SqlEscaping.QualifiedName(_schemaName, "pc_events"); // When pc_events is partitioned by is_archived the tag table also carries // is_archived (PK + FK columns) — see EventTagTable. Carry the source row's diff --git a/src/Polecat/Events/Projections/NaturalKeyProjection.cs b/src/Polecat/Events/Projections/NaturalKeyProjection.cs index 16704c32..628ce9fe 100644 --- a/src/Polecat/Events/Projections/NaturalKeyProjection.cs +++ b/src/Polecat/Events/Projections/NaturalKeyProjection.cs @@ -23,7 +23,7 @@ public NaturalKeyProjection(NaturalKeyDefinition definition, EventGraph events) { _definition = definition; _events = events; - _qualifiedTableName = $"[{events.DatabaseSchemaName}].[pc_natural_key_{definition.AggregateType.Name.ToLowerInvariant()}]"; + _qualifiedTableName = events.NaturalKeyTableName(definition.AggregateType); _isGuidStream = events.StreamIdentity == StreamIdentity.AsGuid; _isConjoined = events.TenancyStyle == TenancyStyle.Conjoined; } diff --git a/src/Polecat/Events/Schema/EventTagTable.cs b/src/Polecat/Events/Schema/EventTagTable.cs index 8add4500..899f3daa 100644 --- a/src/Polecat/Events/Schema/EventTagTable.cs +++ b/src/Polecat/Events/Schema/EventTagTable.cs @@ -12,7 +12,7 @@ namespace Polecat.Events.Schema; internal class EventTagTable : Table { public EventTagTable(EventGraph events, ITagTypeRegistration registration) - : base(new SqlServerObjectName(events.DatabaseSchemaName, $"pc_event_tag_{registration.TableSuffix}")) + : base(new SqlServerObjectName(events.DatabaseSchemaName, EventGraph.TagTableNameFor(registration))) { var sqlType = SqlServerTypeFor(registration.SimpleType); var isConjoined = events.TenancyStyle == TenancyStyle.Conjoined; diff --git a/src/Polecat/Events/Schema/NaturalKeyTable.cs b/src/Polecat/Events/Schema/NaturalKeyTable.cs index 86e0129c..aaace144 100644 --- a/src/Polecat/Events/Schema/NaturalKeyTable.cs +++ b/src/Polecat/Events/Schema/NaturalKeyTable.cs @@ -12,7 +12,7 @@ internal class NaturalKeyTable : Table { public NaturalKeyTable(EventGraph events, NaturalKeyDefinition naturalKey) : base(new SqlServerObjectName(events.DatabaseSchemaName, - $"pc_natural_key_{naturalKey.AggregateType.Name.ToLowerInvariant()}")) + EventGraph.NaturalKeyTableNameFor(naturalKey.AggregateType))) { var columnType = naturalKey.InnerType == typeof(int) ? "int" : naturalKey.InnerType == typeof(long) ? "bigint" diff --git a/src/Polecat/Events/Storage/PolecatQuickAppendEventsOperation.cs b/src/Polecat/Events/Storage/PolecatQuickAppendEventsOperation.cs index eb2ecbb0..d233ec40 100644 --- a/src/Polecat/Events/Storage/PolecatQuickAppendEventsOperation.cs +++ b/src/Polecat/Events/Storage/PolecatQuickAppendEventsOperation.cs @@ -243,7 +243,7 @@ private void WriteTagInserts(Weasel.Core.ICommandBuilder builder, IEvent @event, if (registration == null) continue; var value = registration.ExtractValue(tag.Value); - var table = $"[{_graph.DatabaseSchemaName}].[pc_event_tag_{registration.TableSuffix}]"; + var table = _graph.TagTableName(registration); builder.Append("if not exists (select 1 from "); builder.Append(table); diff --git a/src/Polecat/Events/TestSupport/ProjectionScenario.cs b/src/Polecat/Events/TestSupport/ProjectionScenario.cs index fc5b6bf8..55dc4866 100644 --- a/src/Polecat/Events/TestSupport/ProjectionScenario.cs +++ b/src/Polecat/Events/TestSupport/ProjectionScenario.cs @@ -135,7 +135,8 @@ private async Task CleanDocumentsByTypeAsync(Type type, CancellationToken ct) await using var conn = new Microsoft.Data.SqlClient.SqlConnection(_store.Options.ConnectionString); await conn.OpenAsync(ct); await using var cmd = conn.CreateCommand(); - cmd.CommandText = $"IF OBJECT_ID('{tableName}', 'U') IS NOT NULL DELETE FROM {tableName};"; + cmd.CommandText = + $"IF OBJECT_ID({Polecat.Internal.SqlEscaping.Literal(tableName)}, 'U') IS NOT NULL DELETE FROM {tableName};"; await cmd.ExecuteNonQueryAsync(ct); } } diff --git a/src/Polecat/Internal/Batching/EventsExistBatchItem.cs b/src/Polecat/Internal/Batching/EventsExistBatchItem.cs index 3f7c40e4..886a5bfc 100644 --- a/src/Polecat/Internal/Batching/EventsExistBatchItem.cs +++ b/src/Polecat/Internal/Batching/EventsExistBatchItem.cs @@ -28,7 +28,6 @@ public void WriteSql(ICommandBuilder builder) throw new ArgumentException("EventTagQuery must have at least one condition."); var distinctTagTypes = conditions.Select(c => c.TagType).Distinct().ToList(); - var schema = _eventGraph.DatabaseSchemaName; builder.Append("SELECT CASE WHEN EXISTS (SELECT 1 FROM "); @@ -43,19 +42,19 @@ public void WriteSql(ICommandBuilder builder) var alias = $"t{i}"; if (first) { - builder.Append($"[{schema}].[pc_event_tag_{registration.TableSuffix}] {alias}"); + builder.Append($"{_eventGraph.TagTableName(registration)} {alias}"); first = false; } else { - builder.Append($" INNER JOIN [{schema}].[pc_event_tag_{registration.TableSuffix}] {alias} ON t0.seq_id = {alias}.seq_id"); + builder.Append($" INNER JOIN {_eventGraph.TagTableName(registration)} {alias} ON t0.seq_id = {alias}.seq_id"); } } var hasEventTypeFilter = conditions.Any(c => c.EventType != null); if (hasEventTypeFilter) { - builder.Append($" INNER JOIN [{schema}].[pc_events] e ON t0.seq_id = e.seq_id"); + builder.Append($" INNER JOIN {_eventGraph.EventsTableName} e ON t0.seq_id = e.seq_id"); } builder.Append(" WHERE ("); diff --git a/src/Polecat/Internal/DocumentTableEnsurer.cs b/src/Polecat/Internal/DocumentTableEnsurer.cs index ba6ba950..c4578fee 100644 --- a/src/Polecat/Internal/DocumentTableEnsurer.cs +++ b/src/Polecat/Internal/DocumentTableEnsurer.cs @@ -198,7 +198,7 @@ private static async Task WidenVersionColumnIfNeededAsync(SqlConnection conn, st // forbids function calls inside EXEC(...), hence the SET-then-EXEC pattern). await using var cmd = conn.CreateCommand(); cmd.CommandText = $""" - DECLARE @oid int = OBJECT_ID('{qualifiedTableName}'); + DECLARE @oid int = OBJECT_ID({SqlEscaping.Literal(qualifiedTableName)}); IF @oid IS NOT NULL BEGIN DECLARE @df sysname, @sql nvarchar(max); @@ -207,7 +207,7 @@ IF @oid IS NOT NULL WHERE dc.parent_object_id = @oid AND col.name = 'version'; IF @df IS NOT NULL BEGIN - SET @sql = 'ALTER TABLE {qualifiedTableName} DROP CONSTRAINT ' + QUOTENAME(@df); + SET @sql = '{SqlEscaping.LiteralBody($"ALTER TABLE {qualifiedTableName} DROP CONSTRAINT ")}' + QUOTENAME(@df); EXEC(@sql); END IF EXISTS ( @@ -248,7 +248,7 @@ private static async Task ConvertStrongTypedIdColumnIfNeededAsync(SqlConnection } var qualifiedTableName = mapping.QualifiedTableName; - var pkColumnList = string.Join(", ", table.PrimaryKeyColumns.Select(c => $"[{c}]")); + var pkColumnList = string.Join(", ", table.PrimaryKeyColumns.Select(SqlEscaping.QuoteIdentifier)); // The table/PK names are derived from the document type (not user input) and inlined; only // the discovered PK-constraint name needs dynamic SQL (SQL Server forbids function calls @@ -256,7 +256,7 @@ private static async Task ConvertStrongTypedIdColumnIfNeededAsync(SqlConnection // key first, so the whole convert runs as one guarded batch. await using var cmd = conn.CreateCommand(); cmd.CommandText = $""" - DECLARE @oid int = OBJECT_ID('{qualifiedTableName}'); + DECLARE @oid int = OBJECT_ID({SqlEscaping.Literal(qualifiedTableName)}); IF @oid IS NOT NULL AND EXISTS ( SELECT 1 FROM sys.columns c WHERE c.object_id = @oid AND c.name = 'id' @@ -267,7 +267,7 @@ AND TYPE_NAME(c.system_type_id) IN ('varchar', 'nvarchar', 'char', 'nchar')) WHERE kc.parent_object_id = @oid AND kc.type = 'PK'; IF @pk IS NOT NULL BEGIN - SET @sql = 'ALTER TABLE {qualifiedTableName} DROP CONSTRAINT ' + QUOTENAME(@pk); + SET @sql = '{SqlEscaping.LiteralBody($"ALTER TABLE {qualifiedTableName} DROP CONSTRAINT ")}' + QUOTENAME(@pk); EXEC(@sql); END ALTER TABLE {qualifiedTableName} ALTER COLUMN [id] {targetType} NOT NULL; diff --git a/src/Polecat/Internal/Operations/AssertDcbConsistencyOperation.cs b/src/Polecat/Internal/Operations/AssertDcbConsistencyOperation.cs index a6b80b53..1b700011 100644 --- a/src/Polecat/Internal/Operations/AssertDcbConsistencyOperation.cs +++ b/src/Polecat/Internal/Operations/AssertDcbConsistencyOperation.cs @@ -32,7 +32,6 @@ public void ConfigureCommand(ICommandBuilder builder) { var conditions = _query.Conditions; var distinctTagTypes = conditions.Select(c => c.TagType).Distinct().ToList(); - var schema = _events.DatabaseSchemaName; // Build EXISTS query builder.Append("SELECT CASE WHEN EXISTS (SELECT 1 FROM "); @@ -48,12 +47,12 @@ public void ConfigureCommand(ICommandBuilder builder) var alias = $"t{i}"; if (first) { - builder.Append($"[{schema}].[pc_event_tag_{registration.TableSuffix}] {alias}"); + builder.Append($"{_events.TagTableName(registration)} {alias}"); first = false; } else { - builder.Append($" INNER JOIN [{schema}].[pc_event_tag_{registration.TableSuffix}] {alias} ON t0.seq_id = {alias}.seq_id"); + builder.Append($" INNER JOIN {_events.TagTableName(registration)} {alias} ON t0.seq_id = {alias}.seq_id"); } } @@ -61,7 +60,7 @@ public void ConfigureCommand(ICommandBuilder builder) var hasEventTypeFilter = conditions.Any(c => c.EventType != null); if (hasEventTypeFilter) { - builder.Append($" INNER JOIN [{schema}].[pc_events] e ON t0.seq_id = e.seq_id"); + builder.Append($" INNER JOIN {_events.EventsTableName} e ON t0.seq_id = e.seq_id"); } builder.Append(" WHERE t0.seq_id > "); diff --git a/src/Polecat/Internal/PolecatDocumentSchemaResolver.cs b/src/Polecat/Internal/PolecatDocumentSchemaResolver.cs index b3b4bc2d..d4bf736e 100644 --- a/src/Polecat/Internal/PolecatDocumentSchemaResolver.cs +++ b/src/Polecat/Internal/PolecatDocumentSchemaResolver.cs @@ -42,5 +42,5 @@ public string For(Type documentType, bool qualified = true) public string ForEventProgression(bool qualified = true) => Format("pc_event_progression", qualified); private string Format(string table, bool qualified) - => qualified ? $"[{_options.DatabaseSchemaName}].[{table}]" : table; + => qualified ? SqlEscaping.QualifiedName(_options.DatabaseSchemaName, table) : table; } diff --git a/src/Polecat/Internal/SqlEscaping.cs b/src/Polecat/Internal/SqlEscaping.cs new file mode 100644 index 00000000..e6b60c1f --- /dev/null +++ b/src/Polecat/Internal/SqlEscaping.cs @@ -0,0 +1,66 @@ +namespace Polecat.Internal; + +/// +/// The single place Polecat escapes a value it is about to interpolate into SQL text, for the +/// two positions where T-SQL requires escaping (polecat#390, companion to weasel#416): +/// inside a bracketed identifier, where an embedded ] closes the bracket early and must +/// be doubled, and inside a string literal, where an embedded ' must be doubled. +/// +/// +/// +/// Nothing upstream is a sanitizing boundary, so this type cannot be skipped on the grounds +/// that a value "came from Weasel": SqlServerMigrator.AssertValidIdentifier is a +/// no-op, SchemaUtils.QuoteName brackets only reserved keywords (and escapes +/// nothing), and DbObjectName performs no validation at all. +/// +/// +/// Use one builder per object name. The failure mode this type exists to +/// prevent is two code paths composing the same object name by different means — +/// they agree right up until quoting is needed, and then diverge into a syntax error or a +/// reference to the wrong object. Notably, a name that appears in both an identifier +/// position (ALTER TABLE [s].[t]) and a string-literal position +/// (OBJECT_ID('[s].[t]')) needs both escapes applied, in that order: +/// then . +/// +/// +/// There is deliberately no "is this already escaped?" shortcut. That guess cannot be made +/// safely from the shape of untrusted input — a value that happens to start and end with a +/// quote would skip escaping entirely, which is strictly worse than the missing-escape case +/// it replaces (see the weasel#416 postmortem). +/// +/// +internal static class SqlEscaping +{ + /// + /// Wrap as a bracketed T-SQL identifier, doubling any embedded + /// ] so it cannot terminate the bracket early. Equivalent to QUOTENAME() + /// evaluated client-side. + /// + public static string QuoteIdentifier(string name) + => string.Concat("[", name.Replace("]", "]]"), "]"); + + /// + /// The schema-qualified, bracket-escaped form of a table or other schema-scoped object: + /// [schema].[name]. Every Polecat code path that needs a qualified name in an + /// identifier position should compose it here rather than by hand. + /// + public static string QualifiedName(string schema, string name) + => string.Concat(QuoteIdentifier(schema), ".", QuoteIdentifier(name)); + + /// + /// Escape for embedding inside an existing pair of + /// single quotes (doubles ', adds no quotes of its own). Use when the surrounding + /// literal is already written into the SQL template. + /// + public static string LiteralBody(string value) + => value.Replace("'", "''"); + + /// + /// as a complete, quoted T-SQL string literal with embedded + /// ' doubled — 'value'. Prefer a bound parameter where the position allows + /// one; this is for the positions that cannot take a parameter, such as the object-name + /// argument to OBJECT_ID/COL_LENGTH and nested dynamic-SQL bodies. + /// + public static string Literal(string value) + => string.Concat("'", LiteralBody(value), "'"); +} diff --git a/src/Polecat/Projections/Flattened/EventDeleter.cs b/src/Polecat/Projections/Flattened/EventDeleter.cs index 81bfa8aa..aee4f495 100644 --- a/src/Polecat/Projections/Flattened/EventDeleter.cs +++ b/src/Polecat/Projections/Flattened/EventDeleter.cs @@ -44,7 +44,8 @@ public void Compile(Events.EventGraph events) $"Table {table.Identifier} must have a primary key column."); _compiledSql = - $"DELETE FROM [{table.Identifier.Schema}].[{table.Identifier.Name}] WHERE [{pkColumn}] = @p0;"; + $"DELETE FROM {SqlEscaping.QualifiedName(table.Identifier.Schema, table.Identifier.Name)} " + + $"WHERE {SqlEscaping.QuoteIdentifier(pkColumn)} = @p0;"; } public FlatTableSqlOperation CreateOperation(IEvent e) diff --git a/src/Polecat/Projections/Flattened/IColumnMap.cs b/src/Polecat/Projections/Flattened/IColumnMap.cs index 992779ba..3a0eb0e7 100644 --- a/src/Polecat/Projections/Flattened/IColumnMap.cs +++ b/src/Polecat/Projections/Flattened/IColumnMap.cs @@ -1,3 +1,5 @@ +using Polecat.Internal; + namespace Polecat.Projections.Flattened; /// @@ -127,8 +129,12 @@ public SetStringValueMap(string columnName, string value) public string ColumnName { get; } public bool RequiresInput => false; - public string UpdateExpression(string paramName) => $"[{ColumnName}] = '{_value}'"; - public string InsertExpression(string paramName) => $"'{_value}'"; + // #390: the configured value lands in a T-SQL string literal, so an embedded quote has to be + // doubled — otherwise it terminates the literal and the rest of the value is parsed as SQL. + public string UpdateExpression(string paramName) => + $"{SqlEscaping.QuoteIdentifier(ColumnName)} = {SqlEscaping.Literal(_value)}"; + + public string InsertExpression(string paramName) => SqlEscaping.Literal(_value); } /// diff --git a/src/Polecat/Projections/Flattened/StatementMap.cs b/src/Polecat/Projections/Flattened/StatementMap.cs index 59ab9bee..fde8509c 100644 --- a/src/Polecat/Projections/Flattened/StatementMap.cs +++ b/src/Polecat/Projections/Flattened/StatementMap.cs @@ -136,7 +136,7 @@ internal void Compile(Events.EventGraph events) // Build MERGE SQL var updateClauses = new List(); - var insertColumns = new List { $"[{pkColumn}]" }; + var insertColumns = new List { SqlEscaping.QuoteIdentifier(pkColumn) }; var insertValues = new List { "@p0" }; for (var i = 0; i < _columnMaps.Count; i++) @@ -156,7 +156,7 @@ internal void Compile(Events.EventGraph events) } updateClauses.Add(map.UpdateExpression(paramName)); - insertColumns.Add($"[{map.ColumnName}]"); + insertColumns.Add(SqlEscaping.QuoteIdentifier(map.ColumnName)); insertValues.Add(map.InsertExpression(paramName)); } @@ -166,8 +166,8 @@ internal void Compile(Events.EventGraph events) var insertVals = string.Join(", ", insertValues); _compiledSql = $""" - MERGE [{table.Identifier.Schema}].[{table.Identifier.Name}] AS target - USING (SELECT @p0 AS [{pkColumn}]) AS source ON target.[{pkColumn}] = source.[{pkColumn}] + MERGE {SqlEscaping.QualifiedName(table.Identifier.Schema, table.Identifier.Name)} AS target + USING (SELECT @p0 AS {SqlEscaping.QuoteIdentifier(pkColumn)}) AS source ON target.{SqlEscaping.QuoteIdentifier(pkColumn)} = source.{SqlEscaping.QuoteIdentifier(pkColumn)} WHEN MATCHED THEN UPDATE SET {updateSet} WHEN NOT MATCHED THEN INSERT ({insertCols}) VALUES ({insertVals}); """; diff --git a/src/Polecat/Schema/Identity/Sequences/HiloSequence.cs b/src/Polecat/Schema/Identity/Sequences/HiloSequence.cs index c688310d..2767c408 100644 --- a/src/Polecat/Schema/Identity/Sequences/HiloSequence.cs +++ b/src/Polecat/Schema/Identity/Sequences/HiloSequence.cs @@ -18,6 +18,10 @@ internal class HiloSequence : HiloSequenceBase { private readonly ConnectionFactory _connectionFactory; private readonly string _schemaName; + + /// #390: one escaped builder for the name, rather than seven hand-written copies. + private readonly string _hiloTable; + private readonly ResiliencePipeline _resilience; private readonly AutoCreate _autoCreate; private bool _tableEnsured; @@ -28,6 +32,7 @@ public HiloSequence(ConnectionFactory connectionFactory, string schemaName, stri { _connectionFactory = connectionFactory; _schemaName = schemaName; + _hiloTable = Polecat.Internal.SqlEscaping.QualifiedName(schemaName, HiloTable.TableName); _resilience = resilience; _autoCreate = autoCreate; } @@ -46,7 +51,7 @@ await _resilience.ExecuteAsync(async (state, ct) => await using var cmd = conn.CreateCommand(); cmd.CommandText = - $"UPDATE [{_schemaName}].[pc_hilo] SET hi_value = @floor WHERE entity_name = @name;"; + $"UPDATE {_hiloTable} SET hi_value = @floor WHERE entity_name = @name;"; cmd.Parameters.AddWithValue("@floor", state); cmd.Parameters.AddVarChar("@name", EntityName); await cmd.ExecuteNonQueryAsync(ct); @@ -149,7 +154,7 @@ private async Task TryGetNextHiAsync(SqlConnection conn, CancellationToken await using (var readCmd = conn.CreateCommand()) { readCmd.CommandText = - $"SELECT hi_value FROM [{_schemaName}].[pc_hilo] WHERE entity_name = @entity;"; + $"SELECT hi_value FROM {_hiloTable} WHERE entity_name = @entity;"; readCmd.Parameters.AddVarChar("@entity", EntityName); var raw = await readCmd.ExecuteScalarAsync(ct); currentHi = raw == null || raw == DBNull.Value ? null : Convert.ToInt64(raw); @@ -162,7 +167,7 @@ private async Task TryGetNextHiAsync(SqlConnection conn, CancellationToken { await using var insertCmd = conn.CreateCommand(); insertCmd.CommandText = - $"INSERT INTO [{_schemaName}].[pc_hilo] (entity_name, hi_value) VALUES (@entity, 0);"; + $"INSERT INTO {_hiloTable} (entity_name, hi_value) VALUES (@entity, 0);"; insertCmd.Parameters.AddVarChar("@entity", EntityName); await insertCmd.ExecuteNonQueryAsync(ct); return 0; @@ -179,7 +184,7 @@ private async Task TryGetNextHiAsync(SqlConnection conn, CancellationToken await using (var updateCmd = conn.CreateCommand()) { updateCmd.CommandText = - $"UPDATE [{_schemaName}].[pc_hilo] SET hi_value = @next WHERE entity_name = @entity AND hi_value = @current;"; + $"UPDATE {_hiloTable} SET hi_value = @next WHERE entity_name = @entity AND hi_value = @current;"; updateCmd.Parameters.AddWithValue("@next", nextHi); updateCmd.Parameters.AddVarChar("@entity", EntityName); updateCmd.Parameters.AddWithValue("@current", currentHi.Value); @@ -195,7 +200,7 @@ private long TryGetNextHiSync(SqlConnection conn) using (var readCmd = conn.CreateCommand()) { readCmd.CommandText = - $"SELECT hi_value FROM [{_schemaName}].[pc_hilo] WHERE entity_name = @entity;"; + $"SELECT hi_value FROM {_hiloTable} WHERE entity_name = @entity;"; readCmd.Parameters.AddVarChar("@entity", EntityName); var raw = readCmd.ExecuteScalar(); currentHi = raw == null || raw == DBNull.Value ? null : Convert.ToInt64(raw); @@ -207,7 +212,7 @@ private long TryGetNextHiSync(SqlConnection conn) { using var insertCmd = conn.CreateCommand(); insertCmd.CommandText = - $"INSERT INTO [{_schemaName}].[pc_hilo] (entity_name, hi_value) VALUES (@entity, 0);"; + $"INSERT INTO {_hiloTable} (entity_name, hi_value) VALUES (@entity, 0);"; insertCmd.Parameters.AddVarChar("@entity", EntityName); insertCmd.ExecuteNonQuery(); return 0; @@ -222,7 +227,7 @@ private long TryGetNextHiSync(SqlConnection conn) using (var updateCmd = conn.CreateCommand()) { updateCmd.CommandText = - $"UPDATE [{_schemaName}].[pc_hilo] SET hi_value = @next WHERE entity_name = @entity AND hi_value = @current;"; + $"UPDATE {_hiloTable} SET hi_value = @next WHERE entity_name = @entity AND hi_value = @current;"; updateCmd.Parameters.AddWithValue("@next", nextHi); updateCmd.Parameters.AddVarChar("@entity", EntityName); updateCmd.Parameters.AddWithValue("@current", currentHi.Value); diff --git a/src/Polecat/Storage/DocumentForeignKey.cs b/src/Polecat/Storage/DocumentForeignKey.cs index 4ed9385f..c1319be0 100644 --- a/src/Polecat/Storage/DocumentForeignKey.cs +++ b/src/Polecat/Storage/DocumentForeignKey.cs @@ -1,6 +1,7 @@ using System.Linq.Expressions; using System.Reflection; using Weasel.Core; +using Polecat.Internal; namespace Polecat.Storage; @@ -44,9 +45,9 @@ internal string[] ToDdlStatements(DocumentMapping parentMapping, DocumentMapping { var schema = parentMapping.DatabaseSchemaName; var table = parentMapping.TableName; - var qualifiedTable = $"[{schema}].[{table}]"; + var qualifiedTable = SqlEscaping.QualifiedName(schema, table); var colName = DocumentIndex.ColumnNameForPath(JsonPath); - var refTable = $"[{referenceMapping.DatabaseSchemaName}].[{referenceMapping.TableName}]"; + var refTable = SqlEscaping.QualifiedName(referenceMapping.DatabaseSchemaName, referenceMapping.TableName); var constraintName = ConstraintName ?? DeriveConstraintName(table, colName); // Determine SQL type from the reference document's ID type @@ -63,9 +64,11 @@ internal string[] ToDdlStatements(DocumentMapping parentMapping, DocumentMapping // json storage too (Guid ids still fall back to CAST — RETURNING has no uniqueidentifier). var computedExpr = DocumentIndex.ComputedColumnExpression( JsonPath, sqlType, IndexCasing.Default, DocumentIndex.UsesNativeJson(parentMapping)); + // #390: COL_LENGTH takes its object name as a string literal — escape for that position + // rather than re-composing an unquoted `schema.table` that diverges from qualifiedTable. statements.Add($""" - IF COL_LENGTH('{schema}.{table}', '{colName}') IS NULL - ALTER TABLE {qualifiedTable} ADD [{colName}] AS {computedExpr} PERSISTED; + IF COL_LENGTH({SqlEscaping.Literal(qualifiedTable)}, {SqlEscaping.Literal(colName)}) IS NULL + ALTER TABLE {qualifiedTable} ADD {SqlEscaping.QuoteIdentifier(colName)} AS {computedExpr} PERSISTED; """); // Build ON DELETE clause @@ -81,15 +84,19 @@ IF COL_LENGTH('{schema}.{table}', '{colName}') IS NULL var isConjoined = parentMapping.TenancyStyle == TenancyStyle.Conjoined && referenceMapping.TenancyStyle == TenancyStyle.Conjoined; - var fkCheck = $"NOT EXISTS (SELECT 1 FROM sys.foreign_keys WHERE name = '{constraintName}' AND parent_object_id = OBJECT_ID('{qualifiedTable}'))"; + // #390: ConstraintName is a public-API argument used in both a literal and an identifier + // position below; each gets the escape its position requires. + var fkCheck = + $"NOT EXISTS (SELECT 1 FROM sys.foreign_keys WHERE name = {SqlEscaping.Literal(constraintName)} " + + $"AND parent_object_id = OBJECT_ID({SqlEscaping.Literal(qualifiedTable)}))"; if (isConjoined) { statements.Add($""" IF {fkCheck} ALTER TABLE {qualifiedTable} - ADD CONSTRAINT [{constraintName}] - FOREIGN KEY (tenant_id, [{colName}]) REFERENCES {refTable} (tenant_id, id){onDeleteClause}; + ADD CONSTRAINT {SqlEscaping.QuoteIdentifier(constraintName)} + FOREIGN KEY (tenant_id, {SqlEscaping.QuoteIdentifier(colName)}) REFERENCES {refTable} (tenant_id, id){onDeleteClause}; """); } else @@ -97,8 +104,8 @@ ADD CONSTRAINT [{constraintName}] statements.Add($""" IF {fkCheck} ALTER TABLE {qualifiedTable} - ADD CONSTRAINT [{constraintName}] - FOREIGN KEY ([{colName}]) REFERENCES {refTable} (id){onDeleteClause}; + ADD CONSTRAINT {SqlEscaping.QuoteIdentifier(constraintName)} + FOREIGN KEY ({SqlEscaping.QuoteIdentifier(colName)}) REFERENCES {refTable} (id){onDeleteClause}; """); } diff --git a/src/Polecat/Storage/DocumentIndex.cs b/src/Polecat/Storage/DocumentIndex.cs index c4324a2e..1fb5c029 100644 --- a/src/Polecat/Storage/DocumentIndex.cs +++ b/src/Polecat/Storage/DocumentIndex.cs @@ -1,5 +1,6 @@ using System.Linq.Expressions; using System.Reflection; +using Polecat.Internal; namespace Polecat.Storage; @@ -170,7 +171,7 @@ internal string[] ToDdlStatements(DocumentMapping mapping) { var schema = mapping.DatabaseSchemaName; var table = mapping.TableName; - var qualifiedTable = $"[{schema}].[{table}]"; + var qualifiedTable = SqlEscaping.QualifiedName(schema, table); var name = GetIndexName(table); var unique = IsUnique ? "UNIQUE " : ""; @@ -183,9 +184,11 @@ internal string[] ToDdlStatements(DocumentMapping mapping) var sqlType = ResolveSqlType(path, mapping.ResolveClrMemberType(path)); var castedExpr = ComputedColumnExpression(path, sqlType, Casing, UsesNativeJson(mapping)); + // #390: COL_LENGTH takes the object name as a *string*, so the same qualified name that + // appears bare in ALTER TABLE has to be escaped a second time for the literal position. statements.Add($""" - IF COL_LENGTH('{schema}.{table}', '{colName}') IS NULL - ALTER TABLE {qualifiedTable} ADD [{colName}] AS {castedExpr} PERSISTED; + IF COL_LENGTH({SqlEscaping.Literal(qualifiedTable)}, {SqlEscaping.Literal(colName)}) IS NULL + ALTER TABLE {qualifiedTable} ADD {SqlEscaping.QuoteIdentifier(colName)} AS {castedExpr} PERSISTED; """); } @@ -197,8 +200,8 @@ IF COL_LENGTH('{schema}.{table}', '{colName}') IS NULL var castedExpr = ComputedColumnExpression(path, sqlType, IndexCasing.Default, UsesNativeJson(mapping)); statements.Add($""" - IF COL_LENGTH('{schema}.{table}', '{colName}') IS NULL - ALTER TABLE {qualifiedTable} ADD [{colName}] AS {castedExpr} PERSISTED; + IF COL_LENGTH({SqlEscaping.Literal(qualifiedTable)}, {SqlEscaping.Literal(colName)}) IS NULL + ALTER TABLE {qualifiedTable} ADD {SqlEscaping.QuoteIdentifier(colName)} AS {castedExpr} PERSISTED; """); } @@ -213,20 +216,23 @@ IF COL_LENGTH('{schema}.{table}', '{colName}') IS NULL { var colName = ColumnNameForPath(path, Casing); var sortDir = SortOrder == SortOrder.Descending ? " DESC" : ""; - indexColumns.Add($"[{colName}]{sortDir}"); + indexColumns.Add($"{SqlEscaping.QuoteIdentifier(colName)}{sortDir}"); } var columnList = string.Join(", ", indexColumns); var include = IncludeColumns.Length > 0 ? " INCLUDE (" + string.Join(", ", - IncludeColumns.Select(p => $"[{ColumnNameForPath(p, IndexCasing.Default)}]")) + ")" + IncludeColumns.Select(p => SqlEscaping.QuoteIdentifier(ColumnNameForPath(p, IndexCasing.Default)))) + ")" : ""; var where = !string.IsNullOrEmpty(Predicate) ? $" WHERE {Predicate}" : ""; + // #390: IndexName is a public-API argument (Index(..., indexName)) that lands in a string + // literal here and a bracketed identifier one line later — both positions need escaping, and + // they take different escapes. statements.Add($""" - IF NOT EXISTS (SELECT 1 FROM sys.indexes WHERE name = '{name}' - AND object_id = OBJECT_ID('{qualifiedTable}')) - CREATE {unique}NONCLUSTERED INDEX [{name}] ON {qualifiedTable} ({columnList}){include}{where}; + IF NOT EXISTS (SELECT 1 FROM sys.indexes WHERE name = {SqlEscaping.Literal(name)} + AND object_id = OBJECT_ID({SqlEscaping.Literal(qualifiedTable)})) + CREATE {unique}NONCLUSTERED INDEX {SqlEscaping.QuoteIdentifier(name)} ON {qualifiedTable} ({columnList}){include}{where}; """); return statements.ToArray(); diff --git a/src/Polecat/Storage/DocumentMapping.cs b/src/Polecat/Storage/DocumentMapping.cs index e75435b7..af01962d 100644 --- a/src/Polecat/Storage/DocumentMapping.cs +++ b/src/Polecat/Storage/DocumentMapping.cs @@ -94,7 +94,7 @@ public DocumentMapping(Type documentType, StoreOptions options) } var tableName = $"pc_doc_{documentType.Name.ToLowerInvariant()}"; - QualifiedTableName = $"[{options.DatabaseSchemaName}].[{tableName}]"; + QualifiedTableName = Internal.SqlEscaping.QualifiedName(options.DatabaseSchemaName, tableName); TableName = tableName; DatabaseSchemaName = options.DatabaseSchemaName; DotNetTypeName = $"{documentType.FullName}, {documentType.Assembly.GetName().Name}"; diff --git a/src/Polecat/Storage/JsonIndex.cs b/src/Polecat/Storage/JsonIndex.cs index 0816e4a9..f7b27724 100644 --- a/src/Polecat/Storage/JsonIndex.cs +++ b/src/Polecat/Storage/JsonIndex.cs @@ -1,4 +1,5 @@ using System.Linq.Expressions; +using Polecat.Internal; namespace Polecat.Storage; @@ -68,13 +69,11 @@ internal string[] ToDdlStatements(DocumentMapping mapping) "Set UseNativeJsonType = true (SQL Server 2025+), or use a computed-column Index(...) instead."); } - var schema = mapping.DatabaseSchemaName; - var table = mapping.TableName; - var qualifiedTable = $"[{schema}].[{table}]"; - var name = GetIndexName(table); + var qualifiedTable = SqlEscaping.QualifiedName(mapping.DatabaseSchemaName, mapping.TableName); + var name = GetIndexName(mapping.TableName); var forClause = JsonPaths.Length > 0 - ? " FOR (" + string.Join(", ", JsonPaths.Select(p => $"'{p}'")) + ")" + ? " FOR (" + string.Join(", ", JsonPaths.Select(p => SqlEscaping.Literal(p))) + ")" : ""; var withOptions = new List(); @@ -89,8 +88,8 @@ internal string[] ToDdlStatements(DocumentMapping mapping) [ $""" SET QUOTED_IDENTIFIER ON; - IF NOT EXISTS (SELECT 1 FROM sys.json_indexes WHERE object_id = OBJECT_ID('{qualifiedTable}')) - CREATE JSON INDEX [{name}] ON {qualifiedTable} (data){forClause}{withClause}; + IF NOT EXISTS (SELECT 1 FROM sys.json_indexes WHERE object_id = OBJECT_ID({SqlEscaping.Literal(qualifiedTable)})) + CREATE JSON INDEX {SqlEscaping.QuoteIdentifier(name)} ON {qualifiedTable} (data){forClause}{withClause}; """ ]; } diff --git a/src/Polecat/Storage/MasterTableTenancy.cs b/src/Polecat/Storage/MasterTableTenancy.cs index 38940cb9..b68795c7 100644 --- a/src/Polecat/Storage/MasterTableTenancy.cs +++ b/src/Polecat/Storage/MasterTableTenancy.cs @@ -52,7 +52,7 @@ internal MasterTableTenancy(StoreOptions options, string masterConnectionString, /// /// The fully-qualified master table name, e.g. [dbo].[pc_tenants]. /// - public string QualifiedTableName => $"[{_schemaName}].[{TableName}]"; + public string QualifiedTableName => Internal.SqlEscaping.QualifiedName(_schemaName, TableName); /// /// Always — the tenant database set is read