From 6bb0108d30477f8a88d17a3ba0c6914b134dcacc Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 13:43:53 -0400 Subject: [PATCH] feat(sql): ISqlDialect + SqlServerDialect with golden catalog SQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dialect owns the two things ADO.NET's System.Data.Common base types cannot abstract: identifier quoting and the metadata-catalog SQL. It is the driver's SQL-injection boundary — values always bind as DbParameters, identifiers cannot, so the few that reach a command text are catalog- sourced and pass through QuoteIdentifier. QuoteIdentifier brackets one identifier part and doubles embedded ] (the only metacharacter that can terminate a bracketed identifier early); rejects null/empty/whitespace, >128 chars (T-SQL sysname ceiling), and any control character (incl. NUL) as ArgumentException, without echoing the untrusted value into the message. MapColumnType folds the design §3.7 families case-insensitively and never throws — an unrecognised family falls back to String so a browse over an exotic column still renders; timestamp/rowversion and time are deliberately NOT mapped to DateTime. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../ISqlDialect.cs | 68 ++++++++++ .../SqlServerDialect.cs | 127 ++++++++++++++++++ .../SqlServerDialectTests.cs | 107 +++++++++++++++ 3 files changed, 302 insertions(+) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs new file mode 100644 index 00000000..076bd3b4 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs @@ -0,0 +1,68 @@ +using System.Data.Common; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// The provider seam (design §2.2). ADO.NET's base types abstract +/// connections, commands, parameters and readers; they do not abstract the two things that differ +/// per backend — identifier quoting and the metadata-catalog SQL. Those live here, so no +/// dialect assumption leaks into the reader, the poll planner, or the schema browser. +/// This interface is the driver's SQL-injection boundary. Every runtime value is +/// bound as a and never reaches a command text. Identifiers cannot be +/// parameterized in SQL, so the few that must appear as text are sourced only from catalog-validated +/// (INFORMATION_SCHEMA) names and emitted through . Any code path that +/// builds SQL by concatenating an authored tag field without going through +/// is a defect (design §8.1). +/// Public because Driver.Sql.Browser consumes it — the catalog SQL is the browse +/// engine, so it is shared rather than duplicated. Implementations own their provider package; +/// no provider-specific type appears in this signature ( is the abstract +/// ). +/// +public interface ISqlDialect +{ + /// Which backend this dialect speaks. v1 constructs only. + SqlProvider Provider { get; } + + /// + /// The provider's factory singleton, used to create connections/commands/parameters. Deliberately the + /// abstract base type so consumers never bind to a concrete provider package. + /// + DbProviderFactory Factory { get; } + + /// + /// Quotes one identifier part so it can be safely embedded in a command text, escaping the + /// dialect's own quote character. Rejects anything that cannot be a real catalog identifier rather + /// than emitting it. + /// One part only. A multi-part name such as dbo.TagValues must be split by the + /// caller and each part quoted separately; passing the dotted form yields a single (nonexistent) + /// identifier — safe, but not what the caller meant. + /// + /// The bare, unquoted identifier — as returned by the catalog, not pre-quoted. + /// The quoted identifier, ready to concatenate into a command text. + /// The value cannot be a valid catalog identifier. + string QuoteIdentifier(string ident); + + /// Cheapest statement that proves the connection is alive (SELECT 1; Oracle needs FROM DUAL). + string LivenessSql { get; } + + /// Catalog query listing schemas — the browser's RootAsync level. Takes no parameters. + string ListSchemasSql { get; } + + /// Catalog query listing tables + views in one schema — the browser's schema-expand level. Bind @schema. + string ListTablesSql { get; } + + /// Catalog query listing columns of one table — the browser's table-expand / attributes level. Bind @schema and @table. + string ListColumnsSql { get; } + + /// + /// Folds a catalog data-type family name (as INFORMATION_SCHEMA.COLUMNS.DATA_TYPE reports it — + /// e.g. nvarchar, never nvarchar(50)) onto a . + /// Must never throw: a browse over a table holding one exotic column must still render. + /// An unrecognised family falls back to . + /// + /// The bare SQL type family name; matched case-insensitively. + /// The mapped driver data type, or when unrecognised. + DriverDataType MapColumnType(string sqlDataType); +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs new file mode 100644 index 00000000..bd7eebbd --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs @@ -0,0 +1,127 @@ +using System.Data.Common; +using Microsoft.Data.SqlClient; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// Microsoft SQL Server dialect (design §2.2) — the only provider v1 constructs. Bracket quoting, +/// INFORMATION_SCHEMA catalog SQL, and the design §3.7 type-family map. +/// Stateless and immutable, so a single instance is safely shared by the driver's poll reader and +/// the AdminUI browse session. +/// +public sealed class SqlServerDialect : ISqlDialect +{ + /// + /// T-SQL's regular-identifier ceiling (sysname = nvarchar(128)). Enforced because a + /// longer string cannot name a real catalog object — it is either a config mistake or padding, and + /// rejecting it keeps the quoted output bounded. + /// + internal const int MaxIdentifierLength = 128; + + /// + public SqlProvider Provider => SqlProvider.SqlServer; + + /// + public DbProviderFactory Factory => SqlClientFactory.Instance; + + /// + public string LivenessSql => "SELECT 1"; + + /// + public string ListSchemasSql => + "SELECT DISTINCT TABLE_SCHEMA FROM INFORMATION_SCHEMA.TABLES ORDER BY TABLE_SCHEMA"; + + /// + public string ListTablesSql => + "SELECT TABLE_NAME, TABLE_TYPE FROM INFORMATION_SCHEMA.TABLES " + + "WHERE TABLE_SCHEMA = @schema ORDER BY TABLE_NAME"; + + /// + public string ListColumnsSql => + "SELECT COLUMN_NAME, DATA_TYPE, IS_NULLABLE FROM INFORMATION_SCHEMA.COLUMNS " + + "WHERE TABLE_SCHEMA = @schema AND TABLE_NAME = @table ORDER BY ORDINAL_POSITION"; + + /// + /// Brackets one identifier part, doubling any embedded ] — the T-SQL escape rule, and the only + /// metacharacter that could terminate a bracketed identifier early. [, quotes, semicolons and + /// comment markers are inert inside brackets and are therefore passed through unchanged. + /// + /// + /// Rejects (all as ): null / empty / all-whitespace; longer + /// than ; any control character (which covers embedded NUL, and the + /// C0/C1 ranges that can truncate or corrupt a logged/audited statement). + /// The rejection messages deliberately do not echo the offending value — it is untrusted + /// input and this exception's text reaches the driver log. + /// Rejection is a backstop, not the primary defence: an identifier only reaches here after being + /// validated against the live catalog (design §8.1). + /// + /// The bare, unquoted identifier part. + /// The bracket-quoted identifier. + /// The value cannot be a valid SQL Server identifier. + public string QuoteIdentifier(string ident) + { + if (string.IsNullOrWhiteSpace(ident)) + throw new ArgumentException( + "A SQL identifier must be a non-empty, non-whitespace name.", nameof(ident)); + + if (ident.Length > MaxIdentifierLength) + throw new ArgumentException( + $"A SQL Server identifier may not exceed {MaxIdentifierLength} characters (got {ident.Length}).", + nameof(ident)); + + foreach (var ch in ident) + { + if (char.IsControl(ch)) + throw new ArgumentException( + "A SQL identifier may not contain control characters (including NUL).", nameof(ident)); + } + + return string.Concat("[", ident.Replace("]", "]]", StringComparison.Ordinal), "]"); + } + + /// + /// Folds an INFORMATION_SCHEMA.COLUMNS.DATA_TYPE family name onto a + /// per design §3.7. + /// + /// + /// Precision caveat: decimal / numeric / money / smallmoney + /// collapse to in v1. A .NET decimal carries more + /// significant digits than a double, so a high-precision column (e.g. decimal(38,10)) + /// loses low-order digits. Authoring an explicit "type": "String" on the tag preserves the exact + /// textual value when that matters. + /// Fallback: an unrecognised family maps to and never + /// throws — a schema browse must render a table containing an exotic column (xml, + /// geography, hierarchyid, sql_variant, a CLR UDT, varbinary) rather than + /// crash on it. Null/blank input takes the same fallback. + /// Deliberate non-mappings: SQL Server's timestamp/rowversion is an opaque + /// 8-byte row version, not a date — it falls back to + /// rather than being mistaken for a . time is a + /// time-of-day span, likewise not a point in time, and takes the same fallback. + /// tinyint widens to (it is unsigned 0–255 in T-SQL, so + /// a signed 8-bit type would not hold it, and the driver type set has no Byte). + /// + /// The bare SQL type family name; matched case-insensitively. + /// The mapped driver data type, or when unrecognised. + public DriverDataType MapColumnType(string sqlDataType) + { + if (string.IsNullOrWhiteSpace(sqlDataType)) return DriverDataType.String; + + return sqlDataType.Trim().ToLowerInvariant() switch + { + "bit" or "boolean" => DriverDataType.Boolean, + "tinyint" or "smallint" => DriverDataType.Int16, + "int" or "integer" => DriverDataType.Int32, + "bigint" => DriverDataType.Int64, + "real" => DriverDataType.Float32, + "float" or "double" or "double precision" + or "decimal" or "numeric" or "money" or "smallmoney" => DriverDataType.Float64, + "char" or "nchar" or "varchar" or "nvarchar" or "text" or "ntext" + or "uniqueidentifier" => DriverDataType.String, + "date" or "datetime" or "datetime2" or "smalldatetime" + or "datetimeoffset" => DriverDataType.DateTime, + _ => DriverDataType.String, + }; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs new file mode 100644 index 00000000..9fecd3ba --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs @@ -0,0 +1,107 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// Golden tests for the driver's SQL-injection boundary. Values are always bound as +/// DbParameters; identifiers cannot be, so is the +/// only thing standing between a catalog-sourced name and a command text. These tests pin the escape +/// rule, the rejection rules, and the exact catalog SQL. +/// +public class SqlServerDialectTests +{ + [Fact] + public void QuoteIdentifier_bracketsAndEscapesEmbeddedBrackets() + { + var d = new SqlServerDialect(); + d.QuoteIdentifier("Speed").ShouldBe("[Speed]"); + d.QuoteIdentifier("a]b").ShouldBe("[a]]b]"); // ] doubled per T-SQL rules + } + + [Fact] + public void QuoteIdentifier_rejectsControlCharsAndNul() + => Should.Throw(() => new SqlServerDialect().QuoteIdentifier("x\0y")); + + [Theory] + [InlineData("bit", DriverDataType.Boolean)] + [InlineData("int", DriverDataType.Int32)] + [InlineData("bigint", DriverDataType.Int64)] + [InlineData("real", DriverDataType.Float32)] + [InlineData("float", DriverDataType.Float64)] + [InlineData("decimal", DriverDataType.Float64)] + [InlineData("nvarchar", DriverDataType.String)] + [InlineData("datetime2", DriverDataType.DateTime)] + [InlineData("uniqueidentifier", DriverDataType.String)] + public void MapColumnType_mapsFamilies(string sql, DriverDataType expected) + => new SqlServerDialect().MapColumnType(sql).ShouldBe(expected); + + [Fact] + public void CatalogSql_isInformationSchemaAndParameterized() + { + var d = new SqlServerDialect(); + d.LivenessSql.ShouldBe("SELECT 1"); + d.ListTablesSql.ShouldContain("INFORMATION_SCHEMA.TABLES"); + d.ListTablesSql.ShouldContain("@schema"); // never string-interpolated + d.ListColumnsSql.ShouldContain("@table"); + } + + // ---- extra guards on the injection boundary (beyond the plan's golden set) ---- + + [Fact] + public void Dialect_identifiesItselfAsSqlServer_andExposesTheAbstractFactory() + { + var d = new SqlServerDialect(); + d.Provider.ShouldBe(SqlProvider.SqlServer); + d.Factory.ShouldNotBeNull(); + // The seam must not leak Microsoft.Data.SqlClient into its signature. + typeof(ISqlDialect).GetProperty(nameof(ISqlDialect.Factory))! + .PropertyType.ShouldBe(typeof(System.Data.Common.DbProviderFactory)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("\t")] + public void QuoteIdentifier_rejectsNullEmptyAndWhitespace(string? ident) + => Should.Throw(() => new SqlServerDialect().QuoteIdentifier(ident!)); + + [Fact] + public void QuoteIdentifier_escapesAnIdentifierThatIsNothingButABracket() + => new SqlServerDialect().QuoteIdentifier("]").ShouldBe("[]]]"); // T-SQL for the 1-char name "]" + + [Fact] + public void QuoteIdentifier_acceptsExactly128Chars_andRejects129() + { + var d = new SqlServerDialect(); + d.QuoteIdentifier(new string('a', 128)).ShouldBe("[" + new string('a', 128) + "]"); + Should.Throw(() => d.QuoteIdentifier(new string('a', 129))); + } + + [Fact] + public void QuoteIdentifier_neutralisesAClassicInjectionPayload() + { + // A hostile "column name" cannot escape the brackets: the only metacharacter that could is ], + // and it is doubled. The result is a single (nonexistent) identifier, never executable SQL. + new SqlServerDialect().QuoteIdentifier("x] ; DROP TABLE Users --") + .ShouldBe("[x]] ; DROP TABLE Users --]"); + } + + [Theory] + [InlineData("NVARCHAR", DriverDataType.String)] + [InlineData("BiT", DriverDataType.Boolean)] + public void MapColumnType_isCaseInsensitive(string sql, DriverDataType expected) + => new SqlServerDialect().MapColumnType(sql).ShouldBe(expected); + + [Theory] + [InlineData("geography")] + [InlineData("sql_variant")] + [InlineData("timestamp")] // SQL Server rowversion — binary, deliberately NOT a DateTime + [InlineData("")] + [InlineData(null)] + public void MapColumnType_unknownFamilyFallsBackToString_andNeverThrows(string? sql) + => new SqlServerDialect().MapColumnType(sql!).ShouldBe(DriverDataType.String); +}