From 63262fd972c609e965946adbc974051bbf59c5cb Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 17 Aug 2026 12:36:08 +0100 Subject: [PATCH 1/6] [EFCore] Fix MySQL/MariaDB literal sanitization Sanitize SQL for MySQL and MariaDB when dollar-quoted literals and backslash-escaped single quotes are present. --- .../CHANGELOG.md | 5 + .../EntityFrameworkDiagnosticListener.cs | 18 ++- .../DatabaseSemanticConventionHelper.cs | 5 +- src/Shared/SqlProcessor.cs | 138 +++++++++++++++--- .../SqlProcessorTests.cs | 87 +++++++++++ .../EntityFrameworkDiagnosticListenerTests.cs | 38 +++++ 6 files changed, 271 insertions(+), 20 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index 9310d88622..2634f29fea 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -11,6 +11,11 @@ `db.query.summary` length limit is reached. ([#4929](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4929)) +* Fixed query sanitization so that backslash-escaped quotes (`'a\'b'`) in + MySQL/MariaDB string literals and PostgreSQL dollar-quoted strings + (`$$...$$`) are correctly redacted. + ([#4985](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4985)) + ## 1.17.0-beta.1 Released 2026-Jul-17 diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 3117603cfe..17f564db76 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -189,12 +189,18 @@ public override void OnEventWritten(string name, object? payload) // able to sanitize arbitrary commands for other query dialects. var sanitizeQuery = IsSqlLikeProvider(providerName); + // MySQL/MariaDB treat a backslash as a string-literal escape + // character by default, which the sanitizer must honor to avoid + // leaking literal content into the query text. + var useBackslashEscapes = IsBackslashEscapeProvider(providerName); + DatabaseSemanticConventionHelper.ApplyConventionsForQueryText( activity, commandText, this.options.EmitOldAttributes, this.options.EmitNewAttributes, - sanitizeQuery); + sanitizeQuery, + useBackslashEscapes); break; case CommandType.TableDirect: @@ -369,6 +375,16 @@ DbSystemNames.Sqlite or }; } + internal static bool IsBackslashEscapeProvider(string? providerOrCommandName) + { + // MySQL and MariaDB (which use the MySQL providers) treat a backslash as a + // string-literal escape character unless the NO_BACKSLASH_ESCAPES SQL mode + // is enabled. The other supported engines follow the SQL standard where + // only a doubled quote ('') escapes a quote. + (_, var dbSystemName) = GetDbSystemNames(providerOrCommandName); + return dbSystemName == DbSystemNames.Mysql; + } + private void AddTag(Activity activity, (string Old, string New) attributes, string? value) => this.AddTag(activity, attributes, (value, value)); diff --git a/src/Shared/DatabaseSemanticConventionHelper.cs b/src/Shared/DatabaseSemanticConventionHelper.cs index 6c40334b9b..1c07747914 100644 --- a/src/Shared/DatabaseSemanticConventionHelper.cs +++ b/src/Shared/DatabaseSemanticConventionHelper.cs @@ -69,14 +69,15 @@ public static void ApplyConventionsForQueryText( string? commandText, bool emitOldAttributes, bool emitNewAttributes, - bool sanitizeQuery = true) + bool sanitizeQuery = true, + bool useBackslashEscapes = false) { var queryText = commandText ?? string.Empty; var querySummary = string.Empty; if (sanitizeQuery) { - var sqlStatementInfo = SqlProcessor.GetSanitizedSql(commandText); + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(commandText, useBackslashEscapes); queryText = sqlStatementInfo.SanitizedSql; querySummary = sqlStatementInfo.DbQuerySummary; diff --git a/src/Shared/SqlProcessor.cs b/src/Shared/SqlProcessor.cs index 7711132bd7..445de16e96 100644 --- a/src/Shared/SqlProcessor.cs +++ b/src/Shared/SqlProcessor.cs @@ -22,6 +22,8 @@ internal static class SqlProcessor private const char DashChar = '-'; private const char ForwardSlashChar = '/'; private const char SingleQuoteChar = '\''; + private const char BackslashChar = '\\'; + private const char DollarChar = '$'; private const char AsteriskChar = '*'; private const char UnderscoreChar = '_'; private const char DotChar = '.'; @@ -31,6 +33,7 @@ internal static class SqlProcessor private const char UnicodePrefixChar = 'N'; private static readonly ConcurrentDictionary Cache = new(); + private static readonly ConcurrentDictionary BackslashEscapeCache = new(); private static readonly char[] WhitespaceChars = [SpaceChar, TabChar, CarriageReturnChar, NewLineChar]; #if !NET @@ -93,6 +96,7 @@ internal static class SqlProcessor // We only increment on successful TryAdd. This may result in a slightly oversized cache // under high concurrency but this is acceptable for this scenario. private static int approxCacheCount; + private static int approxBackslashEscapeCacheCount; private enum SqlKeyword { @@ -143,35 +147,52 @@ private enum SqlKeyword View, } - public static SqlStatementInfo GetSanitizedSql(string? sql) + /// + /// Sanitizes a SQL statement by replacing its literal values with placeholders and computes the + /// corresponding db.query.summary. Results are cached per statement and dialect. + /// + /// The SQL statement to sanitize. + /// + /// if the source database treats a backslash as a string-literal escape + /// character (MySQL and MariaDB with the default NO_BACKSLASH_ESCAPES mode disabled); + /// otherwise . + /// + /// The sanitized SQL and query summary. + public static SqlStatementInfo GetSanitizedSql(string? sql, bool useBackslashEscapes = false) => + sql != null + ? useBackslashEscapes + ? GetSanitizedSql(sql, BackslashEscapeCache, ref approxBackslashEscapeCacheCount, useBackslashEscapes: true) + : GetSanitizedSql(sql, Cache, ref approxCacheCount, useBackslashEscapes: false) + : default; + + private static SqlStatementInfo GetSanitizedSql( + string sql, + ConcurrentDictionary cache, + ref int approxCount, + bool useBackslashEscapes) { - if (sql == null) - { - return default; - } - - if (Cache.TryGetValue(sql, out var sqlStatementInfo)) + if (cache.TryGetValue(sql, out var sqlStatementInfo)) { return sqlStatementInfo; } - sqlStatementInfo = SanitizeSql(sql); + sqlStatementInfo = SanitizeSql(sql, useBackslashEscapes); // Fast-path capacity check using our own approximate count to avoid ConcurrentDictionary.Count cost. - if (Volatile.Read(ref approxCacheCount) >= CacheCapacity) + if (Volatile.Read(ref approxCount) >= CacheCapacity) { return sqlStatementInfo; } // Attempt to add when under capacity. Increment our count only on successful add. - if (Cache.TryAdd(sql, sqlStatementInfo)) + if (cache.TryAdd(sql, sqlStatementInfo)) { - Interlocked.Increment(ref approxCacheCount); + Interlocked.Increment(ref approxCount); return sqlStatementInfo; } // If another thread added meanwhile, return the cached value if available. - return Cache.TryGetValue(sql, out var existing) ? existing : sqlStatementInfo; + return cache.TryGetValue(sql, out var existing) ? existing : sqlStatementInfo; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -239,7 +260,7 @@ private static bool HasTerminatingEscapedIdentifier(ReadOnlySpan sql, int return false; } - private static SqlStatementInfo SanitizeSql(string sql) + private static SqlStatementInfo SanitizeSql(string sql, bool useBackslashEscapes) { var sqlSpan = sql.AsSpan(); @@ -252,6 +273,7 @@ private static SqlStatementInfo SanitizeSql(string sql) var buffer = rentedBuffer.AsSpan(); ParseState state = default; + state.UseBackslashEscapes = useBackslashEscapes; // Precompute the summary buffer slice once and carry it via state to avoid repeated Span.Slice calls. state.SummaryBuffer = buffer.Slice(rentedBuffer.Length / 2); @@ -264,6 +286,7 @@ private static SqlStatementInfo SanitizeSql(string sql) } if (SanitizeStringLiteral(sqlSpan, buffer, ref state) || + SanitizeDollarQuotedLiteral(sqlSpan, buffer, ref state) || SanitizeHexLiteral(sqlSpan, buffer, ref state) || SanitizeNumericLiteral(sqlSpan, buffer, ref state)) { @@ -756,6 +779,7 @@ private static bool SanitizeStringLiteral(ReadOnlySpan sql, Span buf // If so, we want to skip the Unicode prefix when sanitizing. var isUnicode = state.ParsePosition >= 1 && sql[state.ParsePosition - 1] is UnicodePrefixChar; + var literalStart = state.ParsePosition; var searchPos = state.ParsePosition + 1; while (searchPos < sql.Length) { @@ -766,6 +790,21 @@ private static bool SanitizeStringLiteral(ReadOnlySpan sql, Span buf } searchPos += quoteIndex; + + // Skip a backslash-escaped quote (\'). MySQL/MariaDB (with the default + // NO_BACKSLASH_ESCAPES disabled) treat a backslash as a string escape + // character, so a quote preceded by an odd number of backslashes does + // not terminate the literal. Without this a value such as 'a\'secret' + // would be mis-parsed and the trailing "secret" copied into the sanitized + // SQL verbatim. This is gated on the dialect because '\' is not an escape + // in the other engines, where treating it as one would instead cause + // a '' -escaped literal to be incorrectly parsed. + if (state.UseBackslashEscapes && IsBackslashEscaped(sql, searchPos, literalStart)) + { + searchPos += 1; + continue; + } + if (searchPos + 1 < sql.Length && sql[searchPos + 1] == SingleQuoteChar) { // Skip escaped quote ('') @@ -793,6 +832,57 @@ private static bool SanitizeStringLiteral(ReadOnlySpan sql, Span buf return false; } + private static bool IsBackslashEscaped(ReadOnlySpan sql, int quoteIndex, int literalStart) + { + var backslashes = 0; + for (var i = quoteIndex - 1; i > literalStart && sql[i] == BackslashChar; i--) + { + backslashes++; + } + + return (backslashes & 1) == 1; + } + + private static bool SanitizeDollarQuotedLiteral(ReadOnlySpan sql, Span buffer, ref ParseState state) + { + // PostgreSQL dollar-quoted string: $tag$...$tag$ (the tag is optional, so $$...$$ is valid). + // The body between the delimiters is a literal with no escaping, so it must be redacted. + // This syntax is unambiguous across the SQL dialects handled here, so it is safe to apply. + var start = state.ParsePosition; + if (sql[start] != DollarChar) + { + return false; + } + + // Parse the opening delimiter: a dollar sign, an optional tag, then a closing dollar sign. + var tagEnd = start + 1; + while (tagEnd < sql.Length && IsDollarQuoteTagChar(sql[tagEnd])) + { + tagEnd++; + } + + if (tagEnd >= sql.Length || sql[tagEnd] != DollarChar) + { + return false; + } + + var delimiter = sql.Slice(start, tagEnd - start + 1); + var bodyStart = tagEnd + 1; + + var closeOffset = sql.Slice(bodyStart).IndexOf(delimiter); + if (closeOffset < 0) + { + return false; + } + + state.ParsePosition = bodyStart + closeOffset + delimiter.Length; + buffer[state.SanitizedPosition++] = SanitizationPlaceholder; + return true; + + static bool IsDollarQuoteTagChar(char c) + => char.IsAsciiLetterOrDigit(c) || c == UnderscoreChar; + } + private static bool SanitizeHexLiteral(ReadOnlySpan sql, Span buffer, ref ParseState state) { var i = state.ParsePosition; @@ -922,7 +1012,7 @@ private static bool TrySanitizeLiteralsForInClause(ReadOnlySpan sql, Span< // "IN ('a)b', 'secret')"), which would leave the parser positioned in the middle of // that literal. Every subsequent quote would then be mismatched and the remaining // values would be copied into the sanitized SQL verbatim instead of being replaced. - if (TryFindEndOfInClause(sql, parsePosition, out var closeParenIndex)) + if (TryFindEndOfInClause(sql, parsePosition, state.UseBackslashEscapes, out var closeParenIndex)) { state.ParsePosition = closeParenIndex; buffer[state.SanitizedPosition++] = SanitizationPlaceholder; @@ -943,7 +1033,7 @@ private static bool TrySanitizeLiteralsForInClause(ReadOnlySpan sql, Span< /// if the clause is not terminated, in which case the caller /// falls back to sanitizing each value individually. /// - private static bool TryFindEndOfInClause(ReadOnlySpan sql, int start, out int closeParenIndex) + private static bool TryFindEndOfInClause(ReadOnlySpan sql, int start, bool useBackslashEscapes, out int closeParenIndex) { var length = sql.Length; var i = start; @@ -970,7 +1060,7 @@ private static bool TryFindEndOfInClause(ReadOnlySpan sql, int start, out return true; case SingleQuoteChar: - i = SkipStringLiteral(sql, i); + i = SkipStringLiteral(sql, i, useBackslashEscapes); break; case DashChar: @@ -993,7 +1083,7 @@ private static bool TryFindEndOfInClause(ReadOnlySpan sql, int start, out // Returns the index after the closing quote, or the end of the input if the // literal is not terminated. - static int SkipStringLiteral(ReadOnlySpan sql, int quotePosition) + static int SkipStringLiteral(ReadOnlySpan sql, int quotePosition, bool useBackslashEscapes) { var length = sql.Length; var i = quotePosition + 1; @@ -1008,6 +1098,14 @@ static int SkipStringLiteral(ReadOnlySpan sql, int quotePosition) i += quoteIndex; + // A backslash-escaped quote (\') does not terminate the literal in dialects that use + // backslash escapes. See the note in SanitizeStringLiteral. + if (useBackslashEscapes && IsBackslashEscaped(sql, i, quotePosition)) + { + i += 1; + continue; + } + // A doubled quote ('') is an escaped quote within the literal. if (i + 1 < length && sql[i + 1] == SingleQuoteChar) { @@ -1100,6 +1198,12 @@ private ref struct ParseState public bool SanitizeNextNonKeywordToken; // 1 byte + /// + /// Whether the source dialect treats a backslash as a string-literal escape character + /// (MySQL/MariaDB). Controls whether \' is recognized as an escaped quote. + /// + public bool UseBackslashEscapes; // 1 byte + /// /// Used to track if we are in an escaped identifier (e.g., "[table]"). /// diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs index 962ac962be..00a6c1fe4f 100644 --- a/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs +++ b/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs @@ -108,6 +108,93 @@ public void GetSanitizedSql_UnterminatedInClauseLiteralContainingCloseParen_Sani Assert.Equal("SELECT * FROM Users WHERE Name IN (?, ?", sqlStatementInfo.SanitizedSql); } + [Fact] + public void GetSanitizedSql_BackslashEscapedQuoteWithBackslashDialect_SanitizesLiteral() + { + var sql = "SELECT * FROM Users WHERE Password = 'a\\'secret-name'"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: true); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-name", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Password = ?", sqlStatementInfo.SanitizedSql); + } + + [Fact] + public void GetSanitizedSql_BackslashEscapedQuoteInInClauseWithBackslashDialect_SanitizesLiterals() + { + var sql = "SELECT * FROM Users WHERE Name IN ('a\\'secret-name', 'b')"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: true); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-name", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Name IN (?)", sqlStatementInfo.SanitizedSql); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void GetSanitizedSql_DoubledQuoteEscape_SanitizesLiteralInEitherDialect(bool useBackslashEscapes) + { + var sql = "SELECT * FROM Users WHERE Password = 'a''secret-name'"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes); + + Assert.DoesNotContain("secret-name", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Password = ?", sqlStatementInfo.SanitizedSql); + } + + [Fact] + public void GetSanitizedSql_BackslashBeforeDoubledQuoteWithoutBackslashDialect_DoesNotLeak() + { + var sql = "SELECT * FROM Users WHERE Password = 'a\\''secret-name'"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: false); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-name", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Password = ?", sqlStatementInfo.SanitizedSql); + } + + [Theory] + [InlineData("SELECT * FROM t WHERE c = $$secret-name$$")] + [InlineData("SELECT * FROM t WHERE c = $tag$se'cret-'name$tag$")] // Body may contain quotes/dollars. + public void GetSanitizedSql_DollarQuotedString_SanitizesLiteral(string sql) + { + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM t WHERE c = ?", sqlStatementInfo.SanitizedSql); + } + + [Theory] + [InlineData("SELECT $IDENTITY FROM t", "SELECT $IDENTITY FROM t")] // SQL Server pseudo-column. + [InlineData("SELECT a WHERE b = $1", "SELECT a WHERE b = $?")] // PostgreSQL positional parameter. + public void GetSanitizedSql_LoneDollarSign_IsNotTreatedAsDollarQuote(string sql, string expected) + { + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.Equal(expected, sqlStatementInfo.SanitizedSql); + } + + [Fact] + public void GetSanitizedSql_DoubleQuotedString_IsPreservedAsIdentifier() + { + var sql = "SELECT * FROM t WHERE c = \"identifier_or_mysql_string\""; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql); + + Assert.Contains("identifier_or_mysql_string", sqlStatementInfo.SanitizedSql); + } + [Fact] public void GetSanitizedSql_UnterminatedInClauseStringLiteral_SanitizesLiteral() { diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs index fd130b5891..0aeed027d4 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs @@ -233,6 +233,35 @@ public static TheoryData IsSqlLikeProviderTestCases() return testCases; } + public static TheoryData IsBackslashEscapeProviderTestCases() + { + var values = DbSystemTestCases().ToDictionary((k) => (string)k[0], (v) => false); + + string[] backslashEscapeProviders = + [ + "Devart.Data.MySql.Entity.EFCore", + "Devart.Data.MySql.MySqlCommand", + "MySql.Data.EntityFrameworkCore", + "MySql.Data.MySqlClient.MySqlCommand", + "MySql.EntityFrameworkCore", + "Pomelo.EntityFrameworkCore.MySql", + ]; + + foreach (var name in backslashEscapeProviders) + { + values[name] = true; + } + + var testCases = new TheoryData(); + + foreach ((var name, var expected) in values) + { + testCases.Add(name, expected); + } + + return testCases; + } + [Theory] [MemberData(nameof(DbSystemTestCases))] public void ShouldReturnCorrectAttributeValuesProviderOrCommandName(string name, string expectedDbSystem, string expectedDbSystemName) @@ -252,6 +281,15 @@ public void ShouldReturnCorrectValueForSqlLikeProviderOrCommandName(string name, Assert.Equal(expected, actual); } + [Theory] + [MemberData(nameof(IsBackslashEscapeProviderTestCases))] + public void ShouldReturnCorrectValueForBackslashEscapeProviderOrCommandName(string name, bool expected) + { + var actual = EntityFrameworkDiagnosticListener.IsBackslashEscapeProvider(name); + + Assert.Equal(expected, actual); + } + [Fact] public void EntityFrameworkContextEventsInstrumentedTest() { From 5f77484bc8f0771b4ba322b967338f34ec5434b5 Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Mon, 17 Aug 2026 12:40:57 +0100 Subject: [PATCH 2/6] [EFCore] Fix typo Fix typo in comment. --- src/Shared/SqlProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/SqlProcessor.cs b/src/Shared/SqlProcessor.cs index 445de16e96..298b18cabf 100644 --- a/src/Shared/SqlProcessor.cs +++ b/src/Shared/SqlProcessor.cs @@ -795,7 +795,7 @@ private static bool SanitizeStringLiteral(ReadOnlySpan sql, Span buf // NO_BACKSLASH_ESCAPES disabled) treat a backslash as a string escape // character, so a quote preceded by an odd number of backslashes does // not terminate the literal. Without this a value such as 'a\'secret' - // would be mis-parsed and the trailing "secret" copied into the sanitized + // would be incorrectly parsed and the trailing "secret" copied into the sanitized // SQL verbatim. This is gated on the dialect because '\' is not an escape // in the other engines, where treating it as one would instead cause // a '' -escaped literal to be incorrectly parsed. From d1f4ff8263d6b806e5030378039089575002d22a Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 17 Aug 2026 13:17:41 +0100 Subject: [PATCH 3/6] [EFCore] Address feedback Avoid false positive for dollar-quoted values. --- src/Shared/SqlProcessor.cs | 14 +++++++++++++- .../SqlProcessorTests.cs | 12 ++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Shared/SqlProcessor.cs b/src/Shared/SqlProcessor.cs index 298b18cabf..3e4af69f70 100644 --- a/src/Shared/SqlProcessor.cs +++ b/src/Shared/SqlProcessor.cs @@ -856,9 +856,18 @@ private static bool SanitizeDollarQuotedLiteral(ReadOnlySpan sql, Span= sql.Length || sql[tagEnd] != DollarChar) @@ -879,6 +888,9 @@ private static bool SanitizeDollarQuotedLiteral(ReadOnlySpan sql, Span char.IsAsciiLetter(c) || c == UnderscoreChar; + static bool IsDollarQuoteTagChar(char c) => char.IsAsciiLetterOrDigit(c) || c == UnderscoreChar; } diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs index 00a6c1fe4f..c2fe9e2229 100644 --- a/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs +++ b/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs @@ -185,6 +185,18 @@ public void GetSanitizedSql_LoneDollarSign_IsNotTreatedAsDollarQuote(string sql, Assert.Equal(expected, sqlStatementInfo.SanitizedSql); } + [Fact] + public void GetSanitizedSql_DollarQuoteTagStartingWithDigit_IsNotTreatedAsDollarQuote() + { + var sql = "SELECT a WHERE b = $1$not-a-secret$1$"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.Contains("not-a-secret", sqlStatementInfo.SanitizedSql); + } + [Fact] public void GetSanitizedSql_DoubleQuotedString_IsPreservedAsIdentifier() { From c2c37e2581255c083781e4b03455fb9a8f3acfc7 Mon Sep 17 00:00:00 2001 From: martincostello Date: Wed, 19 Aug 2026 10:52:36 +0100 Subject: [PATCH 4/6] [EFCore] Address feedback - Fix CHANGELOG PR number. - Sanitize unterminated literals. - Make ternary slightly more readable. - Add comment about `NO_BACKSLASH_ESCAPES`. --- .../CHANGELOG.md | 2 +- .../EntityFrameworkDiagnosticListener.cs | 5 +++++ src/Shared/SqlProcessor.cs | 12 +++++++----- .../SqlProcessorTests.cs | 13 +++++++++++++ 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index 2634f29fea..fca30d65f8 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -14,7 +14,7 @@ * Fixed query sanitization so that backslash-escaped quotes (`'a\'b'`) in MySQL/MariaDB string literals and PostgreSQL dollar-quoted strings (`$$...$$`) are correctly redacted. - ([#4985](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4985)) + ([#4986](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4986)) ## 1.17.0-beta.1 diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 17f564db76..7626a55149 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -381,6 +381,11 @@ internal static bool IsBackslashEscapeProvider(string? providerOrCommandName) // string-literal escape character unless the NO_BACKSLASH_ESCAPES SQL mode // is enabled. The other supported engines follow the SQL standard where // only a doubled quote ('') escapes a quote. + // + // This assumes the default (NO_BACKSLASH_ESCAPES disabled) behaviour: there is + // no way to detect the session SQL mode from the provider/command name alone, + // so if an application has enabled NO_BACKSLASH_ESCAPES the sanitizer will still + // treat '\' as an escape character for that connection. (_, var dbSystemName) = GetDbSystemNames(providerOrCommandName); return dbSystemName == DbSystemNames.Mysql; } diff --git a/src/Shared/SqlProcessor.cs b/src/Shared/SqlProcessor.cs index 3e4af69f70..471638d7ed 100644 --- a/src/Shared/SqlProcessor.cs +++ b/src/Shared/SqlProcessor.cs @@ -159,11 +159,11 @@ private enum SqlKeyword /// /// The sanitized SQL and query summary. public static SqlStatementInfo GetSanitizedSql(string? sql, bool useBackslashEscapes = false) => - sql != null - ? useBackslashEscapes + sql == null + ? default + : useBackslashEscapes ? GetSanitizedSql(sql, BackslashEscapeCache, ref approxBackslashEscapeCacheCount, useBackslashEscapes: true) - : GetSanitizedSql(sql, Cache, ref approxCacheCount, useBackslashEscapes: false) - : default; + : GetSanitizedSql(sql, Cache, ref approxCacheCount, useBackslashEscapes: false); private static SqlStatementInfo GetSanitizedSql( string sql, @@ -881,7 +881,9 @@ private static bool SanitizeDollarQuotedLiteral(ReadOnlySpan sql, Span Date: Sat, 22 Aug 2026 14:04:38 +0100 Subject: [PATCH 5/6] [EFCore] Update CHANGELOG Fix entry location. --- .../CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index c5137bcc0f..7618e73091 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +* Fixed query sanitization so that backslash-escaped quotes (`'a\'b'`) in + MySQL/MariaDB string literals and PostgreSQL dollar-quoted strings + (`$$...$$`) are correctly redacted. + ([#4986](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4986)) + ## 1.18.0-beta.1 Released 2026-Aug-21 @@ -21,11 +26,6 @@ Released 2026-Aug-21 * Updated OpenTelemetry core component version(s) to `1.18.0`. ([#5022](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/5022)) -* Fixed query sanitization so that backslash-escaped quotes (`'a\'b'`) in - MySQL/MariaDB string literals and PostgreSQL dollar-quoted strings - (`$$...$$`) are correctly redacted. - ([#4986](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4986)) - ## 1.17.0-beta.1 Released 2026-Jul-17 From 8b0ded70c6185f8b8380f9c7049e957bb082a969 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 25 Aug 2026 16:37:05 +0100 Subject: [PATCH 6/6] [EFCore] Sanitize double-quoted literals Fix MySQL/MariaDB double-quoted literal (valid syntax when `ANSI_QUOTES` is disabled) not being sanitized. --- .../CHANGELOG.md | 5 + src/Shared/SqlProcessor.cs | 114 ++++++++++-------- .../SqlProcessorTests.cs | 52 ++++++++ 3 files changed, 120 insertions(+), 51 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index 7618e73091..87720ff8c3 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -7,6 +7,11 @@ (`$$...$$`) are correctly redacted. ([#4986](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4986)) +* Fixed query sanitization so that MySQL/MariaDB double-quoted string + literals (`"..."`, valid when `ANSI_QUOTES` is disabled) are correctly + redacted. + ([#4986](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4986)) + ## 1.18.0-beta.1 Released 2026-Aug-21 diff --git a/src/Shared/SqlProcessor.cs b/src/Shared/SqlProcessor.cs index 471638d7ed..bb3beddf03 100644 --- a/src/Shared/SqlProcessor.cs +++ b/src/Shared/SqlProcessor.cs @@ -22,6 +22,7 @@ internal static class SqlProcessor private const char DashChar = '-'; private const char ForwardSlashChar = '/'; private const char SingleQuoteChar = '\''; + private const char DoubleQuoteChar = '"'; private const char BackslashChar = '\\'; private const char DollarChar = '$'; private const char AsteriskChar = '*'; @@ -153,9 +154,10 @@ private enum SqlKeyword /// /// The SQL statement to sanitize. /// - /// if the source database treats a backslash as a string-literal escape - /// character (MySQL and MariaDB with the default NO_BACKSLASH_ESCAPES mode disabled); - /// otherwise . + /// if the source database is MySQL or MariaDB with their default SQL modes + /// (NO_BACKSLASH_ESCAPES and ANSI_QUOTES disabled), in which case a backslash is + /// treated as a string-literal escape character and a double-quoted ("...") value is treated + /// as a string literal rather than a quoted identifier; otherwise . /// /// The sanitized SQL and query summary. public static SqlStatementInfo GetSanitizedSql(string? sql, bool useBackslashEscapes = false) => @@ -770,66 +772,76 @@ private static bool SanitizeStringLiteral(ReadOnlySpan sql, Span buf var currentChar = sql[state.ParsePosition]; if (currentChar == SingleQuoteChar) { - if (TrySanitizeLiteralsForInClause(sql, buffer, ref state, state.ParsePosition)) - { - return true; - } + return TrySanitizeLiteralsForInClause(sql, buffer, ref state, state.ParsePosition) || + SanitizeQuotedLiteral(sql, buffer, ref state, SingleQuoteChar, allowUnicodePrefix: true); + } - // Is the string literal of the form `N'foo'` (i.e. a Unicode literal)? - // If so, we want to skip the Unicode prefix when sanitizing. - var isUnicode = state.ParsePosition >= 1 && sql[state.ParsePosition - 1] is UnicodePrefixChar; + // MySQL/MariaDB (the same dialects for which useBackslashEscapes is set) also treat a + // double-quoted value as a string literal unless the ANSI_QUOTES sql_mode is enabled. + // The other dialects supported here use '"' exclusively to delimit a quoted identifier + // (e.g. "columnName"), so this must remain gated on the dialect flag: otherwise a quoted + // identifier in those dialects would be misidentified as a literal and redacted, and + // conversely a MySQL/MariaDB double-quoted literal would leak into the sanitized SQL + // verbatim (it is never recognized as an identifier or literal by any other check here). + return currentChar == DoubleQuoteChar && + state.UseBackslashEscapes && + SanitizeQuotedLiteral(sql, buffer, ref state, DoubleQuoteChar, allowUnicodePrefix: false); + } - var literalStart = state.ParsePosition; - var searchPos = state.ParsePosition + 1; - while (searchPos < sql.Length) - { - var quoteIndex = sql.Slice(searchPos).IndexOf(SingleQuoteChar); - if (quoteIndex < 0) - { - break; - } + private static bool SanitizeQuotedLiteral(ReadOnlySpan sql, Span buffer, ref ParseState state, char delimiter, bool allowUnicodePrefix) + { + // Is the string literal of the form `N'foo'` (i.e. a Unicode literal)? + // If so, we want to skip the Unicode prefix when sanitizing. + var isUnicode = allowUnicodePrefix && state.ParsePosition >= 1 && sql[state.ParsePosition - 1] is UnicodePrefixChar; - searchPos += quoteIndex; - - // Skip a backslash-escaped quote (\'). MySQL/MariaDB (with the default - // NO_BACKSLASH_ESCAPES disabled) treat a backslash as a string escape - // character, so a quote preceded by an odd number of backslashes does - // not terminate the literal. Without this a value such as 'a\'secret' - // would be incorrectly parsed and the trailing "secret" copied into the sanitized - // SQL verbatim. This is gated on the dialect because '\' is not an escape - // in the other engines, where treating it as one would instead cause - // a '' -escaped literal to be incorrectly parsed. - if (state.UseBackslashEscapes && IsBackslashEscaped(sql, searchPos, literalStart)) - { - searchPos += 1; - continue; - } + var literalStart = state.ParsePosition; + var searchPos = state.ParsePosition + 1; + while (searchPos < sql.Length) + { + var quoteIndex = sql.Slice(searchPos).IndexOf(delimiter); + if (quoteIndex < 0) + { + break; + } - if (searchPos + 1 < sql.Length && sql[searchPos + 1] == SingleQuoteChar) - { - // Skip escaped quote ('') - searchPos += 2; - continue; - } + searchPos += quoteIndex; + + // Skip a backslash-escaped delimiter (\' or \"). MySQL/MariaDB (with the default + // NO_BACKSLASH_ESCAPES disabled) treat a backslash as a string escape character in + // both single- and double-quoted literals, so a delimiter preceded by an odd number + // of backslashes does not terminate the literal. Without this a value such as + // 'a\'secret' would be incorrectly parsed and the trailing "secret" copied into the + // sanitized SQL verbatim. This is gated on the dialect because '\' is not an escape + // in the other engines, where treating it as one would instead cause a doubled-quote + // -escaped literal to be incorrectly parsed. + if (state.UseBackslashEscapes && IsBackslashEscaped(sql, searchPos, literalStart)) + { + searchPos += 1; + continue; + } - // Found terminating quote - if (isUnicode) - { - // Skip the Unicode prefix by overwriting the previous position instead - state.SanitizedPosition--; - } + if (searchPos + 1 < sql.Length && sql[searchPos + 1] == delimiter) + { + // Skip escaped delimiter ('' or "") + searchPos += 2; + continue; + } - state.ParsePosition = searchPos + 1; - buffer[state.SanitizedPosition++] = SanitizationPlaceholder; - return true; + // Found terminating delimiter + if (isUnicode) + { + // Skip the Unicode prefix by overwriting the previous position instead + state.SanitizedPosition--; } - state.ParsePosition = sql.Length; + state.ParsePosition = searchPos + 1; buffer[state.SanitizedPosition++] = SanitizationPlaceholder; return true; } - return false; + state.ParsePosition = sql.Length; + buffer[state.SanitizedPosition++] = SanitizationPlaceholder; + return true; } private static bool IsBackslashEscaped(ReadOnlySpan sql, int quoteIndex, int literalStart) diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs index e651c0247c..ec52a9f011 100644 --- a/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs +++ b/test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs @@ -220,6 +220,58 @@ public void GetSanitizedSql_DoubleQuotedString_IsPreservedAsIdentifier() Assert.Contains("identifier_or_mysql_string", sqlStatementInfo.SanitizedSql); } + [Fact] + public void GetSanitizedSql_DoubleQuotedStringWithBackslashDialect_SanitizesLiteral() + { + var sql = "SELECT * FROM Users WHERE Name = \"secret-value\" AND Id = 1"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: true); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-value", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Name = ? AND Id = ?", sqlStatementInfo.SanitizedSql); + } + + [Fact] + public void GetSanitizedSql_DoubledDoubleQuoteEscapeWithBackslashDialect_SanitizesLiteral() + { + var sql = "SELECT * FROM Users WHERE Name = \"a\"\"secret-value\""; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: true); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-value", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Name = ?", sqlStatementInfo.SanitizedSql); + } + + [Fact] + public void GetSanitizedSql_BackslashEscapedDoubleQuoteWithBackslashDialect_SanitizesLiteral() + { + var sql = "SELECT * FROM Users WHERE Name = \"a\\\"secret-value\""; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: true); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-value", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Name = ?", sqlStatementInfo.SanitizedSql); + } + + [Fact] + public void GetSanitizedSql_UnterminatedDoubleQuotedStringWithBackslashDialect_SanitizesLiteral() + { + var sql = "SELECT * FROM Users WHERE Name = \"secret-value"; + + var sqlStatementInfo = SqlProcessor.GetSanitizedSql(sql, useBackslashEscapes: true); + + this.output.WriteLine($"Sanitized: {sqlStatementInfo.SanitizedSql}"); + + Assert.DoesNotContain("secret-value", sqlStatementInfo.SanitizedSql); + Assert.Equal("SELECT * FROM Users WHERE Name = ?", sqlStatementInfo.SanitizedSql); + } + [Fact] public void GetSanitizedSql_UnterminatedInClauseStringLiteral_SanitizesLiteral() {