From 74594e6083ed91ffa790c56d9e847ca8e2a6b59b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:58:04 -0400 Subject: [PATCH] fix(sql-dialect): enforce Postgres/Oracle identifier ceilings in UTF-8 bytes (Driver.Sql.Dialect-001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NAMEDATALEN-1 (63) and Oracle 12.2's 128 are byte limits in the database encoding, not character counts, so a 32-character Cyrillic name passed ident.Length and was then silently truncated (Postgres) or rejected by the server (Oracle). Both dialects now measure Encoding.UTF8.GetByteCount and say 'bytes' in the rejection. SqlServerDialect deliberately keeps ident.Length — sysname is nvarchar(128), a real character limit — pinned by a control test. --- .../OracleDialect.cs | 20 ++++++++++-- .../PostgresDialect.cs | 21 ++++++++++-- .../SqlServerDialect.cs | 4 +++ .../OracleDialectTests.cs | 20 +++++++++++- .../PostgresDialectTests.cs | 32 ++++++++++++++++++- .../SqlServerDialectTests.cs | 13 ++++++++ 6 files changed, 102 insertions(+), 8 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs index a6fd198e..8d12a01c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs @@ -1,5 +1,6 @@ using System.Data.Common; using System.Globalization; +using System.Text; using Oracle.ManagedDataAccess.Client; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; @@ -24,6 +25,9 @@ public sealed class OracleDialect : ISqlDialect /// /// Oracle 12.2+'s identifier ceiling. 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. + /// The unit is BYTES in the database character set, not characters — see + /// . Practically that means UTF-8 (AL32UTF8) on any modern estate, + /// so a 65-character Cyrillic name is already over the limit. /// internal const int MaxIdentifierLength = 128; @@ -187,8 +191,14 @@ public sealed class OracleDialect : ISqlDialect /// (ALL_TAB_COLUMNS.column_name, i.e. what the server actually stored) before anything reaches /// this method — an operator who authored speed against an Oracle SPEED column resolves /// to SPEED at Initialize, and it is SPEED that gets quoted. + /// The length ceiling is measured in UTF-8 bytes, not characters. Oracle's 128 bounds the + /// name in the database character set (AL32UTF8 on any modern estate), so counting + /// UTF-16 code units would accept a 65-character Cyrillic name — 130 bytes — and let it fail at the + /// server with ORA-00972 rather than here, where the message can name the tag. + /// SqlServerDialect deliberately does not do this: T-SQL's sysname is + /// nvarchar(128), a genuine 128-character limit. /// Rejects (all as ): null / empty / all-whitespace; longer - /// than ; any control character (which covers embedded NUL, and the + /// than bytes; any control character (which covers embedded NUL, and the /// C0/C1 ranges that can truncate or corrupt a logged/audited statement); and any Unicode /// character — bidi overrides, zero-width spaces, soft hyphen, /// BOM. Those last cannot escape the quotes, but they spoof the rendered text of a log line or @@ -209,9 +219,13 @@ public sealed class OracleDialect : ISqlDialect throw new ArgumentException( "A SQL identifier must be a non-empty, non-whitespace name.", nameof(ident)); - if (ident.Length > MaxIdentifierLength) + // Byte count, not ident.Length: Oracle's 128 is measured in the database character set, so a + // 65-character Cyrillic name is 130 UTF-8 bytes and the server answers ORA-00972. Counting UTF-16 + // code units would send it anyway, failing at the server for a reason this dialect could have named. + var byteLength = Encoding.UTF8.GetByteCount(ident); + if (byteLength > MaxIdentifierLength) throw new ArgumentException( - $"An Oracle identifier may not exceed {MaxIdentifierLength} characters (got {ident.Length}).", + $"An Oracle identifier may not exceed {MaxIdentifierLength} bytes (got {byteLength}).", nameof(ident)); foreach (var ch in ident) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs index eaa5a9b6..d2221f3b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs @@ -1,5 +1,6 @@ using System.Data.Common; using System.Globalization; +using System.Text; using Npgsql; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; @@ -20,6 +21,9 @@ public sealed class PostgresDialect : ISqlDialect /// PostgreSQL's identifier ceiling — NAMEDATALEN - 1, 63 by default. Enforced because the /// server silently truncates anything longer, so a 70-character authored name would resolve to /// a different object than it spells; rejecting it surfaces the config mistake instead. + /// The unit is BYTES in the database encoding, not characters — see + /// . Practically that means UTF-8 on any modern estate, so a 32-character + /// Cyrillic name is already over the limit. /// internal const int MaxIdentifierLength = 63; @@ -149,12 +153,19 @@ public sealed class PostgresDialect : ISqlDialect /// Rejects (all as , mirroring /// SqlServerDialect.QuoteIdentifier so a test written against one dialect fails the same way /// against the other): null / empty / all-whitespace; longer than - /// ; any control character (which covers embedded NUL and the C0/C1 + /// bytes; any control character (which covers embedded NUL and the C0/C1 /// ranges that can truncate or corrupt a logged/audited statement); and any Unicode /// character — bidi overrides, zero-width spaces, soft hyphen, /// BOM. Those last cannot escape the quotes, but they spoof the rendered text of a log line /// or an AdminUI label while comparing byte-different from the real catalog name (the Trojan-Source /// class, CVE-2021-42574). + /// The length ceiling is measured in UTF-8 bytes, not characters — NAMEDATALEN + /// bounds the stored name in the database encoding, and every modern estate is UTF-8. Counting UTF-16 + /// code units (ident.Length) would accept a 32-character Cyrillic name, which is 64 bytes and + /// is therefore truncated server-side to a name that denotes a different object — precisely + /// the silent-truncation failure this rule exists to prevent. SqlServerDialect deliberately does + /// not do this: T-SQL's sysname is nvarchar(128), a genuine 128-character limit, + /// so counting characters is correct there and counting bytes would reject legal names. /// NUL deserves its own note here: Postgres rejects a NUL byte anywhere in a query string /// outright, so passing one through would fail the whole statement rather than just the one /// identifier. @@ -175,9 +186,13 @@ public sealed class PostgresDialect : ISqlDialect throw new ArgumentException( "A SQL identifier must be a non-empty, non-whitespace name.", nameof(ident)); - if (ident.Length > MaxIdentifierLength) + // Byte count, not ident.Length: NAMEDATALEN bounds the stored name in the database encoding, so a + // 32-character Cyrillic name is 64 UTF-8 bytes and the server truncates it — the exact silent + // wrong-object outcome this check exists to prevent, and one a UTF-16 code-unit count waves through. + var byteLength = Encoding.UTF8.GetByteCount(ident); + if (byteLength > MaxIdentifierLength) throw new ArgumentException( - $"A PostgreSQL identifier may not exceed {MaxIdentifierLength} characters (got {ident.Length}).", + $"A PostgreSQL identifier may not exceed {MaxIdentifierLength} bytes (got {byteLength}).", nameof(ident)); foreach (var ch in ident) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs index aafce4e7..b1dd602f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs @@ -18,6 +18,10 @@ 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. + /// The unit here is genuinely CHARACTERS (UTF-16 code units, which is what + /// nvarchar(128) counts), deliberately unlike PostgresDialect and OracleDialect, + /// whose ceilings are byte limits in the database encoding. Measuring bytes here would reject a legal + /// 128-character non-ASCII name. /// internal const int MaxIdentifierLength = 128; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs index e8b0c375..1fbe2bfd 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs @@ -165,13 +165,31 @@ public class OracleDialectTests => Should.Throw(() => new OracleDialect().QuoteIdentifier(ident)); [Fact] - public void QuoteIdentifier_acceptsExactly128Chars_andRejects129() + public void QuoteIdentifier_acceptsExactly128Bytes_andRejects129() { var d = new OracleDialect(); d.QuoteIdentifier(new string('a', 128)).ShouldBe("\"" + new string('a', 128) + "\""); Should.Throw(() => d.QuoteIdentifier(new string('a', 129))); } + /// + /// Oracle 12.2's 128 is a byte ceiling in the database character set (UTF-8 on any modern + /// estate), so a 65-character Cyrillic name is 130 bytes and the server rejects it with + /// ORA-00972. Measuring UTF-16 code units here would pass it through to fail at the server + /// instead — for a reason this dialect could have named. + /// + [Fact] + public void QuoteIdentifier_measuresTheCeilingInUtf8Bytes_notCharacters() + { + var d = new OracleDialect(); + + // 64 Cyrillic characters = 128 UTF-8 bytes — exactly at the limit. + d.QuoteIdentifier(new string('б', 64)).ShouldBe("\"" + new string('б', 64) + "\""); + + // 65 characters = 130 bytes. A character-counting ceiling waves this through. + Should.Throw(() => d.QuoteIdentifier(new string('б', 65))); + } + // ---- liveness + row limit ---- [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs index 0ca35ec1..d68f0594 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs @@ -69,13 +69,43 @@ public class PostgresDialectTests // NAMEDATALEN - 1: a longer string cannot name a real Postgres object (the server truncates it), so it // is a config mistake rather than something to quote and send. [Fact] - public void QuoteIdentifier_acceptsExactly63Chars_andRejects64() + public void QuoteIdentifier_acceptsExactly63Bytes_andRejects64() { var d = new PostgresDialect(); d.QuoteIdentifier(new string('a', 63)).ShouldBe("\"" + new string('a', 63) + "\""); Should.Throw(() => d.QuoteIdentifier(new string('a', 64))); } + /// + /// NAMEDATALEN - 1 is 63 bytes in the database encoding, not 63 characters — so a + /// 32-character Cyrillic name is 64 UTF-8 bytes and the server truncates it to something that names a + /// different object. A ceiling measured in UTF-16 code units lets that name straight through, which + /// is precisely the silent-truncation failure this rule documents itself as preventing. + /// + [Fact] + public void QuoteIdentifier_measuresTheCeilingInUtf8Bytes_notCharacters() + { + var d = new PostgresDialect(); + + // 31 Cyrillic characters = 62 UTF-8 bytes — inside the limit, and well inside a character count. + d.QuoteIdentifier(new string('б', 31)).ShouldBe("\"" + new string('б', 31) + "\""); + + // 32 characters = 64 bytes. Under a character-counting ceiling this passes; the server truncates it. + Should.Throw(() => d.QuoteIdentifier(new string('б', 32))); + } + + /// A non-BMP character is 4 UTF-8 bytes (and 2 UTF-16 code units) — 16 of them fill the limit. + [Fact] + public void QuoteIdentifier_countsASupplementaryCharacterAsItsFourUtf8Bytes() + { + var d = new PostgresDialect(); + // U+1D400 MATHEMATICAL BOLD CAPITAL A, as a surrogate pair. + const string a = "\U0001D400"; + d.QuoteIdentifier(string.Concat(Enumerable.Repeat(a, 15))).ShouldNotBeNullOrEmpty(); // 60 bytes + Should.Throw( + () => d.QuoteIdentifier(string.Concat(Enumerable.Repeat(a, 16)))); // 64 bytes + } + [Fact] public void QuoteIdentifier_neutralisesAClassicInjectionPayload() { 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 index e10995bc..8545d423 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs @@ -117,6 +117,19 @@ public class SqlServerDialectTests Should.Throw(() => d.QuoteIdentifier(new string('a', 129))); } + /// + /// The falsifiability control for the Postgres/Oracle byte ceilings: T-SQL's sysname is + /// nvarchar(128), a genuine 128-character (UTF-16 code unit) limit, so a 128-character + /// Cyrillic name — 256 UTF-8 bytes — is legal here and must still be accepted. A byte rule applied + /// uniformly across the three dialects would reject it, and only this test would say so. + /// + [Fact] + public void QuoteIdentifier_countsCharacters_notBytes_becauseSysnameIsNvarchar128() + { + var ident = new string('б', 128); // 128 chars, 256 UTF-8 bytes + new SqlServerDialect().QuoteIdentifier(ident).ShouldBe("[" + ident + "]"); + } + [Fact] public void QuoteIdentifier_neutralisesAClassicInjectionPayload() {