diff --git a/Darling/Darling.Tests/CallerPlanXmlTests.cs b/Darling/Darling.Tests/CallerPlanXmlTests.cs new file mode 100644 index 0000000000..10e789ee09 --- /dev/null +++ b/Darling/Darling.Tests/CallerPlanXmlTests.cs @@ -0,0 +1,289 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Text.Json; +using System.Xml.Linq; +using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.PlanAnalysis; +using Xunit; + +namespace Darling.Tests; + +/// +/// Plan XML that a caller supplies (a file opened or dropped, pasted text, or the plan_xml argument of +/// analyze_plan_xml) is bounded before it reaches the parser. Measured on the shipped parser, one element +/// with N attributes costs about N squared: 100,000 attributes take 0.27 s and 1,000,000 take 27 s (11.9M +/// characters, inside the 16M cap), while the same attributes over many elements are linear. The count walks +/// the attributes of each tag in one pass, so a refused input costs milliseconds. +/// +[Collection("timing")] +public sealed class CallerPlanXmlTests +{ + private const string Ns = "http://schemas.microsoft.com/sqlserver/2004/07/showplan"; + + private static string Wrap(string inner) => + $"{inner}"; + + /// One element carrying attributes with unique names. + private static string ElementWithAttributes(int count, string value = "1") + { + var sb = new StringBuilder(count * 12 + 64); + sb.Append(""); + return Wrap(sb.ToString()); + } + + private static string FixturePath(string fileName) => + Path.Combine(AppContext.BaseDirectory, "Fixtures", "OriginPlans", fileName); + + private static string[] FixtureFiles() => + Directory.GetFiles(Path.Combine(AppContext.BaseDirectory, "Fixtures", "OriginPlans"), "*.sqlplan"); + + [Fact] + public void Limit_LeavesTenTimesTheWidestElementInTheRealFixturePlans() + { + var widest = 0; + foreach (var file in FixtureFiles()) + { + var doc = XDocument.Parse(File.ReadAllText(file)); + widest = Math.Max(widest, doc.Descendants().Max(e => e.Attributes().Count())); + } + + Assert.InRange(widest, 1, int.MaxValue); + Assert.True(CallerPlanXml.MaxAttributesPerElement >= 10 * widest, + $"the limit {CallerPlanXml.MaxAttributesPerElement} must stay at least 10x the widest real element ({widest} attributes)."); + } + + [Fact] + public void SizeCap_IsStillSixteenMillionCharacters() + { + // Size alone is linear (1,000,000 one-attribute elements parse in 0.8 s), and people open very large + // real plans, so the caller gate adds no size limit of its own. + Assert.Equal(16 * 1024 * 1024, ShowPlanParser.MaxParseCharacters); + } + + [Fact] + public void ElementAtTheLimit_IsNotRefused_AndStillParses() + { + var xml = ElementWithAttributes(CallerPlanXml.MaxAttributesPerElement); + + Assert.Null(CallerPlanXml.RefusalReason(xml)); + Assert.Null(ShowPlanParser.Parse(xml).ParseError); + } + + [Fact] + public void ElementJustPastTheLimit_IsRefused_NamingTheLimit() + { + var xml = ElementWithAttributes(CallerPlanXml.MaxAttributesPerElement + 1); + + var reason = CallerPlanXml.RefusalReason(xml); + + Assert.NotNull(reason); + Assert.Contains(CallerPlanXml.MaxAttributesPerElement.ToString("N0"), reason); + Assert.Contains("attributes", reason); + } + + [Fact] + public void AMillionAttributeElement_IsRefusedInWellUnderASecond_NotParsedForHalfAMinute() + { + var xml = ElementWithAttributes(1_000_000); + Assert.InRange(xml.Length, 1, ShowPlanParser.MaxParseCharacters); + + var sw = Stopwatch.StartNew(); + var reason = CallerPlanXml.RefusalReason(xml); + sw.Stop(); + + Assert.NotNull(reason); + Assert.True(sw.ElapsedMilliseconds < 1000, $"refusal took {sw.ElapsedMilliseconds} ms; the parse it replaces takes about 27 s."); + } + + [Fact] + public void AGreaterThanSignInEachValue_DoesNotHideTheAttributes() + { + // A plain '<'..'>' count resets at the '>' inside the first value and sees one attribute; this counter + // treats the value as quoted text, so the tag still counts every attribute. + var sb = new StringBuilder("\" "); + sb.Append("/>"); + + Assert.NotNull(CallerPlanXml.RefusalReason(Wrap(sb.ToString()))); + } + + [Fact] + public void SingleQuotedValues_AreTrackedToo() + { + var sb = new StringBuilder(""); + + Assert.NotNull(CallerPlanXml.RefusalReason(Wrap(sb.ToString()))); + } + + [Fact] + public void EqualsSignsInsideOneValue_AreNotAttributes() + { + // A predicate such as [a]=(1) OR [a]=(2) ... holds an '=' per term; a real plan with a long IN list + // must not be counted as thousands of attributes. + var predicate = string.Join(" OR ", Enumerable.Range(0, CallerPlanXml.MaxAttributesPerElement * 5).Select(i => $"[a]=({i})")); + var xml = Wrap($""); + + Assert.Null(CallerPlanXml.RefusalReason(xml)); + Assert.Null(ShowPlanParser.Parse(xml).ParseError); + } + + [Fact] + public void AQuoteInAComment_DoesNotCarryIntoTheNextTag() + { + // The '<' of the next tag clears the quote, so an odd quote in a comment cannot hide a flood after it. + var flood = ElementWithAttributes(CallerPlanXml.MaxAttributesPerElement + 1); + var xml = "" + flood; + + Assert.NotNull(CallerPlanXml.RefusalReason(xml)); + } + + [Fact] + public void ManyLargeButNarrowElements_AreNotRefused_AndParse() + { + // 100,000 elements with 20 attributes each (about 7M characters): big, linear to parse, and nothing like a flood. + var attrs = string.Join(" ", Enumerable.Range(0, 20).Select(i => $"a{i}=\"1\"")); + var sb = new StringBuilder(""); + for (var i = 0; i < 100_000; i++) sb.Append(""); + sb.Append(""); + var xml = Wrap(sb.ToString()); + + var sw = Stopwatch.StartNew(); + var reason = CallerPlanXml.RefusalReason(xml); + sw.Stop(); + + Assert.Null(reason); + Assert.True(sw.ElapsedMilliseconds < 1000, $"counting {xml.Length:N0} characters took {sw.ElapsedMilliseconds} ms."); + Assert.Null(ShowPlanParser.Parse(xml).ParseError); + } + + [Fact] + public void ADoctype_IsRefused() + { + var xml = "]>" + Wrap(""); + + var reason = CallerPlanXml.RefusalReason(xml); + + Assert.NotNull(reason); + Assert.Contains("DOCTYPE", reason); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void NothingToCount_IsNotRefused(string? xml) => Assert.Null(CallerPlanXml.RefusalReason(xml)); + + [Fact] + public void EveryRealFixturePlan_IsNotRefused_AndStillParsesAndAnalyzes() + { + var files = FixtureFiles(); + Assert.NotEmpty(files); + + foreach (var file in files) + { + var xml = File.ReadAllText(file); + Assert.True(CallerPlanXml.RefusalReason(xml) == null, $"{Path.GetFileName(file)} was refused."); + Assert.Null(ShowPlanParser.Parse(xml).ParseError); + + using var doc = JsonDocument.Parse(DarlingMcpPlanTools.AnalyzePlanXml(xml)); + Assert.Equal("xml", doc.RootElement.GetProperty("source").GetString()); + Assert.True(doc.RootElement.GetProperty("statement_count").GetInt32() >= 1, $"{Path.GetFileName(file)} analyzed to no statements."); + } + } + + [Fact] + public void AnalyzePlanXml_OverTheLimit_IsRefusedBeforeParsing() + { + var xml = ElementWithAttributes(1_000_000); + var reason = CallerPlanXml.RefusalReason(xml); + Assert.NotNull(reason); + + var sw = Stopwatch.StartNew(); + var result = DarlingMcpPlanTools.AnalyzePlanXml(xml); + sw.Stop(); + + Assert.Equal(McpHelpers.Refusal("plan_xml", reason!), result); + Assert.Contains(CallerPlanXml.MaxAttributesPerElement.ToString("N0"), McpHelpers.ErrorMessageOf(result)); + Assert.True(sw.ElapsedMilliseconds < 1000, $"analyze_plan_xml took {sw.ElapsedMilliseconds} ms to refuse; parsing it takes about 27 s."); + } + + [Fact] + public void AnalyzePlanXml_AtTheLimit_IsNotRefused() + { + var result = DarlingMcpPlanTools.AnalyzePlanXml(ElementWithAttributes(CallerPlanXml.MaxAttributesPerElement)); + + using var doc = JsonDocument.Parse(result); + Assert.Equal("xml", doc.RootElement.GetProperty("source").GetString()); + } + + [Fact] + public void AnalyzePlanXml_Doctype_IsRefused() + { + var xml = "]>" + Wrap(""); + + var result = DarlingMcpPlanTools.AnalyzePlanXml(xml); + + Assert.Equal(McpHelpers.Refusal("plan_xml", CallerPlanXml.RefusalReason(xml)!), result); + } + + // ---- Where the gate is wired: source reads, the way this suite pins the viewer's WPF handlers ---- + + private static string UiFile(string fileName, [CallerFilePath] string thisFile = "") => + File.ReadAllText(Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", "..", "PerformanceMonitor.Ui", fileName))); + + private static string AnalysisFile(string fileName, [CallerFilePath] string thisFile = "") => + File.ReadAllText(Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", "..", "PerformanceMonitor.PlanAnalysis", fileName))); + + private static int Count(string source, string token) => + (source.Length - source.Replace(token, "", StringComparison.Ordinal).Length) / token.Length; + + [Fact] + public void EveryViewerReadOfCallerText_IsGated() + { + // Open, drop (two paths), the paste button and Ctrl+V in the controller: three file reads and two clipboard reads. + var controller = UiFile("StandalonePlanViewerController.cs"); + var controllerReads = Count(controller, "File.ReadAllText(") + Count(controller, "ClipboardText.TryReadAsync("); + Assert.Equal(5, controllerReads); + Assert.Equal(controllerReads, Count(controller, "CallerPlanXmlPrompt.Refuse(")); + + // Ctrl+V inside the plan viewer control. + var interaction = UiFile("PlanViewerControl.Interaction.cs"); + Assert.Equal(1, Count(interaction, "ClipboardText.TryReadAsync(")); + Assert.Equal(1, Count(interaction, "CallerPlanXmlPrompt.Refuse(")); + } + + [Fact] + public void StoredPlanPaths_AreNotGated() + { + // The collectors' plans reach the parser and the viewer through these, and they keep their own limits. + Assert.DoesNotContain("CallerPlanXml", AnalysisFile("ShowPlanParser.cs")); + Assert.DoesNotContain("CallerPlanXml", AnalysisFile("McpPlanAnalysisFormatter.cs")); + Assert.DoesNotContain("CallerPlanXml", AnalysisFile("PlanAdvisoryAggregator.cs")); + Assert.DoesNotContain("CallerPlanXml", AnalysisFile("PlanAnalysisPipeline.cs")); + Assert.DoesNotContain("CallerPlanXml", UiFile("PlanViewerControl.xaml.cs")); + + // LoadPlanIntoSubTab is also how the Darling viewer opens a stored plan in a tab. + var controller = UiFile("StandalonePlanViewerController.cs"); + var start = controller.IndexOf("public async Task LoadPlanIntoSubTab(", StringComparison.Ordinal); + var end = controller.IndexOf("private string GetUniqueSubTabLabel(", StringComparison.Ordinal); + Assert.InRange(start, 0, controller.Length); + Assert.True(end > start, "the read of LoadPlanIntoSubTab is broken."); + Assert.DoesNotContain("CallerPlanXml", controller.Substring(start, end - start)); + } +} diff --git a/Darling/Darling.Tests/DarlingSmtpWarningTests.cs b/Darling/Darling.Tests/DarlingSmtpWarningTests.cs new file mode 100644 index 0000000000..b7a2c81a26 --- /dev/null +++ b/Darling/Darling.Tests/DarlingSmtpWarningTests.cs @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.IO; +using System.Runtime.CompilerServices; +using Microsoft.Extensions.Logging; +using PerformanceMonitor.Darling.Service; +using Xunit; + +namespace Darling.Tests; + +/// +/// An SMTP setup with SSL off sends its login without encryption. The settings windows warn when someone saves it; +/// the service logs the same warning at start and on every config reload, for the setups no window touched: one +/// declared in darling.json, and one saved before the windows warned. +/// +public sealed class DarlingSmtpWarningTests +{ + private static SmtpConfig Smtp(string host = "mail.example.test", bool useSsl = false, string? username = "relay", + string? password = "literal", string? encrypted = null) => + new() { Host = host, Port = 25, UseSsl = useSsl, Username = username, Password = password, EncryptedPassword = encrypted, From = "a@example.test" }; + + [Fact] + public void ASetupWithSslOffAndALogin_LogsOneWarning_NamingTheHost() + { + var logger = new CapturingTestLogger(); + + DarlingSmtpWarning.Log(Smtp(), logger); + + Assert.Equal(1, logger.CountAtLevel(LogLevel.Warning)); + Assert.Contains("mail.example.test:25", logger.Joined); + Assert.Contains("without encryption", logger.Joined); + } + + [Fact] + public void ASealedPassword_CountsAsAPassword_AndItsValueIsNeverLogged() + { + var logger = new CapturingTestLogger(); + + DarlingSmtpWarning.Log(Smtp(password: null, encrypted: "c2VhbGVk"), logger); + + Assert.Equal(1, logger.CountAtLevel(LogLevel.Warning)); + Assert.DoesNotContain("c2VhbGVk", logger.Joined); + } + + [Theory] + [InlineData("mail.example.test", true, "relay", "literal")] // SSL on + [InlineData("mail.example.test", false, "", "literal")] // no username + [InlineData("mail.example.test", false, null, "literal")] + [InlineData("mail.example.test", false, "relay", null)] // no password + [InlineData("", false, "relay", "literal")] // SMTP not set up + public void EveryOtherSetup_LogsNothing(string host, bool useSsl, string? username, string? password) + { + var logger = new CapturingTestLogger(); + + DarlingSmtpWarning.Log(Smtp(host, useSsl, username, password), logger); + + Assert.Equal(0, logger.CountAtLevel(LogLevel.Warning)); + } + + [Fact] + public void TheServiceLogsItAtStartAndOnReload_OnTheSettingsInForce() + { + var worker = File.ReadAllText(Path.GetFullPath(Path.Combine( + Path.GetDirectoryName(ThisFile())!, "..", "PerformanceMonitor.Darling.Service", "DarlingWorker.cs"))); + + // After each of the two places that put the store's settings into the held config. + var start = worker.IndexOf("StoreConfigProvider.ApplyToConfig(config, initialView);", StringComparison.Ordinal); + var reload = worker.IndexOf("StoreConfigProvider.ApplyToConfig(config, view);", StringComparison.Ordinal); + Assert.True(start > 0 && reload > 0); + Assert.Contains("DarlingSmtpWarning.Log(config.Smtp, _logger);", worker.Substring(start, 2500)); + Assert.Contains("DarlingSmtpWarning.Log(config.Smtp, _logger);", worker.Substring(reload, 400)); + } + + private static string ThisFile([CallerFilePath] string path = "") => path; +} diff --git a/Darling/Darling.Tests/GridFilterBehaviourTests.cs b/Darling/Darling.Tests/GridFilterBehaviourTests.cs index 3989da3aab..7043ac72f0 100644 --- a/Darling/Darling.Tests/GridFilterBehaviourTests.cs +++ b/Darling/Darling.Tests/GridFilterBehaviourTests.cs @@ -122,7 +122,9 @@ public void CopyAndCsv_CarryTheFilteredRows_InSortOrder_WithEveryColumn() var r = Run("exportFiltered"); Assert.Equal(new[] { "Alpha", "ALPHA two" }, Arr(r, "shown")); Assert.Equal("Name\tA\tB\tTags\nAlpha\t3\t30\tred; hot\nALPHA two\t2\t20\tblue; hot", Str(r, "copy")); - Assert.Equal("Name,A,B,Tags\r\nAlpha,3,30,red; hot\r\nALPHA two,2,20,blue; hot", Str(r, "csv")); + // The Tags values hold a semicolon, so the CSV quotes them whatever the separator is: a spreadsheet whose + // list separator is a semicolon would otherwise split "red; hot" into two cells. + Assert.Equal("Name,A,B,Tags\r\nAlpha,3,30,\"red; hot\"\r\nALPHA two,2,20,\"blue; hot\"", Str(r, "csv")); Assert.Equal("Exported 2 rows.", Str(r, "status")); } diff --git a/Darling/Darling.Tests/GridToolsBehaviourTests.cs b/Darling/Darling.Tests/GridToolsBehaviourTests.cs index 79e0e9f3ca..fe9c5f622a 100644 --- a/Darling/Darling.Tests/GridToolsBehaviourTests.cs +++ b/Darling/Darling.Tests/GridToolsBehaviourTests.cs @@ -93,7 +93,8 @@ public void Copy_PutsTheFormattedTextOnTheClipboard_TabSeparated_AndFollowsTheSo var texts = r.GetProperty("texts").EnumerateArray().Select(e => e.GetString()!).ToArray(); Assert.Equal(3, texts.Length); Assert.Equal("5", texts[0]); - Assert.StartsWith("=SUM(A1)\t5\t", texts[1]); + // Copy row writes formula-looking text as text, as the desktop apps do; Copy cell (texts[0]) stays raw. + Assert.StartsWith("'=SUM(A1)\t5\t", texts[1]); var all = texts[2].Split('\n'); Assert.Equal("Name\tN\tWhen\tTags", all[0]); // Sorted ascending by N, with the thousands separator the grid shows (the CSV carries 1234567). @@ -129,12 +130,33 @@ public void TheRowHook_IsCalledOncePerRow_AndRowClassStylesTheRow_AndToolsCanBeL public void TheHelpers_QuoteNeutraliseAndNameFiles() { var r = Run("helpers"); - Assert.Equal(new[] { "\"a,b\"", "\"q\"\"q\"", "'=1+1", "'+x", "'@y", "-5", "-5", "+3.5e-2", "'-5x", "", "plain" }, + Assert.Equal(new[] { "\"a,b\"", "\"q\"\"q\"", "'=1+1", "'+x", "'@y", "-5", "-5", "+3.5e-2", "'-5x", "", "plain", "\"a;b\"", "\"a\tb\"", "a b", "' =1" }, r.GetProperty("field").EnumerateArray().Select(e => e.GetString()!).ToArray()); Assert.Equal("wait-stats-top-10-20260105-143007.csv", Str(r, "name")); Assert.Equal("table-20260105-143007.csv", Str(r, "nameEmpty")); } + [Fact] + public void CopyRowAndCopyTable_WriteFormulaLookingTextAsText_AndKeepOneCellOneField() + { + var r = Run("tsvFormulas"); + var texts = r.GetProperty("texts").EnumerateArray().Select(e => e.GetString()!).ToArray(); + + // Copy cell stays raw: query text pasted into SSMS keeps its first character. + Assert.Equal("=cmd", texts[0]); + // Copy row: a leading formula character gets an apostrophe; a line break or tab before one is a space first. + Assert.Equal("'=cmd\t-1,234.5\t-7", texts[1]); + Assert.Equal("x =HYPERLINK(1)\t-2\t+3", texts[2]); + // Copy table: the header, then one line per row, three tab-separated fields on every line. + var all = texts[3].Split('\n'); + Assert.Equal(new[] { "Q\tA\tB", "'=cmd\t-1,234.5\t-7", "x =HYPERLINK(1)\t-2\t+3", "'+1 =2\t1\t2", "'@SUM(1)\t1\t2", "'-cmd\t1\t2" }, all); + Assert.All(all, line => Assert.Equal(3, line.Split('\t').Length)); + Assert.Equal(new[] { "'=1", "'+x", "'-x", "'@y", "' =1", "-5", "-1,234.5", "-1e-3", "'-5x", "x =1", + "' =1", "' @SUM(1)", " -5", "'\u00a0=1", " x", "' =1", + "-.5", "-1e5", "'-.e5", "'-1e" }, + r.GetProperty("cells").EnumerateArray().Select(e => e.GetString()!).ToArray()); + } + [Fact] public void ColumnOptions_CsvFalseLeavesAColumnOut_AndCopyValueSuppliesTheFullText() { diff --git a/Darling/Darling.Tests/PlanRegexTimeoutTests.cs b/Darling/Darling.Tests/PlanRegexTimeoutTests.cs new file mode 100644 index 0000000000..e1fbb1f5eb --- /dev/null +++ b/Darling/Darling.Tests/PlanRegexTimeoutTests.cs @@ -0,0 +1,646 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using System.Threading; +using PerformanceMonitor.PlanAnalysis; +using Xunit; + +namespace Darling.Tests; + +/// +/// The plan-analysis project runs regular expressions on statement text and predicate strings from plan XML that a +/// caller may have written. Two limits apply: each call has a 250 ms match timeout, and all the calls of one plan +/// share a time budget. Neither one throws, and neither one quietly changes an answer: the check is recorded, the +/// analyzer notes it on the statement, and each call site answers on the side that keeps a warning. A tiny timeout +/// through the helpers' overloads, and a zero budget, force both in a few milliseconds. +/// +[Collection("timing")] +public sealed class PlanRegexTimeoutTests +{ + // Unbounded backtracking: about 2^40 ways to fail the trailing $, far past any timeout. + private const string SlowPattern = @"(a+)+$"; + private static readonly string SlowInput = new string('a', 40) + "!"; + private static readonly TimeSpan Tiny = TimeSpan.FromMilliseconds(1); + + [Fact] + public void TheCraftedInput_ReallyPassesTheTimeout() + { + // Guards the tests below against a pattern the engine happens to handle fast. + Assert.Throws(() => Regex.IsMatch(SlowInput, SlowPattern, RegexOptions.None, Tiny)); + } + + [Fact] + public void DefaultTimeout_IsAQuarterOfASecond_AndThePlanBudgetIsTwoSeconds() + { + Assert.Equal(250, PlanRegex.TimeoutMilliseconds); + Assert.Equal(2000, PlanRegex.PlanBudgetMilliseconds); + } + + // ---- A timeout: the answer the call site chose, and a record of the skipped check ---- + + [Fact] + public void IsMatch_OnTimeout_GivesTheCallersAnswer_InsteadOfThrowing() + { + using var plan = PlanRegex.BeginPlan(); + + Assert.False(PlanRegex.IsMatch(SlowInput, SlowPattern, RegexOptions.None, Tiny, "first check")); + Assert.True(PlanRegex.IsMatch(SlowInput, SlowPattern, RegexOptions.None, Tiny, "second check", onTimeout: true)); + + Assert.Equal(new[] { "first check", "second check" }, PlanRegex.TakeSkipped()); + Assert.Empty(PlanRegex.TakeSkipped()); + } + + [Fact] + public void Match_OnTimeout_ReturnsNoMatch_AndSaysItDidNotComplete() + { + using var plan = PlanRegex.BeginPlan(); + + var match = PlanRegex.Match(SlowInput, SlowPattern, RegexOptions.None, Tiny, "a match", out var completed); + + Assert.False(match.Success); + Assert.False(completed); + Assert.Equal(new[] { "a match" }, PlanRegex.TakeSkipped()); + } + + [Fact] + public void Replace_OnTimeout_ReturnsTheInputUnchanged_AndSaysItDidNotComplete() + { + using var plan = PlanRegex.BeginPlan(); + + var result = PlanRegex.Replace(SlowInput, SlowPattern, "", RegexOptions.None, Tiny, "a replace", out var completed); + + Assert.Equal(SlowInput, result); + Assert.False(completed); + Assert.Equal(new[] { "a replace" }, PlanRegex.TakeSkipped()); + } + + [Fact] + public void RegexOverloads_OnTimeout_GiveTheSameFallbacks_ForARegexInstance() + { + // A [GeneratedRegex] field carries its timeout in the instance, so the test builds one with a tiny timeout. + var regex = new Regex(SlowPattern, RegexOptions.None, Tiny); + using var plan = PlanRegex.BeginPlan(); + + Assert.False(PlanRegex.IsMatch(regex, SlowInput, "instance")); + Assert.True(PlanRegex.IsMatch(regex, SlowInput, "instance", onTimeout: true)); + Assert.False(PlanRegex.TryIsMatch(regex, SlowInput, "instance", out var matched)); + Assert.False(matched); + Assert.False(PlanRegex.Match(regex, SlowInput, "instance", out var completed).Success); + Assert.False(completed); + Assert.Equal(new[] { "instance" }, PlanRegex.TakeSkipped()); + } + + [Fact] + public void Matches_KeepsTheMatchesFoundBeforeATimeout_AndSaysThePassWasCutShort() + { + // "foo" is found at once; the scan after it hits the slow branch. + var regex = new Regex(@"foo|(a+)+$", RegexOptions.None, Tiny); + var input = "foo " + SlowInput; + using var plan = PlanRegex.BeginPlan(); + + var run = PlanRegex.Matches(regex, input, "a pass"); + var found = run.Select(m => m.Value).ToList(); + + Assert.Equal(new[] { "foo" }, found); + Assert.True(run.Incomplete); + Assert.Equal(new[] { "a pass" }, PlanRegex.TakeSkipped()); + } + + [Fact] + public void Matches_IsLazy_SoAnEarlyExitNeverReachesTheSlowTail() + { + var regex = new Regex(@"foo|(a+)+$", RegexOptions.None, Tiny); + var input = "foo " + SlowInput; + using var plan = PlanRegex.BeginPlan(); + + var run = PlanRegex.Matches(regex, input, "a pass"); + foreach (var match in run) + { + Assert.Equal("foo", match.Value); + break; + } + + Assert.False(run.Incomplete); + Assert.Empty(PlanRegex.TakeSkipped()); + } + + // ---- The plan budget ---- + + [Fact] + public void ASpentBudget_SkipsEveryFurtherCall_AndRecordsEachCheckOnce() + { + using var plan = PlanRegex.BeginPlan(TimeSpan.Zero); + + Assert.False(PlanRegex.IsMatch("TOP 5", @"\bTOP\b", RegexOptions.None, "top check")); + Assert.True(PlanRegex.IsMatch("TOP 5", @"\bTOP\b", RegexOptions.None, "top check", onTimeout: true)); + Assert.False(PlanRegex.Match("a", "a", RegexOptions.None, "a match", out var completed).Success); + Assert.False(completed); + var run = PlanRegex.Matches(new Regex("a", RegexOptions.None, TimeSpan.FromSeconds(1)), "aaa", "a pass"); + Assert.Empty(run); + Assert.True(run.Incomplete); + + Assert.Equal(new[] { "top check", "a match", "a pass" }, PlanRegex.TakeSkipped()); + } + + [Fact] + public void TheBudget_IsSharedByTheCallsOfOnePlan_NotGivenToEachOne() + { + // 30 ms in all: the first slow call runs into the budget's remainder instead of the 250 ms default, + // and the calls after it are skipped without running. + using var plan = PlanRegex.BeginPlan(TimeSpan.FromMilliseconds(30)); + var clock = System.Diagnostics.Stopwatch.StartNew(); + + Assert.False(PlanRegex.IsMatch(SlowInput, SlowPattern, RegexOptions.None, "slow one")); + var afterFirst = clock.Elapsed; + for (var i = 0; i < 20; i++) + Assert.True(PlanRegex.IsMatch(SlowInput, SlowPattern, RegexOptions.None, "slow " + i, onTimeout: true)); + + Assert.True(afterFirst < TimeSpan.FromMilliseconds(PlanRegex.TimeoutMilliseconds), $"the first call took {afterFirst}"); + Assert.True(clock.Elapsed < TimeSpan.FromSeconds(2), $"21 calls took {clock.Elapsed}"); + Assert.Equal(21, PlanRegex.TakeSkipped().Count); + } + + [Fact] + public void ABudget_IsEndedWithItsScope_AndANestedScopeSharesTheOuterOne() + { + using (var outer = PlanRegex.BeginPlan(TimeSpan.Zero)) + { + using (var inner = PlanRegex.BeginPlan(TimeSpan.FromSeconds(30))) + { + Assert.False(PlanRegex.IsMatch("a", "a", RegexOptions.None, "inside")); + } + + Assert.False(PlanRegex.IsMatch("a", "a", RegexOptions.None, "outer still spent")); + Assert.Equal(new[] { "inside", "outer still spent" }, PlanRegex.TakeSkipped()); + } + + // Outside any plan there is no budget, and nothing is recorded. + Assert.True(PlanRegex.IsMatch("a", "a", RegexOptions.None, "no plan")); + Assert.Empty(PlanRegex.TakeSkipped()); + } + + [Fact] + public void TheBudget_IsPerThread() + { + using var plan = PlanRegex.BeginPlan(TimeSpan.Zero); + var otherThread = true; + var thread = new Thread(() => otherThread = PlanRegex.IsMatch("a", "a", RegexOptions.None, "other thread")); + thread.Start(); + thread.Join(); + + Assert.True(otherThread); + } + + // ---- Normal input: the same answers as the plain Regex calls ---- + + [Theory] + [InlineData("SELECT 1 OPTION (MAXDOP 1)", @"MAXDOP\s+1\b", RegexOptions.IgnoreCase)] + [InlineData("SELECT 1 OPTION (MAXDOP 10)", @"MAXDOP\s+1\b", RegexOptions.IgnoreCase)] + [InlineData("select top 5 * from t", @"\bTOP\b", RegexOptions.None)] + [InlineData("SELECT TOP 5 * FROM t", @"\bTOP\b", RegexOptions.None)] + [InlineData("", @"\bTOP\b", RegexOptions.None)] + public void IsMatch_GivesTheSameAnswerAsRegex_OnNormalInput(string input, string pattern, RegexOptions options) + { + using var plan = PlanRegex.BeginPlan(); + + Assert.Equal(Regex.IsMatch(input, pattern, options), PlanRegex.IsMatch(input, pattern, options, "a check")); + Assert.Empty(PlanRegex.TakeSkipped()); + } + + [Fact] + public void Match_GivesTheSameMatchAsRegex_OnNormalInput() + { + const string text = "DECLARE c CURSOR LOCAL FAST_FORWARD FOR SELECT 1"; + const string pattern = @"\bDECLARE\s+\w+\s+(?:INSENSITIVE\s+|SCROLL\s+)*CURSOR\b(.*?)\bFOR\b"; + const RegexOptions options = RegexOptions.IgnoreCase | RegexOptions.Singleline; + + var expected = Regex.Match(text, pattern, options); + var actual = PlanRegex.Match(text, pattern, options, "a match", out var completed); + + Assert.True(completed); + Assert.True(actual.Success); + Assert.Equal(expected.Groups[1].Value, actual.Groups[1].Value); + Assert.Equal(" LOCAL FAST_FORWARD ", actual.Groups[1].Value); + } + + [Fact] + public void Replace_GivesTheSameTextAsRegex_OnNormalInput() + { + const string predicate = "[t].[a]=(1) AND PROBE([Bitmap1001],[t].[b])"; + const string pattern = @"\s*AND\s+PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)"; + + var actual = PlanRegex.Replace(predicate, pattern, "", RegexOptions.IgnoreCase, "a replace", out var completed); + + Assert.True(completed); + Assert.Equal(Regex.Replace(predicate, pattern, "", RegexOptions.IgnoreCase), actual); + Assert.Equal("[t].[a]=(1)", actual); + } + + [Fact] + public void Matches_ListsEveryMatch_InOrder_OnNormalInput() + { + var regex = new Regex(@"\[\w+\]", RegexOptions.None, TimeSpan.FromSeconds(5)); + + var run = PlanRegex.Matches(regex, "[a].[b].[c]", "a pass"); + + Assert.Equal(new[] { "[a]", "[b]", "[c]" }, run.Select(m => m.Value).ToArray()); + Assert.False(run.Incomplete); + } + + // ---- The analyzer: a cut-short check is named, and each site keeps its warning ---- + + private static PlanStatement Analyze(string statementText, TimeSpan budget) + { + var statement = new PlanStatement { StatementText = statementText }; + var plan = new ParsedPlan { Batches = [new PlanBatch { Statements = [statement] }] }; + PlanAnalyzer.Analyze(plan, null, null, budget, CancellationToken.None); + return statement; + } + + [Fact] + public void Analyze_WithAnEmptyBudget_DecidesTheOptimizeForUnknownHintFromTheText_AndRecordsNoSkip() + { + const string withHint = "SELECT a FROM dbo.t OPTION (OPTIMIZE FOR UNKNOWN)"; + const string withoutHint = "SELECT a FROM dbo.t WHERE a = 1"; + + var spent = Analyze(withHint, TimeSpan.Zero); + var normal = Analyze(withHint, TimeSpan.FromSeconds(30)); + var lacking = Analyze(withoutHint, TimeSpan.Zero); + + // A text search answers the question the skipped regex would have, so the answers match and no skip is named. + Assert.Contains(spent.PlanWarnings, w => w.WarningType == "Optimize For Unknown"); + Assert.Contains(normal.PlanWarnings, w => w.WarningType == "Optimize For Unknown"); + Assert.DoesNotContain(spent.PlanWarnings, w => w.Message.Contains("OPTIMIZE FOR UNKNOWN hint")); + // A statement without the hint never gets the warning, whatever the budget. + Assert.DoesNotContain(lacking.PlanWarnings, w => w.WarningType == "Optimize For Unknown"); + } + + [Fact] + public void Analyze_WithAnEmptyBudget_NamesTheSkippedChecks() + { + // The cursor declaration is read by a regex with no text-search answer: it is named as skipped. + var spent = Analyze("DECLARE c CURSOR FAST_FORWARD FOR SELECT 1", TimeSpan.Zero); + + var note = Assert.Single(spent.PlanWarnings, w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + Assert.Contains("too large or complex", note.Message); + Assert.Contains("cursor declaration", note.Message); + Assert.Equal(40, note.RuleNumber); + Assert.DoesNotContain(spent.PlanWarnings, w => w.WarningType == "Cursor Missing LOCAL"); + + var normal = Analyze("DECLARE c CURSOR FAST_FORWARD FOR SELECT 1", TimeSpan.FromSeconds(30)); + Assert.Contains(normal.PlanWarnings, w => w.WarningType == "Cursor Missing LOCAL"); + Assert.DoesNotContain(normal.PlanWarnings, w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + } + + [Fact] + public void Analyze_WithAnEmptyBudget_AddsNoSkippedChecksNote_WhenRule40IsDisabled() + { + const string cursor = "DECLARE c CURSOR FAST_FORWARD FOR SELECT 1"; + var disabled = new AnalyzerConfig { Rules = new RulesConfig { Disabled = [40] } }; + + var off = new PlanStatement { StatementText = cursor }; + PlanAnalyzer.Analyze( + new ParsedPlan { Batches = [new PlanBatch { Statements = [off] }] }, + disabled, null, TimeSpan.Zero, CancellationToken.None); + + // Rule 40 is switched off, so the skipped-checks note is not added. + Assert.DoesNotContain(off.PlanWarnings, w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + Assert.DoesNotContain(off.PlanWarnings, w => w.RuleNumber == 40); + + // The same statement and budget with the default config (rule 40 enabled) does get the note. + var on = Analyze(cursor, TimeSpan.Zero); + Assert.Single(on.PlanWarnings, w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + } + + private static PlanStatement SerialStatement(string text) => new() + { + StatementText = text, + NonParallelPlanReason = "MaxDOPSetToOne", + StatementSubTreeCost = 5.0 + }; + + private static PlanStatement AnalyzeStatement(PlanStatement statement, TimeSpan budget) + { + var plan = new ParsedPlan { Batches = [new PlanBatch { Statements = [statement] }] }; + PlanAnalyzer.Analyze(plan, null, null, budget, CancellationToken.None); + return statement; + } + + [Fact] + public void Analyze_WithAnEmptyBudget_KeepsTheSerialPlanWarningWhenTheTextHoldsMaxdop1_AndDropsItWhenItDoesNot() + { + var with = AnalyzeStatement(SerialStatement("SELECT 1 OPTION (MAXDOP 1)"), TimeSpan.Zero); + var without = AnalyzeStatement(SerialStatement("SELECT 1 FROM dbo.t"), TimeSpan.Zero); + + var warning = Assert.Single(with.PlanWarnings, w => w.WarningType == "Serial Plan"); + Assert.Equal(PlanWarningSeverity.Warning, warning.Severity); + Assert.DoesNotContain(without.PlanWarnings, w => w.WarningType == "Serial Plan"); + Assert.DoesNotContain(with.PlanWarnings, w => w.Message.Contains("MAXDOP 1 hint")); + } + + /// A clock that moves 1 ms on every read: a plan with a budget of n ms is out of time after about n reads. + private static Func SteppingClock() + { + var now = TimeSpan.Zero; + return () => now += TimeSpan.FromMilliseconds(1); + } + + [Fact] + public void Analyze_WhereverTheTimeLimitFalls_NeverReportsWhatTheTextLacks() + { + // The limit passes at a different check on each run. A statement with none of the hints below gets none + // of their warnings, and a predicate with a function only on the parameter side never reads as a column. + for (var reads = 0; reads <= 80; reads++) + { + try + { + PlanRegex.ClockForTest = SteppingClock(); + var cursor = AnalyzeStatement(new PlanStatement { StatementText = "DECLARE c CURSOR LOCAL FAST_FORWARD FOR SELECT 1 OPTION (MAXDOP 4)" }, TimeSpan.FromMilliseconds(reads)); + Assert.DoesNotContain(cursor.PlanWarnings, w => w.WarningType is "Cursor Missing LOCAL" or "Optimize For Unknown"); + + PlanRegex.ClockForTest = SteppingClock(); + var serial = AnalyzeStatement(SerialStatement("SELECT 1 FROM dbo.t"), TimeSpan.FromMilliseconds(reads)); + Assert.DoesNotContain(serial.PlanWarnings, w => w.WarningType == "Serial Plan"); + + PlanRegex.ClockForTest = SteppingClock(); + var scan = AnalyzeScanWithPredicate("[dbo].[t].[a]=isnull([@p],(1)) AND [dbo].[t].[b]=upper([@q]) AND [dbo].[t].[c]=CONVERT_IMPLICIT(int,[@r],0)", TimeSpan.FromMilliseconds(reads)); + Assert.DoesNotContain(scan.RootNode!.Warnings, w => w.Message.Contains("wrapping column") || w.Message.Contains("Function call") || w.Message.Contains("Implicit conversion")); + } + finally + { + PlanRegex.ClockForTest = null; + } + } + } + + [Fact] + public void Analyze_ADeadlineThatPassesInsideTheConvertImplicitLoop_StopsTheLoop_AndWordsTheResultAsUnverified() + { + // The clock moves with every read, so the limit passes a few matches into the loop, not at its start. + PlanRegex.ClockForTest = SteppingClock(); + try + { + using var plan = PlanRegex.BeginPlan(TimeSpan.FromMilliseconds(40)); + var reason = PlanAnalyzer.DetectNonSargablePattern(HugeConvertImplicitPredicate(5_000)); + + Assert.Equal(PlanAnalyzer.UnverifiedNonSargableReason, reason); + Assert.Contains("CONVERT_IMPLICIT in a predicate", PlanRegex.TakeSkipped()); + } + finally + { + PlanRegex.ClockForTest = null; + } + } + + [Fact] + public void Analyze_WithAnEmptyBudget_NamesTheChecksOnEveryStatement() + { + var first = new PlanStatement { StatementText = "SELECT 1 OPTION (OPTIMIZE FOR UNKNOWN)" }; + var second = new PlanStatement { StatementText = "SELECT 2 OPTION (OPTIMIZE FOR UNKNOWN)" }; + var plan = new ParsedPlan { Batches = [new PlanBatch { Statements = [first, second] }] }; + + PlanAnalyzer.Analyze(plan, null, null, TimeSpan.Zero, CancellationToken.None); + + Assert.Single(first.PlanWarnings, w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + Assert.Single(second.PlanWarnings, w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + } + + [Fact] + public void ReadsAnotherInput_WhenTheCheckIsCutShort_ProvesNothing_AndNamesTheSkippedCheck() + { + // "[@p1]" alone is a parameter list: no other input. + Assert.False(PlanAnalyzer.ReadsAnotherInput("[@p1]")); + + // A text that does name another input proves it only when the pass can read it. + Assert.True(PlanAnalyzer.ReadsAnotherInput("[dbo].[t].[c]")); + + using var plan = PlanRegex.BeginPlan(TimeSpan.Zero); + Assert.False(PlanAnalyzer.ReadsAnotherInput("[dbo].[t].[c]")); + Assert.Contains("lookup branch column references", PlanRegex.TakeSkipped()); + } + + [Fact] + public void DetectNonSargablePattern_WhenACheckIsCutShort_ReportsTheUnverifiedReason_NotNothing() + { + const string predicate = "[t].[a]=[@1]"; + Assert.Null(PlanAnalyzer.DetectNonSargablePattern(predicate)); + + using var plan = PlanRegex.BeginPlan(TimeSpan.Zero); + Assert.Equal(PlanAnalyzer.UnverifiedNonSargableReason, PlanAnalyzer.DetectNonSargablePattern(predicate)); + } + + [Fact] + public void ConvertImplicitWrapsColumn_WhenTheCheckIsCutShort_ReportsNoColumn_AndNamesTheSkippedCheck() + { + const string predicate = "[t].[a]=CONVERT_IMPLICIT(int,[@1],0)"; + Assert.False(PlanAnalyzer.ConvertImplicitWrapsColumn(predicate)); + + using var plan = PlanRegex.BeginPlan(TimeSpan.Zero); + Assert.False(PlanAnalyzer.ConvertImplicitWrapsColumn(predicate)); + Assert.Contains("CONVERT_IMPLICIT in a predicate", PlanRegex.TakeSkipped()); + } + + private static string HugeConvertImplicitPredicate(int depth) => + "[dbo].[t].[a]='" + string.Concat(Enumerable.Repeat("CONVERT_IMPLICIT(", depth)) + new string(')', depth) + "'"; + + private static PlanStatement AnalyzeScanWithPredicate(string predicate, TimeSpan budget) + { + var scan = new PlanNode + { + NodeId = 0, + PhysicalOp = "Table Scan", + LogicalOp = "Table Scan", + ObjectName = "dbo.t", + Predicate = predicate + }; + var statement = new PlanStatement { StatementText = "SELECT a FROM dbo.t", RootNode = scan }; + var plan = new ParsedPlan { Batches = [new PlanBatch { Statements = [statement] }] }; + PlanAnalyzer.Analyze(plan, null, null, budget, CancellationToken.None); + return statement; + } + + [Fact] + public void Analyze_AHugeConvertImplicitPredicate_StopsAtTheTimeLimit_AndNamesTheSkippedCheck() + { + // CONVERT_IMPLICIT( matches inside a string literal too, and each match scans to its closing parenthesis: + // the work outside the regex calls grows with the square of the predicate's length. + var predicate = HugeConvertImplicitPredicate(25_000); + var clock = System.Diagnostics.Stopwatch.StartNew(); + + var statement = AnalyzeScanWithPredicate(predicate, TimeSpan.FromMilliseconds(200)); + + Assert.True(clock.Elapsed < TimeSpan.FromSeconds(5), $"took {clock.Elapsed}"); + var note = Assert.Single(statement.RootNode!.Warnings.Concat(statement.PlanWarnings), + w => w.WarningType == PlanAnalyzer.SkippedChecksWarningType); + Assert.Contains("CONVERT_IMPLICIT in a predicate", note.Message); + } + + [Fact] + public void ManyFunctionsOnTheParameterSide_StopsAtTheBudget_AndSaysSo() + { + // Each ISNULL sits on the parameter side, so the scan walks every one, and each walk reads the predicate + // again. The budget bounds the whole pass instead of letting it run to the end. + var predicate = string.Join(" AND ", Enumerable.Range(0, 3000).Select(i => $"[t].[c{i}]=isnull([@p],(1))")); + var clock = System.Diagnostics.Stopwatch.StartNew(); + + using var plan = PlanRegex.BeginPlan(TimeSpan.FromMilliseconds(50)); + var reason = PlanAnalyzer.DetectNonSargablePattern(predicate); + + Assert.True(clock.Elapsed < TimeSpan.FromSeconds(10), $"took {clock.Elapsed}"); + // A check that could not finish keeps the warning: some reason comes back, never null. + Assert.NotNull(reason); + Assert.NotEmpty(PlanRegex.TakeSkipped()); + } + + // ---- The whole project: every regex is behind a helper and carries the timeout ---- + + private static IReadOnlyList<(string File, string Code)> ProjectSources([CallerFilePath] string thisFile = "") + { + var directory = Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", "..", "PerformanceMonitor.PlanAnalysis")); + // Comments mention the helpers by name, and some files have block comments whose continuation lines carry + // no prefix, so only the walk separates them from code. It also blanks each regex's pattern text. + return Directory.EnumerateFiles(directory, "*.cs", SearchOption.AllDirectories) + .Where(f => !f.Contains(Path.DirectorySeparatorChar + "obj" + Path.DirectorySeparatorChar) + && !f.Contains(Path.DirectorySeparatorChar + "bin" + Path.DirectorySeparatorChar)) + .Select(f => (Path.GetFileName(f), CSharpSourceWalker.StripCommentsAndStrings(File.ReadAllText(f)))) + .ToList(); + } + + /// The text between the parenthesis that opens at and its match. + private static string Arguments(string code, int open) + { + var depth = 0; + for (var i = open; i < code.Length; i++) + { + if (code[i] == '(') + depth++; + else if (code[i] == ')' && --depth == 0) + return code.Substring(open, i - open + 1); + } + + throw new Xunit.Sdk.XunitException("unbalanced parentheses"); + } + + [Fact] + public void ProjectSources_AreTheWholeProject() + { + var names = ProjectSources().Select(s => s.File).ToList(); + + Assert.Contains("PlanAnalyzer.cs", names); + Assert.Contains("ParameterCard.cs", names); + Assert.Contains("ReproScriptBuilder.cs", names); + Assert.Contains("QueryModificationDetector.cs", names); + Assert.Contains("PlanRegex.cs", names); + } + + // A static call on the Regex type, by its short or its fully qualified name. Only a word character before + // "Regex" rules it out (PlanRegex.IsMatch is a helper call). + private static readonly Regex StaticRegexCall = + new(@"(? s.File != "PlanRegex.cs").ToList(); + + // The names of every Regex-typed field, local or generated-regex method, whatever they are called. + var names = sources + .SelectMany(s => Regex.Matches(s.Code, @"\bRegex\s+(\w+)\s*(?:=|\(|;)").Cast().Select(m => m.Groups[1].Value)) + .Distinct() + .ToList(); + Assert.True(names.Count >= 15, $"expected the project's regex fields, found {names.Count}."); + + foreach (var (file, code) in sources) + { + // Static calls (Regex.IsMatch, Regex.Replace ...), also fully qualified. + Assert.DoesNotMatch(StaticRegexCall, code); + + // Calls on a field, a local or a generated-regex method, by any name. + foreach (var name in names) + { + var instanceCall = new Regex($@"(? s.File == "PlanAnalyzer.cs").Code; + Assert.True(Regex.Matches(analyzer, @"PlanRegex\.(IsMatch|TryIsMatch|Match|Matches|Replace)\(").Count >= 25, + "PlanAnalyzer.cs is expected to route its regex calls through PlanRegex."); + } + + [Fact] + public void PlanAnalysisProject_EveryRegexCarriesTheTimeout() + { + var sources = ProjectSources().Where(s => s.File != "PlanRegex.cs").ToList(); + var generated = 0; + var constructed = 0; + + foreach (var (file, code) in sources) + { + foreach (Match attribute in Regex.Matches(code, @"\[GeneratedRegex\(")) + { + generated++; + var arguments = Arguments(code, attribute.Index + attribute.Length - 1); + Assert.True(arguments.Contains("matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds"), + $"{file}: a [GeneratedRegex] has no match timeout: {arguments}"); + } + + // new Regex(...) and the target-typed form, Regex s_x = new(...). + foreach (Match construction in Regex.Matches(code, RegexConstruction)) + { + constructed++; + var arguments = Arguments(code, construction.Index + construction.Length - 1); + Assert.True(arguments.Contains("PlanRegex.DefaultTimeout") || arguments.Contains("TimeoutMilliseconds"), + $"{file}: a new Regex has no match timeout: {arguments}"); + } + } + + Assert.True(generated >= 17, $"expected the project's generated regexes, found {generated}."); + Assert.True(constructed >= 2, $"expected the compiled regexes, found {constructed}."); + } +} diff --git a/Darling/Darling.Tests/PlanTextSearchTests.cs b/Darling/Darling.Tests/PlanTextSearchTests.cs new file mode 100644 index 0000000000..48663976ce --- /dev/null +++ b/Darling/Darling.Tests/PlanTextSearchTests.cs @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Text.RegularExpressions; +using PerformanceMonitor.PlanAnalysis; +using Xunit; + +namespace Darling.Tests; + +/// +/// The plain text searches the analyzer asks when a regex is skipped must give the answer the regex gives, so a +/// statement is judged the same way whether or not the plan's time limit has passed. +/// +[Collection("timing")] +public sealed class PlanTextSearchTests +{ + private static readonly TimeSpan Limit = TimeSpan.FromSeconds(5); + + [Theory] + [InlineData("SELECT 1 OPTION (OPTIMIZE FOR UNKNOWN)")] + [InlineData("select 1 option (optimize for\tunknown)")] + [InlineData("OPTIMIZE FOR\r\nUNKNOWN")] + [InlineData("OPTIMIZE FOR (@p UNKNOWN)")] + [InlineData("OPTIMIZEFOR UNKNOWN")] + [InlineData("OPTIMIZE FOR UNKNOWNS")] + [InlineData("OPTIMIZE OPTIMIZE FOR UNKNOWN")] + [InlineData("SELECT 1")] + [InlineData("")] + public void OptimizeForUnknown_MatchesTheRegex(string text) + { + var expected = Regex.IsMatch(text, @"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase, Limit); + Assert.Equal(expected, PlanTextSearch.ContainsWords(text, false, false, "OPTIMIZE", "FOR", "UNKNOWN")); + } + + [Theory] + [InlineData("OPTION (MAXDOP 1)")] + [InlineData("option (maxdop 1)")] + [InlineData("OPTION (MAXDOP 10)")] + [InlineData("OPTION (MAXDOP 1_)")] + [InlineData("OPTION (MAXDOP1)")] + [InlineData("OPTION (MAXDOP MAXDOP 1)")] + [InlineData("MAXDOP 1")] + [InlineData("MAXDOP")] + [InlineData("OPTION (MAXDOP 4)")] + public void MaxdopOne_MatchesTheRegex(string text) + { + var expected = Regex.IsMatch(text, @"MAXDOP\s+1\b", RegexOptions.IgnoreCase, Limit); + Assert.Equal(expected, PlanTextSearch.ContainsWords(text, false, true, "MAXDOP", "1")); + } + + [Theory] + [InlineData("WHERE a NOT IN (SELECT b FROM t)")] + [InlineData("WHERE a not in(1)")] + [InlineData("WHERE a NOT INTEGER")] + [InlineData("WHERE a ANOT IN (1)")] + [InlineData("WHERE a IN (1)")] + [InlineData("NOT")] + public void NotIn_MatchesTheRegex(string text) + { + var expected = Regex.IsMatch(text, @"\bNOT\s+IN\b", RegexOptions.IgnoreCase, Limit); + Assert.Equal(expected, PlanTextSearch.ContainsWords(text, true, true, "NOT", "IN")); + } + + [Theory] + [InlineData(" LOCAL FAST_FORWARD ")] + [InlineData(" local ")] + [InlineData(" LOCALE ")] + [InlineData(" NONLOCAL ")] + [InlineData(" _LOCAL ")] + [InlineData("LOCAL")] + [InlineData(" STATIC ")] + public void Local_MatchesTheRegex(string text) + { + var expected = Regex.IsMatch(text, @"\bLOCAL\b", RegexOptions.IgnoreCase, Limit); + Assert.Equal(expected, PlanTextSearch.ContainsWords(text, true, true, "LOCAL")); + } + + [Theory] + [InlineData("PROBE([Bitmap1001],[t].[b])")] + [InlineData("PROBE([Bitmap1001],[t].[b]) AND PROBE([Bitmap1002],[t].[c])")] + [InlineData("probe ([Bitmap1001],f(x)) or PROBE([Bitmap1002],[t].[c])")] + [InlineData("[t].[a]=(1) AND PROBE([Bitmap1001],[t].[b])")] + [InlineData("PROBE([Bitmap1001],[t].[b]) AND [t].[a]=(1)")] + [InlineData("PROBE([Bitmap1001],[t].[b]")] + [InlineData("PROBE")] + [InlineData("ANDROID")] + public void ProbeOnly_AgreesWithTheRegexPair_OnTheShapesAPlanHolds(string predicate) + { + var stripped = Regex.Replace(predicate, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", RegexOptions.IgnoreCase, Limit).Trim(); + stripped = Regex.Replace(stripped, @"\b(AND|OR)\b", "", RegexOptions.IgnoreCase, Limit).Trim(); + + Assert.Equal(stripped.Length == 0, PlanTextSearch.IsProbeOnly(predicate)); + } + + [Fact] + public void TheSearches_AreLinear_OnALongInputWithManyNearMisses() + { + var text = string.Concat(System.Linq.Enumerable.Repeat("OPTIMIZE FOR ", 200_000)) + "UNKNOWN"; + var clock = System.Diagnostics.Stopwatch.StartNew(); + + Assert.True(PlanTextSearch.ContainsWords(text, false, false, "OPTIMIZE", "FOR", "UNKNOWN")); + Assert.False(PlanTextSearch.ContainsWords(new string(' ', 4_000_000), true, true, "NOT", "IN")); + Assert.False(PlanTextSearch.ContainsWords("MAXDOP" + new string(' ', 4_000_000), false, true, "MAXDOP", "1")); + Assert.True(clock.Elapsed < TimeSpan.FromSeconds(5), $"took {clock.Elapsed}"); + } +} diff --git a/Darling/Darling.Tests/ViewerContextMenuTests.cs b/Darling/Darling.Tests/ViewerContextMenuTests.cs index a9a198d4a4..54b72ca099 100644 --- a/Darling/Darling.Tests/ViewerContextMenuTests.cs +++ b/Darling/Darling.Tests/ViewerContextMenuTests.cs @@ -100,7 +100,8 @@ public void ChartCsvHeaderLine_QuotesAZoneNameContainingTheSeparator() var zone = TimeZoneInfo.CreateCustomTimeZone("Custom, Zone", TimeSpan.Zero, "Custom", "Custom"); Assert.Equal("\"DateTime (Custom, Zone)\",Series,Value,UTC offset", ViewerServerTab.ChartCsvHeaderLine(",", zone)); - Assert.Equal("DateTime (Custom, Zone);Series;Value;UTC offset", ViewerServerTab.ChartCsvHeaderLine(";", zone)); + /* A comma is quoted whatever the separator is: a spreadsheet may split on its own list separator. */ + Assert.Equal("\"DateTime (Custom, Zone)\";Series;Value;UTC offset", ViewerServerTab.ChartCsvHeaderLine(";", zone)); } [Fact] @@ -188,11 +189,26 @@ public void FormatChartCsvLine_QuotesASeriesNameContainingTheSeparator() Assert.Equal("2026-07-06 00:00:00,\"reads, writes\",3", line); } + [Fact] + public void FormatChartCsvLine_WritesAFormulaLookingSeriesAsText_AndANegativeValueAsANumber() + { + /* A series name is collected text. One that starts with a formula character is written with a leading + apostrophe so a spreadsheet shows it instead of evaluating it; the value stays a number. */ + var line = ViewerServerTab.FormatChartCsvLine(new DateTime(2026, 7, 6, 0, 0, 0), "=1+1", -5, ","); + Assert.Equal("2026-07-06 00:00:00,'=1+1,-5", line); + } + [Theory] [InlineData("plain", ",", "plain")] [InlineData("a,b", ",", "\"a,b\"")] [InlineData("a;b", ";", "\"a;b\"")] [InlineData("has\"quote", ",", "\"has\"\"quote\"")] + [InlineData("a,b", ";", "\"a,b\"")] + [InlineData("a;b", ",", "\"a;b\"")] + [InlineData("a\tb", ",", "\"a\tb\"")] + [InlineData("a\nb", ";", "\"a\nb\"")] + [InlineData("a\rb", ";", "\"a\rb\"")] + [InlineData("a|b", "|", "\"a|b\"")] public void CsvEscape_QuotesOnlyWhenNeeded_AndDoublesEmbeddedQuotes(string value, string separator, string expected) { Assert.Equal(expected, ViewerServerTab.CsvEscape(value, separator)); diff --git a/Darling/Darling.Tests/web-grid-tools-harness.mjs b/Darling/Darling.Tests/web-grid-tools-harness.mjs index 357da0cb1f..3cb98b8bf7 100644 --- a/Darling/Darling.Tests/web-grid-tools-harness.mjs +++ b/Darling/Darling.Tests/web-grid-tools-harness.mjs @@ -146,8 +146,30 @@ const scenarios = { out.texts = texts; out.goneStatus = status(gone); }, + async tsvFormulas() { + const texts = []; + setNav({ clipboard: { writeText: async (t) => { texts.push(t); } } }); + const trows = [ + { q: "=cmd", a: "-1,234.5", b: -7 }, + { q: "x\n=HYPERLINK(1)", a: "-2", b: "+3" }, + { q: "+1\t=2", a: "1", b: "2" }, + { q: "@SUM(1)", a: "1", b: "2" }, + { q: "-cmd", a: "1", b: "2" }, + ]; + const tcols = [{ key: "q", label: "Q" }, { key: "a", label: "A" }, { key: "b", label: "B" }]; + const w = VIZ.table({ rows: trows }, { rowsKey: "rows", columns: tcols, title: "Tsv" }); + const tb = tbodyOf(w); + tb.fire("click", { target: tb.children[0].children[0] }); + btn(w, "Copy cell").click(); await flush(); + btn(w, "Copy row").click(); await flush(); + tb.fire("click", { target: tb.children[1].children[0] }); + btn(w, "Copy row").click(); await flush(); + btn(w, "Copy all").click(); await flush(); + out.texts = texts; + out.cells = ["=1", "+x", "-x", "@y", "\t=1", "-5", "-1,234.5", "-1e-3", "-5x", "x\n=1", " =1", "\r@SUM(1)", " -5", "\u00a0=1", " x", "\n\t =1", "-.5", "-1e5", "-.e5", "-1e"].map(tools.tsvCell); + }, async helpers() { - out.field = [tools.csvField("a,b"), tools.csvField('q"q'), tools.csvField("=1+1"), tools.csvField("+x"), tools.csvField("@y"), tools.csvField(-5), tools.csvField("-5"), tools.csvField("+3.5e-2"), tools.csvField("-5x"), tools.csvField(null), tools.csvField("plain")]; + out.field = [tools.csvField("a,b"), tools.csvField('q"q'), tools.csvField("=1+1"), tools.csvField("+x"), tools.csvField("@y"), tools.csvField(-5), tools.csvField("-5"), tools.csvField("+3.5e-2"), tools.csvField("-5x"), tools.csvField(null), tools.csvField("plain"), tools.csvField("a;b"), tools.csvField("a\tb"), tools.csvField("a b"), tools.csvField(" =1")]; out.name = tools.csvFileName("Wait Stats / Top 10!", new Date("2026-01-05T14:30:07Z")); out.nameEmpty = tools.csvFileName("", new Date("2026-01-05T14:30:07Z")); }, diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingSmtpWarning.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingSmtpWarning.cs new file mode 100644 index 0000000000..e08d27e63c --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingSmtpWarning.cs @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using Microsoft.Extensions.Logging; +using PerformanceMonitor.Notifications; + +namespace PerformanceMonitor.Darling.Service; + +/// +/// The service-log form of the settings windows' SMTP warning. A setup with SSL off sends its login to the mail +/// server without encryption. The settings windows say so when someone saves, tests or validates; this says so +/// in the service log for the setups no window touched: one declared in darling.json, and one saved before the +/// windows warned. It runs at service start and on every config reload, on the settings the service holds. +/// +internal static class DarlingSmtpWarning +{ + /// + /// The warning for these settings, or null when they are fine or SMTP is not set up. The same rule the + /// windows use (). The password counts as set when + /// either password field holds text; its value is never read. + /// + internal static string? Warning(SmtpConfig smtp) + { + if (string.IsNullOrWhiteSpace(smtp.Host)) + { + return null; + } + + var passwordSet = !string.IsNullOrWhiteSpace(smtp.EncryptedPassword) || !string.IsNullOrWhiteSpace(smtp.Password); + return EmailSendCore.CleartextCredentialsWarning(smtp.UseSsl, smtp.Username, passwordSet); + } + + /// Logs at Warning level when there is one. + internal static void Log(SmtpConfig smtp, ILogger logger) + { + var warning = Warning(smtp); + if (warning is not null) + { + logger.LogWarning("SMTP settings for {Host}:{Port}: {Warning}", smtp.Host, smtp.Port, warning); + } + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs index 3d6cd9a61d..8a3f39d131 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs @@ -3143,6 +3143,9 @@ read is empty (a partially-seeded store), so the service never starts up monitor holds — including on a store-unreachable boot. Re-published on every reload below. */ _mcpState.Publish(config.Mcp.Enabled, config.Mcp.Port); _webState.Publish(config.Web.Enabled, config.Web.Port); + /* The SMTP settings in force (the store's when the view loaded, else darling.json's): a setup that sends its + login without encryption is named in the log, because no settings window is open for it. */ + DarlingSmtpWarning.Log(config.Smtp, _logger); /* #2298: publish the effective server set the same way — store-authoritative when the view loaded, else the darling.json servers, which is what this run will actually collect from either way. The MCP host's plan-fetch resolver reads this instead of re-reading the store as the mcp role. */ @@ -6286,6 +6289,7 @@ forever. Cheap (an indexed UPDATE matching nothing in the normal case) and safe } StoreConfigProvider.ApplyToConfig(config, view); + DarlingSmtpWarning.Log(config.Smtp, _logger); // the reloaded SMTP settings, same warning as at start /* #1560/#1562: the MCP + web host supervisors pick these up within their poll interval — the viewer's Settings toggles round-trip to a live start/stop/rebind with no service restart. */ _mcpState.Publish(config.Mcp.Enabled, config.Mcp.Port); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanTools.cs index be7f0722f4..3178b05dd7 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanTools.cs @@ -189,6 +189,11 @@ public static string AnalyzePlanXml( if (string.IsNullOrWhiteSpace(plan_xml)) return McpHelpers.Refusal("plan_xml", "No plan XML provided."); + // The caller wrote this XML, so bound it before it reaches the parser (the stored-plan tools skip this). + var tooLarge = CallerPlanXml.RefusalReason(plan_xml); + if (tooLarge != null) + return McpHelpers.Refusal("plan_xml", tooLarge); + try { // #4348: the caller's XML has no stored-plan read to filter it, so AnalyzeFilteredPlan runs the statement diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/grid-tools.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/grid-tools.js index 1631863691..bf3836fab9 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/grid-tools.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/grid-tools.js @@ -14,23 +14,41 @@ export function plainCell(text) { return String(text ?? "").replace(/[\t\r\n]+/g, " "); } +/* A spreadsheet runs a cell that starts with one of these as a formula. A leading apostrophe makes it text. A real + number is left alone (a negative value is data, not a formula), and so is a string that is only a number + ("-5", "+3", "1e-3", "-1,234.5", "-.5"); any other string is neutralised. A spreadsheet drops leading white space and + control characters, so the rule looks at the first character after them. A string whose very first character is a + tab or a carriage return is neutralised as well. Mirrors SpreadsheetText.Neutralize. */ +const FORMULA_LEAD = /^[=+\-@]/; +const TAB_OR_CR_LEAD = /^[\t\r]/; +const LEADING_SKIPPED = /^[\s\u0000-\u001f\u007f-\u009f\ufeff]+/; +// The same strings SpreadsheetText.IsPlainNumber accepts: an optional sign, a run of digits, "." and "," with at +// least one digit (".5", "1,234.5"), and an optional exponent. The leading [.,]* keeps the match linear. +const PLAIN_NUMBER = /^[+-]?[.,]*\d[\d.,]*([eE][+-]?\d+)?$/; + +/** The text with a leading apostrophe when a spreadsheet would read it as a formula; any other text comes back as it is. */ +export function neutralise(s) { + if (TAB_OR_CR_LEAD.test(s)) return "'" + s; + const lead = s.replace(LEADING_SKIPPED, ""); + return FORMULA_LEAD.test(lead) && !PLAIN_NUMBER.test(lead) ? "'" + s : s; +} + +/** One cell as Copy row and Copy table write it: line breaks collapsed to a space first, then written as text when it starts like a formula. */ +export function tsvCell(text) { + return neutralise(plainCell(text)); +} + /** Rows of already-formatted cells as tab-separated lines (the form a spreadsheet pastes into columns). */ export function toTsv(rows) { - return rows.map((r) => r.map(plainCell).join("\t")).join("\n"); + return rows.map((r) => r.map(tsvCell).join("\t")).join("\n"); } -/* A spreadsheet runs a cell that starts with one of these as a formula. A leading apostrophe makes it text. A real - number is left alone (a negative value is data, not a formula), and so is a string that is only a number - ("-5", "+3", "1e-3"); any other string is neutralised. */ -const FORMULA_LEAD = /^[=+\-@\t\r]/; -const PLAIN_NUMBER = /^[+-]?\d+(\.\d+)?([eE][+-]?\d+)?$/; - -/** One CSV field, RFC 4180: quoted when it holds a comma, quote, CR or LF, with quotes doubled. */ +/** One CSV field, RFC 4180: quoted when it holds a comma, semicolon, tab, quote, CR or LF, with quotes doubled. */ export function csvField(v) { if (v == null) return ""; let s = v instanceof Date ? v.toISOString() : String(v); - if (typeof v === "string" && FORMULA_LEAD.test(s) && !PLAIN_NUMBER.test(s)) s = "'" + s; - return /[",\r\n]/.test(s) ? '"' + s.replace(/"/g, '""') + '"' : s; + if (typeof v === "string") s = neutralise(s); + return /[",;\t\r\n]/.test(s) ? '"' + s.replace(/"/g, '""') + '"' : s; } /** Rows (arrays of raw values) as CSV text, CRLF-separated per RFC 4180. */ diff --git a/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs b/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs index 5b4a68d3a5..eb134062be 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs @@ -1581,7 +1581,8 @@ private void ValidateSmtpButton_Click(object sender, RoutedEventArgs e) if (errors.Count == 0) { - SmtpStatusText.Text = "Settings look good. Use 'Send Test Email' to verify delivery."; + SmtpStatusText.Text = CleartextSmtpWarning(includeStoredPassword: true) + ?? "Settings look good. Use 'Send Test Email' to verify delivery."; } else { @@ -1592,8 +1593,50 @@ private void ValidateSmtpButton_Click(object sender, RoutedEventArgs e) } } + /// + /// The unencrypted-login warning for the SMTP settings as they stand in the boxes, or null. A Save keeps + /// the store's existing password when the box is empty (), so it counts that + /// one too (); a test sends only what the box holds. + /// + private string? CleartextSmtpWarning(bool includeStoredPassword) => + EmailSendCore.CleartextCredentialsWarning( + SmtpSslCheckBox.IsChecked == true, + SmtpUsernameBox.Text?.Trim(), + !string.IsNullOrEmpty(SmtpPasswordBox.Password) + || (includeStoredPassword && !string.IsNullOrEmpty(_loadedNotification.SmtpEncryptedPassword))); + + /// + /// Non-blocking confirm shown at Save and Send Test when SSL is off and a login is set, so the username + /// and password would go to the mail server unencrypted. Yes proceeds; No cancels the action. + /// is the verb ("Save" / "Send"). Not blocked outright: an internal relay on + /// port 25 is a real setup. + /// + private bool ConfirmCleartextSmtp(string action, bool includeStoredPassword) + { + if (SmtpEnabledCheckBox.IsChecked != true) + { + return true; + } + + var warning = CleartextSmtpWarning(includeStoredPassword); + if (warning == null) + { + return true; + } + + var result = MessageBox.Show( + $"{warning}\n\n{action} anyway?", + "Unencrypted SMTP Login", MessageBoxButton.YesNo, MessageBoxImage.Warning); + return result == MessageBoxResult.Yes; + } + private async void TestEmailButton_Click(object sender, RoutedEventArgs e) { + if (!ConfirmCleartextSmtp("Send", includeStoredPassword: false)) + { + return; + } + TestEmailButton.IsEnabled = false; TestEmailButton.Content = "Sending..."; @@ -1890,6 +1933,14 @@ to fix the URL. NOT blocked — a plaintext POST to a trusted LAN listener is le return; } + /* An SMTP login that would go to the mail server unencrypted (SSL off with a password set) is confirmed + the same way, before anything is persisted. NOT blocked - an internal relay on port 25 is a real + setup. */ + if (!ConfirmCleartextSmtp("Save", includeStoredPassword: true)) + { + return; + } + /* Persist the viewer-LOCAL preferences immediately (valid values applied above), and capture the edited viewer preferences for MainWindow to save + re-seed tabs from. */ SaveConnectionTimeout(); diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.ChartContextMenu.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.ChartContextMenu.cs index b9223df4b0..71e24a5ef2 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.ChartContextMenu.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.ChartContextMenu.cs @@ -102,7 +102,7 @@ internal static string FormatChartCsvLine(DateTime dateTime, string series, doub string.Join(separator, new[] { dateTime.ToString("yyyy-MM-dd HH:mm:ss", CultureInfo.InvariantCulture), - CsvEscape(series, separator), + CsvEscape(SpreadsheetText.Neutralize(series), separator), value.ToString(CultureInfo.InvariantCulture), }); @@ -123,15 +123,7 @@ internal static string ChartCsvDataLine(double plottedX, string series, double v } /// RFC-4180 CSV quoting (Lite's ContextMenuHelper.CsvEscape). - internal static string CsvEscape(string value, string separator) - { - if (value.Contains(separator, StringComparison.Ordinal) || value.Contains('"') || value.Contains('\n') || value.Contains('\r')) - { - return "\"" + value.Replace("\"", "\"\"", StringComparison.Ordinal) + "\""; - } - - return value; - } + internal static string CsvEscape(string value, string separator) => DataGridExport.CsvEscape(value, separator); /// /// Builds the copy/save/export for a chart and takes right-click over from diff --git a/Lite.Tests/CallerPlanXmlLiteTests.cs b/Lite.Tests/CallerPlanXmlLiteTests.cs new file mode 100644 index 0000000000..209bc35fca --- /dev/null +++ b/Lite.Tests/CallerPlanXmlLiteTests.cs @@ -0,0 +1,94 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Diagnostics; +using System.IO; +using System.Runtime.CompilerServices; +using System.Text; +using System.Text.Json; +using PerformanceMonitor.Common; +using PerformanceMonitor.PlanAnalysis; +using PerformanceMonitorLite.Mcp; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// Lite's side of the caller-supplied plan XML limits (the count itself is pinned in Darling.Tests' +/// CallerPlanXmlTests): Lite's analyze_plan_xml refuses an element with too many attributes +/// before it parses, still analyzes a plan just under the limit, and Lite's Ctrl+V on the server tab checks +/// pasted text before it opens a plan tab. +/// +[Collection("timing")] +public sealed class CallerPlanXmlLiteTests +{ + private const string Ns = "http://schemas.microsoft.com/sqlserver/2004/07/showplan"; + + private const string SmallPlan = + "" + + "" + + "" + + ""; + + private static string PlanWithWideElement(int attributes) + { + var sb = new StringBuilder(attributes * 12 + 1024); + sb.Append(""); + sb.Append("").Append(SmallPlan).Append(""); + sb.Append(""); + return sb.ToString(); + } + + [Fact] + public void AnalyzePlanXml_OverTheLimit_IsRefusedBeforeParsing() + { + var xml = PlanWithWideElement(1_000_000); + var reason = CallerPlanXml.RefusalReason(xml); + Assert.NotNull(reason); + + var sw = Stopwatch.StartNew(); + var result = McpPlanTools.AnalyzePlanXml(xml); + sw.Stop(); + + Assert.Equal(McpHelpers.Refusal("plan_xml", reason!), result); + Assert.Contains(CallerPlanXml.MaxAttributesPerElement.ToString("N0"), McpHelpers.ErrorMessageOf(result)); + Assert.True(sw.ElapsedMilliseconds < 1000, $"analyze_plan_xml took {sw.ElapsedMilliseconds} ms to refuse; parsing it takes about 27 s."); + } + + [Fact] + public void AnalyzePlanXml_AtTheLimit_StillAnalyzes() + { + var xml = PlanWithWideElement(CallerPlanXml.MaxAttributesPerElement); + Assert.Null(CallerPlanXml.RefusalReason(xml)); + + using var doc = JsonDocument.Parse(McpPlanTools.AnalyzePlanXml(xml)); + + Assert.Equal("xml", doc.RootElement.GetProperty("source").GetString()); + Assert.True(doc.RootElement.GetProperty("statement_count").GetInt32() >= 1); + } + + [Fact] + public void ServerTabPaste_ChecksThePastedText_BeforeOpeningAPlanTab() + { + var source = File.ReadAllText(ServerTabCs()); + var read = source.IndexOf("ClipboardText.TryReadAsync(", StringComparison.Ordinal); + var gate = source.IndexOf("CallerPlanXmlPrompt.Refuse(", StringComparison.Ordinal); + var open = source.IndexOf("await OpenPlanTab(xml, \"Pasted Plan\")", StringComparison.Ordinal); + + Assert.True(read >= 0 && gate > read && open > gate, + "ServerTab_KeyDown must read the clipboard, check the text with CallerPlanXmlPrompt.Refuse, then open the plan tab."); + Assert.Equal(1, (source.Length - source.Replace("ClipboardText.TryReadAsync(", "", StringComparison.Ordinal).Length) / "ClipboardText.TryReadAsync(".Length); + } + + private static string ServerTabCs([CallerFilePath] string thisFile = "") => + Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", "Lite", "Controls", "ServerTab.xaml.cs")); +} diff --git a/Lite.Tests/SmtpCleartextWarningTests.cs b/Lite.Tests/SmtpCleartextWarningTests.cs new file mode 100644 index 0000000000..7af1f88322 --- /dev/null +++ b/Lite.Tests/SmtpCleartextWarningTests.cs @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using PerformanceMonitor.Notifications; +using Xunit; + +namespace Lite.Tests; + +/// +/// With SSL off, the SMTP client ignores a STARTTLS offer and sends the login in the clear. The settings +/// windows warn at Save, Test and Validate when that would happen. The rule is one shared predicate; these +/// tests pin it, and pin that both windows (Lite, and the Darling viewer) call it from all three handlers. +/// +[Trait("Reads", "Darling")] +public sealed class SmtpCleartextWarningTests +{ + [Fact] + public void SslOffWithAUsernameAndPassword_WarnsAndNamesTheRisk() + { + var warning = EmailSendCore.CleartextCredentialsWarning(useSsl: false, username: "relay", passwordSet: true); + + Assert.NotNull(warning); + Assert.Contains("SSL is off", warning); + Assert.Contains("without encryption", warning); + } + + [Theory] + [InlineData(true, "relay", true)] // SSL on: the login is encrypted + [InlineData(false, "relay", false)] // no password: nothing secret to send + [InlineData(false, "", true)] // no username: the send attaches no credentials at all + [InlineData(false, " ", true)] + [InlineData(false, null, true)] + [InlineData(true, "", false)] + public void EveryOtherCombination_DoesNotWarn(bool useSsl, string? username, bool passwordSet) + { + Assert.Null(EmailSendCore.CleartextCredentialsWarning(useSsl, username, passwordSet)); + } + + [Theory] + [InlineData("Lite/Windows/SettingsWindow.xaml.cs", "SaveButton_Click", "ConfirmCleartextSmtp(\"Save\"")] + [InlineData("Lite/Windows/SettingsWindow.xaml.cs", "TestEmailButton_Click", "ConfirmCleartextSmtp(\"Send\"")] + [InlineData("Lite/Windows/SettingsWindow.xaml.cs", "ValidateSmtpButton_Click", "CleartextSmtpWarning(")] + [InlineData("Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs", "SaveButton_Click", "ConfirmCleartextSmtp(\"Save\"")] + [InlineData("Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs", "TestEmailButton_Click", "ConfirmCleartextSmtp(\"Send\"")] + [InlineData("Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs", "ValidateSmtpButton_Click", "CleartextSmtpWarning(")] + public void EachSettingsWindowHandler_ChecksTheWarning(string file, string handler, string call) + { + var body = MethodBody(ParitySource.ReadFile(file), handler); + + Assert.Contains(call, body); + } + + [Theory] + [InlineData("Lite/Windows/SettingsWindow.xaml.cs")] + [InlineData("Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs")] + public void EachSettingsWindow_AsksTheSharedPredicate(string file) + { + var source = ParitySource.ReadFile(file); + + Assert.Contains("EmailSendCore.CleartextCredentialsWarning(", source); + } + + /// The text of the method whose declaration contains (, by brace matching. + private static string MethodBody(string source, string name) + { + var declaration = source.IndexOf(name + "(object sender", System.StringComparison.Ordinal); + Assert.True(declaration >= 0, $"{name} was not found"); + + var open = source.IndexOf('{', declaration); + var depth = 0; + for (var i = open; i < source.Length; i++) + { + if (source[i] == '{') + { + depth++; + } + else if (source[i] == '}' && --depth == 0) + { + return source.Substring(open, i - open + 1); + } + } + + throw new Xunit.Sdk.XunitException($"The braces of {name} do not balance"); + } +} diff --git a/Lite.Tests/SmtpLoadedWarningTests.cs b/Lite.Tests/SmtpLoadedWarningTests.cs new file mode 100644 index 0000000000..7ab78514c1 --- /dev/null +++ b/Lite.Tests/SmtpLoadedWarningTests.cs @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using PerformanceMonitorLite; +using Xunit; + +namespace Lite.Tests; + +/// +/// An SMTP setup saved before the settings window warned about sending the login without encryption is named in +/// the Lite log when it loads, and the same shared predicate decides. Shares the collection of the other classes +/// that write the App alert statics. +/// +[Collection("app-alert-statics")] +[Trait("Reads", "Darling")] +public sealed class SmtpLoadedWarningTests +{ + private static string? WithSettings(bool enabled, bool useSsl, string username, string? password, out bool passwordRead) + { + var (e, s, u) = (App.SmtpEnabled, App.SmtpUseSsl, App.SmtpUsername); + var read = false; + try + { + App.SmtpEnabled = enabled; + App.SmtpUseSsl = useSsl; + App.SmtpUsername = username; + var warning = App.LoadedSmtpCleartextWarning(() => { read = true; return password; }); + passwordRead = read; + return warning; + } + finally + { + (App.SmtpEnabled, App.SmtpUseSsl, App.SmtpUsername) = (e, s, u); + } + } + + [Fact] + public void ASetupWithSslOffAndALogin_WarnsWhenItLoads() + { + var warning = WithSettings(enabled: true, useSsl: false, username: "relay", password: "secret", out _); + + Assert.NotNull(warning); + Assert.Contains("without encryption", warning); + } + + [Theory] + [InlineData(false, false, "relay", "secret")] // SMTP is off + [InlineData(true, true, "relay", "secret")] // SSL on + [InlineData(true, false, "", "secret")] // no username: no credentials are sent + [InlineData(true, false, "relay", null)] // no password stored + public void EveryOtherSetup_DoesNotWarn(bool enabled, bool useSsl, string username, string? password) + { + Assert.Null(WithSettings(enabled, useSsl, username, password, out _)); + } + + [Fact] + public void ThePassword_IsOnlyReadWhenTheOtherConditionsHold() + { + WithSettings(enabled: true, useSsl: true, username: "relay", password: "secret", out var readWithSsl); + WithSettings(enabled: true, useSsl: false, username: "relay", password: "secret", out var readWithoutSsl); + + Assert.False(readWithSsl); + Assert.True(readWithoutSsl); + } + + [Fact] + public void LiteStartup_LogsTheLoadedWarning() + { + var source = ParitySource.ReadFile("Lite/App.xaml.cs"); + + Assert.Contains("LoadedSmtpCleartextWarning(GetSmtpPassword)", source); + Assert.Contains("AppLogger.Warn(\"SMTP\", smtpWarning)", source); + } +} diff --git a/Lite.Tests/SpreadsheetTextTests.cs b/Lite.Tests/SpreadsheetTextTests.cs new file mode 100644 index 0000000000..2a6b58d8c2 --- /dev/null +++ b/Lite.Tests/SpreadsheetTextTests.cs @@ -0,0 +1,412 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Linq; +using System.Windows.Controls; +using System.Windows.Data; +using PerformanceMonitor.Ui; +using PerformanceMonitorLite.Helpers; +using Xunit; + +namespace Lite.Tests; + +/// +/// A spreadsheet that opens an exported CSV, or receives copied cells, evaluates a text cell that starts with +/// =, +, - or @ (or a tab or carriage return). Every grid copy that lands in a +/// spreadsheet (Copy Row, Copy All Rows, Ctrl+C on selected rows), grid CSV export and chart CSV export writes +/// such text with a leading apostrophe, and writes numbers, negative ones too, as numbers. The single-cell Copy +/// Cell is the exception: it copies the text exactly as it is, because a cell is often query text on its way +/// into SSMS, where a leading apostrophe breaks a line that starts with --. These tests pin the rule and +/// each path that applies it. +/// +public sealed class SpreadsheetTextTests +{ + private sealed class Row + { + public string Name { get; set; } = ""; + public int Value { get; set; } + } + + private static T RunSta(Func body) => StaTestThread.Run(body); + + private static DataGrid BuildGrid() + { + var grid = new DataGrid(); + grid.Columns.Add(new DataGridTextColumn { Header = "Name", Binding = new Binding(nameof(Row.Name)) }); + grid.Columns.Add(new DataGridTextColumn { Header = "Value", Binding = new Binding(nameof(Row.Value)) }); + grid.ItemsSource = new[] { new Row { Name = "=1+1", Value = -5 } }; + return grid; + } + + private static DataGrid BuildGridWithTexts(params string[] names) + { + var grid = new DataGrid(); + grid.Columns.Add(new DataGridTextColumn { Header = "Name", Binding = new Binding(nameof(Row.Name)) }); + grid.Columns.Add(new DataGridTextColumn { Header = "Value", Binding = new Binding(nameof(Row.Value)) }); + grid.ItemsSource = names.Select((n, i) => new Row { Name = n, Value = -(i + 1) }).ToArray(); + return grid; + } + + [Theory] + [InlineData("=1+1", "'=1+1")] + [InlineData("+1+1", "'+1+1")] + [InlineData("-1+1", "'-1+1")] + [InlineData("@SUM(1)", "'@SUM(1)")] + [InlineData("\t=1", "'\t=1")] + [InlineData("\r=1", "'\r=1")] + [InlineData(" =1+1", "' =1+1")] + [InlineData(" +1+1", "' +1+1")] + [InlineData(" -1+1", "' -1+1")] + [InlineData(" @SUM(1)", "' @SUM(1)")] + [InlineData(" \t=1", "' \t=1")] + [InlineData(" \r@SUM(1)", "' \r@SUM(1)")] + [InlineData("\t =1", "'\t =1")] + [InlineData("\n=1", "'\n=1")] + [InlineData("\r\n=1", "'\r\n=1")] + [InlineData("\n\n+1+1", "'\n\n+1+1")] + [InlineData(" \n\t\r -1+1", "' \n\t\r -1+1")] + [InlineData("\u00a0=1", "'\u00a0=1")] + [InlineData("\u0001=1", "'\u0001=1")] + [InlineData(" -- a comment", "' -- a comment")] + [InlineData("-- a comment", "'-- a comment")] + [InlineData("-", "'-")] + [InlineData("-1e", "'-1e")] + public void TextThatStartsLikeAFormula_GetsALeadingApostrophe(string text, string expected) + { + Assert.Equal(expected, SpreadsheetText.Neutralize(text)); + } + + [Theory] + [InlineData("-5")] + [InlineData("+5")] + [InlineData("-0.5")] + [InlineData("-1,234.5")] + [InlineData("-1.5E-05")] + [InlineData("5")] + [InlineData(" -5")] + [InlineData(" -1,234.5")] + [InlineData(" +5")] + [InlineData("\n-1.5E-05")] + [InlineData(" \n -0.5")] + public void PlainNumbers_StayNumbers(string text) + { + Assert.Equal(text, SpreadsheetText.Neutralize(text)); + } + + [Theory] + [InlineData("SELECT 1")] + [InlineData("a=b")] + [InlineData("'=1")] + [InlineData("")] + [InlineData(" ")] + [InlineData(" \n ")] + [InlineData("\n")] + [InlineData("\nSELECT 1")] + [InlineData(" a=1")] + [InlineData(" 'x")] + public void OtherText_IsUnchanged(string text) + { + Assert.Equal(text, SpreadsheetText.Neutralize(text)); + } + + [Fact] + public void Neutralize_IsIdempotent_AndTreatsNullAsEmpty() + { + var once = SpreadsheetText.Neutralize("=1+1"); + + Assert.Equal(once, SpreadsheetText.Neutralize(once)); + Assert.Equal("", SpreadsheetText.Neutralize(null)); + } + + [Fact] + public void NeutralizeCell_LeavesNumbersAlone_AndNeutralizesStrings() + { + Assert.Equal(-5, SpreadsheetText.NeutralizeCell(-5)); + Assert.Equal(-1.5, SpreadsheetText.NeutralizeCell(-1.5)); + Assert.Null(SpreadsheetText.NeutralizeCell(null)); + Assert.Equal("'=1+1", SpreadsheetText.NeutralizeCell("=1+1")); + } + + [Fact] + public void FormatForExport_WritesTextNeutralized_AndNumbersAsNumbers() + { + Assert.Equal("'=1+1", DataGridExport.FormatForExport("=1+1")); + Assert.Equal("-5", DataGridExport.FormatForExport(-5)); + Assert.Equal("-1.5", DataGridExport.FormatForExport(-1.5)); + } + + [Fact] + public void GridCsv_WritesTheTextCellNeutralized_AndTheNegativeNumberAsANumber() + { + var csv = RunSta(() => DataGridExport.BuildCsv(BuildGrid(), ",").Replace("\r\n", "\n")); + + Assert.Equal("'=1+1,-5", csv.TrimEnd('\n').Split('\n')[1]); + } + + [Fact] + public void GridCopy_WritesTheTextCellNeutralized_AndTheNegativeNumberAsANumber() + { + var text = RunSta(() => DataGridExport.BuildClipboardText(BuildGrid()).Replace("\r\n", "\n")); + + Assert.Equal("'=1+1\t-5", text.TrimEnd('\n').Split('\n')[1]); + } + + [Fact] + public void LiteContextMenuCells_ReadThroughTheSameRule() + { + var (name, value) = RunSta(() => + { + var grid = BuildGrid(); + var item = grid.Items.Cast().First(); + return (ContextMenuHelper.GetCellValue(grid.Columns[0], item), + ContextMenuHelper.GetCellValue(grid.Columns[1], item)); + }); + + Assert.Equal("'=1+1", name); + Assert.Equal("-5", value); + } + + [Fact] + public void KeyboardCopy_NeutralizesTextCells_AndLeavesNumbersAndHeadersAlone() + { + var (headerText, dataName, dataValue) = RunSta(() => + { + var grid = BuildGrid(); + var item = grid.Items.Cast().First(); + var nameColumn = grid.Columns[0]; + var valueColumn = grid.Columns[1]; + + var header = new DataGridRowClipboardEventArgs(item, 0, 1, isColumnHeadersRow: true); + header.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, nameColumn, "=Header")); + DataGridClipboardBehavior.FixHeaderCopy(grid, header); + + var data = new DataGridRowClipboardEventArgs(item, 0, 1, isColumnHeadersRow: false); + data.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, nameColumn, "=1+1")); + data.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, valueColumn, -5)); + DataGridClipboardBehavior.FixHeaderCopy(grid, data); + + return (header.ClipboardRowContent[0].Content, data.ClipboardRowContent[0].Content, + data.ClipboardRowContent[1].Content); + }); + + Assert.Equal("=Header", headerText); + Assert.Equal("'=1+1", dataName); + Assert.Equal(-5, dataValue); + } + + [Theory] + [InlineData("--x")] + [InlineData("=1+1")] + [InlineData("@SUM(A1)")] + [InlineData("-5 rows")] + public void CopyCell_CopiesTheTextExactlyAsItIs(string text) + { + var (name, value) = RunSta(() => + { + var grid = BuildGridWithTexts(text); + var item = grid.Items.Cast().First(); + return (DataGridExport.GetRawCellValue(grid.Columns[0], item), + DataGridExport.GetRawCellValue(grid.Columns[1], item)); + }); + + Assert.Equal(text, name); + Assert.Equal("-1", value); + } + + [Fact] + public void CopyCell_ReadsTheCellRaw_InBothImplementations() + { + // Copy Cell has no pure builder (it reads the grid's current cell and writes the clipboard), so the wiring + // is pinned: both CopyCell bodies read through GetRawCellValue and never through the neutralizing reader. + foreach (var (file, next) in new[] + { + ("PerformanceMonitor.Ui/DataGridExport.cs", "public static void CopyRow("), + ("Lite/Helpers/ContextMenuHelper.cs", "public static void CopyRow("), + }) + { + var source = ParitySource.ReadFile(file); + var start = source.IndexOf("public static void CopyCell(", StringComparison.Ordinal); + var end = source.IndexOf(next, StringComparison.Ordinal); + Assert.True(start >= 0 && end > start, $"the read of CopyCell in {file} is broken."); + + var body = source.Substring(start, end - start); + Assert.Contains("GetRawCellValue(", body); + Assert.DoesNotContain("GetCellValue(", body); + } + } + + [Fact] + public void MultiRowCopy_StillNeutralizesTextThatStartsWithDashes() + { + var (allRows, rowValues, csv) = RunSta(() => + { + var grid = BuildGridWithTexts("--x", "=1+1"); + var first = grid.Items.Cast().First(); + return (DataGridExport.BuildClipboardText(grid).Replace("\r\n", "\n"), + DataGridExport.GetRowValues(grid, first), + DataGridExport.BuildCsv(grid, ",").Replace("\r\n", "\n")); + }); + + // Copy All Rows: header, then one line per row. + Assert.Equal(new[] { "Name\tValue", "'--x\t-1", "'=1+1\t-2" }, allRows.TrimEnd('\n').Split('\n')); + // Copy Row is the tab-joined GetRowValues of one row. + Assert.Equal(new[] { "'--x", "-1" }, rowValues); + Assert.Equal(new[] { "Name,Value", "'--x,-1", "'=1+1,-2" }, csv.TrimEnd('\n').Split('\n')); + } + + [Fact] + public void LiteChartCsv_NeutralizesTheSeriesName() + { + var line = ContextMenuHelper.ChartCsvLine(46000.5, "=1+1", 3, ",", TimeZoneInfo.Utc); + + Assert.Contains(",'=1+1,", line); + } + + [Theory] + [InlineData("a\t=1", "a =1")] + [InlineData("a\n=1", "a =1")] + [InlineData("a\r\n+1", "a +1")] + [InlineData("a\r\n\r\n-1", "a -1")] + [InlineData("a\t\t@SUM(1)", "a @SUM(1)")] + [InlineData("\t=1", "' =1")] + [InlineData("\r\n=1", "' =1")] + [InlineData("\r@SUM(1)", "' @SUM(1)")] + [InlineData("\t@SUM(1)", "' @SUM(1)")] + [InlineData(" =1", "' =1")] + [InlineData(" \t=SUM(1)", "' =SUM(1)")] + [InlineData("\n\n+1+1", "' +1+1")] + [InlineData("\r\n\t -1+1", "' -1+1")] + [InlineData(" -5", " -5")] + [InlineData("\t-5", " -5")] + [InlineData("\r\n-1,234.5", " -1,234.5")] + [InlineData(" ", " ")] + [InlineData("=1\n=2", "'=1 =2")] + [InlineData("plain", "plain")] + public void ClipboardCell_CollapsesTabsAndLineBreaksToOneSpace_BeforeTheFormulaRule(string text, string expected) + { + Assert.Equal(expected, SpreadsheetText.NeutralizeClipboardCell(text)); + } + + [Theory] + [InlineData("SELECT 1 /*\n=HYPERLINK(\"https://x/\",\"open\")\n*/")] + [InlineData("x\r\n=1+1")] + [InlineData("x\r\n+1")] + [InlineData("x\r\n-1")] + [InlineData("x\r\n@SUM(1)")] + [InlineData("x\t=1+1")] + [InlineData("x\t+1")] + [InlineData("x\t-1")] + [InlineData("x\t@SUM(1)")] + [InlineData("x\r=1+1")] + [InlineData("\t=1+1")] + [InlineData("\r\n@SUM(1)")] + [InlineData(" =1+1")] + [InlineData(" @SUM(1)")] + [InlineData("\r@SUM(1)")] + [InlineData("\t\r\n -1+1")] + [InlineData("\n\t =HYPERLINK(1)")] + public void RowCopies_KeepOneCellOneField_AndNoFieldStartsLikeAFormula(string text) + { + var (copyAll, rowValues) = RunSta(() => + { + var grid = BuildGridWithTexts(text); + var first = grid.Items.Cast().First(); + return (DataGridExport.BuildClipboardText(grid), DataGridExport.GetClipboardRowValues(grid, first)); + }); + + // Copy All Rows: header line plus one line, two fields each. A tab or line break inside the cell would add fields or lines. + var lines = copyAll.TrimEnd('\r', '\n').Split("\r\n"); + Assert.Equal(2, lines.Length); + Assert.All(lines, l => Assert.Equal(2, l.Split('\t').Length)); + AssertNoFormulaField(copyAll.Split('\t', '\r', '\n')); + + // Copy Row: the same two fields. + Assert.Equal(2, rowValues.Count); + AssertNoFormulaField(rowValues); + Assert.Equal(rowValues[0], lines[1].Split('\t')[0]); + } + + private static void AssertNoFormulaField(System.Collections.Generic.IEnumerable fields) + { + foreach (var field in fields) + { + // Leading white space is ignored by a spreadsheet, so the first character that is not white space decides. + if (field.Length == 0 || field[0] == '\'') continue; + var lead = field.TrimStart(); + if (lead.Length == 0 || int.TryParse(lead, out _)) continue; + Assert.DoesNotContain(lead[0], "=+-@"); + } + } + + [Fact] + public void ClipboardCell_LeavesNumbersAndTheCsvExportAlone() + { + Assert.Equal("-5", DataGridExport.FormatForClipboard(-5)); + Assert.Equal("-1,234.5", SpreadsheetText.NeutralizeClipboardCell("-1,234.5")); + + // The CSV export keeps the line break inside a quoted field and still neutralizes the start. + var csv = RunSta(() => DataGridExport.BuildCsv(BuildGridWithTexts("a\n=1", "=1\r\nb"), ",").Replace("\r\n", "\n")); + Assert.Equal("Name,Value\n\"a\n=1\",-1\n\"'=1\nb\",-2\n", csv); + } + + [Fact] + public void KeyboardCopy_CollapsesTabsAndLineBreaksInsideATextCell() + { + var content = RunSta(() => + { + var grid = BuildGrid(); + var item = grid.Items.Cast().First(); + var data = new DataGridRowClipboardEventArgs(item, 0, 1, isColumnHeadersRow: false); + data.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, grid.Columns[0], "x\n=1+1")); + data.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, grid.Columns[0], "x\t+1")); + data.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, grid.Columns[0], "\r\n@SUM(1)")); + data.ClipboardRowContent.Add(new DataGridClipboardCellContent(item, grid.Columns[1], -5)); + DataGridClipboardBehavior.FixHeaderCopy(grid, data); + return data.ClipboardRowContent.Select(c => c.Content).ToArray(); + }); + + Assert.Equal(new object?[] { "x =1+1", "x +1", "' @SUM(1)", -5 }, content); + } + + [Fact] + public void RowCopies_ReadCellsThroughTheClipboardReader_InBothImplementations() + { + var ui = ParitySource.ReadFile("PerformanceMonitor.Ui/DataGridExport.cs"); + var copyRow = ui.Substring(ui.IndexOf("public static void CopyRow(", StringComparison.Ordinal)); + copyRow = copyRow.Substring(0, copyRow.IndexOf("public static void CopyAllRows(", StringComparison.Ordinal)); + Assert.Contains("GetClipboardRowValues(", copyRow); + Assert.DoesNotContain("GetRowValues(", copyRow); + + var build = ui.Substring(ui.IndexOf("public static string BuildClipboardText(", StringComparison.Ordinal)); + build = build.Substring(0, build.IndexOf("public static string BuildCsv(", StringComparison.Ordinal)); + Assert.Contains("GetClipboardRowValues(", build); + Assert.DoesNotContain("GetRowValues(", build); + + var lite = ParitySource.ReadFile("Lite/Helpers/ContextMenuHelper.cs"); + var rows = lite.Substring(lite.IndexOf("public static void CopyRow(", StringComparison.Ordinal)); + rows = rows.Substring(0, rows.IndexOf("public static void ExportToCsv(", StringComparison.Ordinal)); + Assert.Equal(2, rows.Split("GetClipboardCellValue(").Length - 1); + Assert.DoesNotContain("GetCellValue(", rows); + } + + [Theory] + [InlineData("a,b", ";", "\"a,b\"")] + [InlineData("a;b", ",", "\"a;b\"")] + [InlineData("a\tb", ",", "\"a\tb\"")] + [InlineData("a|b", "|", "\"a|b\"")] + [InlineData("a\"b", ";", "\"a\"\"b\"")] + [InlineData("a\nb", ";", "\"a\nb\"")] + [InlineData("a\rb", "\t", "\"a\rb\"")] + [InlineData("plain", ";", "plain")] + [InlineData("", ";", "")] + public void CsvEscape_QuotesTheSeparatorQuoteBreakAndTheCommonDelimiters_WhateverTheSeparatorIs(string value, string separator, string expected) + { + Assert.Equal(expected, DataGridExport.CsvEscape(value, separator)); + } +} diff --git a/Lite/App.xaml.cs b/Lite/App.xaml.cs index 51c48a98c5..428f32c551 100644 --- a/Lite/App.xaml.cs +++ b/Lite/App.xaml.cs @@ -566,6 +566,9 @@ land in the file a few statements later. */ LoadDuckDbMemoryLimit(); LoadDefaultTimeRange(); LoadAlertSettings(); + /* An SMTP setup saved before the settings window warned about it is named in the log when it loads. */ + if (LoadedSmtpCleartextWarning(GetSmtpPassword) is { } smtpWarning) + AppLogger.Warn("SMTP", smtpWarning); /* #3577: the operator's per-theme color overrides live beside settings.json, in the same per-user config directory, and are read by ThemeManager on every Apply — so the path and the log hooks @@ -1184,6 +1187,19 @@ private static void LoadDuckDbMemoryLimit() public static void LoadAlertSettings() => LoadAlertSettings(ConfigDirectory, GetWebhookUrl, SaveWebhookUrl); + /// + /// The settings window's SMTP warning, for the settings just loaded: non-null when SMTP is on, SSL is off and a + /// username and password are set, so the login would cross the network readable. The password is read only + /// when the earlier conditions hold, through so a test needs no credential store. + /// + internal static string? LoadedSmtpCleartextWarning(Func readPassword) + { + if (!SmtpEnabled || SmtpUseSsl || string.IsNullOrWhiteSpace(SmtpUsername)) + return null; + + return EmailSendCore.CleartextCredentialsWarning(SmtpUseSsl, SmtpUsername, !string.IsNullOrEmpty(readPassword())); + } + /// /// Path- and secret-store-injected form, so the "secrets load even without settings.json" contract is /// testable without touching the process-wide config directory or the real Credential Manager. diff --git a/Lite/Controls/ServerTab.xaml.cs b/Lite/Controls/ServerTab.xaml.cs index c853e87ed8..d759adcfb1 100644 --- a/Lite/Controls/ServerTab.xaml.cs +++ b/Lite/Controls/ServerTab.xaml.cs @@ -496,7 +496,7 @@ e.OriginalSource is not System.Windows.Controls.TextBox && try { var (ok, xml) = await ClipboardText.TryReadAsync(); - if (ok && !string.IsNullOrWhiteSpace(xml)) + if (ok && !string.IsNullOrWhiteSpace(xml) && !CallerPlanXmlPrompt.Refuse(xml)) { // Await so the guard spans the off-thread parse, not just the clipboard read (#2870). await OpenPlanTab(xml, "Pasted Plan"); diff --git a/Lite/Helpers/ContextMenuHelper.cs b/Lite/Helpers/ContextMenuHelper.cs index 08191388a5..b7b4cc61cf 100644 --- a/Lite/Helpers/ContextMenuHelper.cs +++ b/Lite/Helpers/ContextMenuHelper.cs @@ -56,12 +56,15 @@ public static string GetCellValue(DataGridColumn col, object item) return ""; } + /* One cell is copied exactly as it is: it is often query text on its way into SSMS, where a leading + apostrophe would break it. Copy Row and Copy All Rows also turn a tab or line break inside a cell into a space and neutralize; the CSV + export quotes such a cell and neutralizes. */ public static void CopyCell(object sender) { var grid = FindParentDataGrid(sender); if (grid?.CurrentCell.Column == null || grid.CurrentItem == null) return; - var value = GetCellValue(grid.CurrentCell.Column, grid.CurrentItem); + var value = DataGridExport.GetRawCellValue(grid.CurrentCell.Column, grid.CurrentItem); if (value.Length > 0) ClipboardText.TrySetDataObject(value); } @@ -73,7 +76,7 @@ public static void CopyRow(object sender) var sb = new StringBuilder(); foreach (var col in grid.Columns) { - sb.Append(GetCellValue(col, grid.CurrentItem)); + sb.Append(DataGridExport.GetClipboardCellValue(col, grid.CurrentItem)); sb.Append('\t'); } ClipboardText.TrySetDataObject(sb.ToString().TrimEnd('\t')); @@ -97,7 +100,7 @@ public static void CopyAllRows(object sender) { foreach (var col in grid.Columns) { - sb.Append(GetCellValue(col, item)); + sb.Append(DataGridExport.GetClipboardCellValue(col, item)); sb.Append('\t'); } sb.AppendLine(); @@ -150,22 +153,12 @@ public static void ExportToCsv(object sender, string defaultFilePrefix) } } - private static string FormatForExport(object? value) - { - if (value == null) return ""; - if (value is IFormattable formattable) - return formattable.ToString(null, CultureInfo.InvariantCulture); - return value.ToString() ?? ""; - } + /* One formatter and one quoting rule for every grid copy and CSV export. The shared DataGridExport + versions neutralize text that starts with a formula character (SpreadsheetText), so a cell of collected + text is not evaluated when the copy or the CSV is opened in a spreadsheet. */ + private static string FormatForExport(object? value) => DataGridExport.FormatForExport(value); - private static string CsvEscape(string value, string separator) - { - if (value.Contains(separator, StringComparison.Ordinal) || value.Contains('"') || value.Contains('\n') || value.Contains('\r')) - { - return "\"" + value.Replace("\"", "\"\"") + "\""; - } - return value; - } + private static string CsvEscape(string value, string separator) => DataGridExport.CsvEscape(value, separator); /// The time text of a chart CSV export line, the format the export has always used. private const string ChartCsvTimeFormat = "yyyy-MM-dd HH:mm:ss"; @@ -202,7 +195,7 @@ internal static string ChartCsvLine(double x, string seriesName, double y, strin return string.Join(separator, new[] { shown.ToString(ChartCsvTimeFormat, CultureInfo.InvariantCulture), - CsvEscape(seriesName, separator), + CsvEscape(SpreadsheetText.Neutralize(seriesName), separator), y.ToString(CultureInfo.InvariantCulture), DisplayZone.UtcOffsetText(plotted, displayZone) }); diff --git a/Lite/Mcp/McpPlanTools.cs b/Lite/Mcp/McpPlanTools.cs index 4a2e3cbde6..a33ab38624 100644 --- a/Lite/Mcp/McpPlanTools.cs +++ b/Lite/Mcp/McpPlanTools.cs @@ -252,6 +252,11 @@ public static string AnalyzePlanXml( if (string.IsNullOrWhiteSpace(plan_xml)) return McpHelpers.Refusal("plan_xml", "No plan XML provided."); + // The caller wrote this XML, so bound it before it reaches the parser (the stored-plan tools skip this). + var tooLarge = CallerPlanXml.RefusalReason(plan_xml); + if (tooLarge != null) + return McpHelpers.Refusal("plan_xml", tooLarge); + try { // #4348: the caller's XML has no stored-plan read to filter it, so AnalyzeFilteredPlan runs the statement diff --git a/Lite/Windows/SettingsWindow.xaml.cs b/Lite/Windows/SettingsWindow.xaml.cs index d85cd11efd..56df931422 100644 --- a/Lite/Windows/SettingsWindow.xaml.cs +++ b/Lite/Windows/SettingsWindow.xaml.cs @@ -271,6 +271,14 @@ private void PauseResumeButton_Click(object sender, RoutedEventArgs e) /// private async void SaveButton_Click(object sender, RoutedEventArgs e) { + /* An SMTP login that would go to the mail server unencrypted is confirmed before anything is applied: + No cancels the whole save and leaves the window open to fix it. Not blocked - an internal relay on + port 25 is a real setup. */ + if (!ConfirmCleartextSmtp("Save")) + { + return; + } + JsonNode root; bool mcpChanged, mcpValid, alertsValid, webhooksValid, memoryLimitValid; @@ -1183,7 +1191,8 @@ private void ValidateSmtpButton_Click(object sender, RoutedEventArgs e) if (errors.Count == 0) { - SmtpStatusText.Text = "Settings look good. Use 'Send Test Email' to verify delivery."; + SmtpStatusText.Text = CleartextSmtpWarning() + ?? "Settings look good. Use 'Send Test Email' to verify delivery."; } else { @@ -1194,8 +1203,50 @@ private void ValidateSmtpButton_Click(object sender, RoutedEventArgs e) } } + /// + /// Whether a password will be sent with the login: the one typed here, or the one already in the + /// credential store (Save never clears the stored password when the box is empty). + /// + private bool SmtpPasswordIsSet() => + !string.IsNullOrEmpty(SmtpPasswordBox.Password) || !string.IsNullOrEmpty(App.GetSmtpPassword()); + + /// The unencrypted-login warning for the SMTP settings as they stand in the boxes, or null. + private string? CleartextSmtpWarning() => + EmailSendCore.CleartextCredentialsWarning( + SmtpSslCheckBox.IsChecked == true, SmtpUsernameBox.Text?.Trim(), SmtpPasswordIsSet()); + + /// + /// Non-blocking confirm shown at Save and Send Test when SSL is off and a login is set, so the username + /// and password would go to the mail server unencrypted. Yes proceeds; No cancels the action. + /// is the verb ("Save" / "Send"). Not blocked outright: an internal relay on + /// port 25 is a real setup. + /// + private bool ConfirmCleartextSmtp(string action) + { + if (SmtpEnabledCheckBox.IsChecked != true) + { + return true; + } + + var warning = CleartextSmtpWarning(); + if (warning == null) + { + return true; + } + + var result = MessageBox.Show( + $"{warning}\n\n{action} anyway?", + "Unencrypted SMTP Login", MessageBoxButton.YesNo, MessageBoxImage.Warning); + return result == MessageBoxResult.Yes; + } + private async void TestEmailButton_Click(object sender, RoutedEventArgs e) { + if (!ConfirmCleartextSmtp("Send")) + { + return; + } + /* Save current App values so we can restore after the test */ var origServer = App.SmtpServer; var origPort = App.SmtpPort; diff --git a/PerformanceMonitor.Notifications/EmailSendCore.cs b/PerformanceMonitor.Notifications/EmailSendCore.cs index 92dfa1cc4b..45beead6bf 100644 --- a/PerformanceMonitor.Notifications/EmailSendCore.cs +++ b/PerformanceMonitor.Notifications/EmailSendCore.cs @@ -353,6 +353,28 @@ fold or a muted alert attempted nothing and the record reads a channel with no e } } + /// + /// The warning a settings window shows at Save, Test and Validate when these SMTP settings would send the + /// login to the mail server without encryption, or null when they would not. With SSL off, + /// ignores a STARTTLS offer and sends AUTH in the clear, so the username and + /// password cross the network readable by anyone on the path. The send attaches credentials only when a + /// username is set (see ), so a password with no username is not a risk. + /// A warning, not a block: an internal relay on port 25 is a real setup, and the send still goes out. + /// + /// Whether a password will be sent: typed in the settings window or already + /// stored. + public static string? CleartextCredentialsWarning(bool useSsl, string? username, bool passwordSet) + { + if (useSsl || string.IsNullOrWhiteSpace(username) || !passwordSet) + { + return null; + } + + return "SSL is off, so the SMTP username and password are sent to the mail server without " + + "encryption. Anyone on the network path can read them. Turn SSL on, or use this only " + + "on a network you trust."; + } + /// /// Shared SMTP send helper with multipart/alternative (HTML + plain text) and an optional /// XML attachment (deadlock graph / blocked process report). diff --git a/PerformanceMonitor.PlanAnalysis/CallerPlanXml.cs b/PerformanceMonitor.PlanAnalysis/CallerPlanXml.cs new file mode 100644 index 0000000000..5974141b40 --- /dev/null +++ b/PerformanceMonitor.PlanAnalysis/CallerPlanXml.cs @@ -0,0 +1,82 @@ +using System; + +namespace PerformanceMonitor.PlanAnalysis; + +/// +/// The limits on plan XML that a person or an MCP client supplies: a file opened or dropped in a viewer, +/// pasted text, or the plan_xml argument of analyze_plan_xml. The check is one pass over the +/// characters and runs BEFORE any parse, so a refused input costs milliseconds instead of the parse. +/// It is deliberately not part of or the viewer's LoadPlan: +/// plans the collectors stored go through both, and those keep their own limits (16M characters, depth +/// 1,000). Only the entry points that take text from a caller call this. +/// +public static class CallerPlanXml +{ + /// + /// The most attributes one element may carry. Measured on the shipped parser (Release build, this + /// machine): the cost of one element grows with the SQUARE of its attribute count, 0.27 s at 100,000 + /// attributes, 0.9 s at 200,000, 6.3 s at 400,000 and 27 s at 1,000,000 (11.9M characters, inside the 16M + /// cap). The same attributes spread over many elements cost almost nothing: 1,000,000 elements with one + /// attribute each parse in 0.8 s, and 15M characters of elements with 10,000 attributes each in 0.5 s. + /// Size alone is linear, so the 16M-character cap stays as it is. The widest element in the largest + /// real plans of the test fixtures has 20 attributes, so 1,000 leaves 50x of room; at 1,000 attributes per + /// element the worst 16M-character input parses in about half a second. + /// + internal const int MaxAttributesPerElement = 1_000; + + /// + /// Returns why is refused, or null when it may go on to the parser. The sentence + /// names the limit, so it can go to the caller as it is (an MCP refusal or a viewer dialog). + /// + public static string? RefusalReason(string? xml) + { + if (string.IsNullOrEmpty(xml)) return null; + + // Showplan XML never has a DTD. Refusing one closes entity expansion (up to 10M characters from + // a few hundred) without asking anyone to hand-craft a plan that needs it. + if (xml.Contains("' that are OUTSIDE a quoted + // value. A '>' or an '=' inside a value is text, not markup: a predicate such as [a]=[b] holds + // '=' signs, and a hostile tag can hide behind a '>' in a value (a plain '<'..'>' count resets + // there, so 1,000,000 attributes each holding a '>' would count as one). '<' always starts a new + // tag and clears the quote, because an attribute value cannot hold '<' and the parser rejects one + // that does, so a stray quote in a comment or CDATA section cannot carry over into the next tag. + var inTag = false; + var quote = '\0'; + var attributes = 0; + foreach (var c in xml) + { + if (c == '<') + { + inTag = true; + quote = '\0'; + attributes = 0; + } + else if (!inTag) + { + continue; + } + else if (quote != '\0') + { + if (c == quote) quote = '\0'; + } + else if (c == '"' || c == '\'') + { + quote = c; + } + else if (c == '=') + { + if (++attributes > MaxAttributesPerElement) + return $"An element in the plan XML has more than {MaxAttributesPerElement:N0} attributes. A real showplan element has fewer than 50, and the parser slows down sharply as one element grows past that, so the plan is not read."; + } + else if (c == '>') + { + inTag = false; + } + } + + return null; + } +} diff --git a/PerformanceMonitor.PlanAnalysis/ParameterCard.cs b/PerformanceMonitor.PlanAnalysis/ParameterCard.cs index 8d1f0bcd2c..4c7d58cee5 100644 --- a/PerformanceMonitor.PlanAnalysis/ParameterCard.cs +++ b/PerformanceMonitor.PlanAnalysis/ParameterCard.cs @@ -103,7 +103,7 @@ public static List Annotations( // comment is not a hint, so the OPTIMIZE FOR UNKNOWN check runs on the masked text, not // the raw statement text. var hasOptimizeForUnknown = statementText.Contains("OPTIMIZE", StringComparison.OrdinalIgnoreCase) - && OptimizeForUnknownRegExp().IsMatch(maskedStatementText); + && PlanRegex.IsMatch(OptimizeForUnknownRegExp(), maskedStatementText, "OPTIMIZE FOR UNKNOWN in the parameter card"); annotations.Add(hasOptimizeForUnknown ? new ParameterCardAnnotation( @@ -146,7 +146,8 @@ public static List FindUnresolvedVariables( if (rootNode != null) CollectTableVariableNames(rootNode, tableVarNames); - var matches = AtVariableRegExp().Matches(queryText); + // A pass cut short by a timeout lists the variables found so far. + var matches = PlanRegex.Matches(AtVariableRegExp(), queryText, "unresolved variables in the parameter card"); var seenVars = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (Match match in matches) @@ -179,10 +180,10 @@ private static void CollectTableVariableNames(PlanNode node, HashSet nam CollectTableVariableNames(child, names); } - [GeneratedRegex(@"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex OptimizeForUnknownRegExp(); - [GeneratedRegex(@"@\w+", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"@\w+", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex AtVariableRegExp(); } diff --git a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs index 06f9a7e233..815640419f 100644 --- a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs +++ b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs @@ -107,10 +107,22 @@ public static void Analyze( ParsedPlan plan, AnalyzerConfig? config, ServerMetadata? serverMetadata, + CancellationToken cancellationToken) => + Analyze(plan, config, serverMetadata, TimeSpan.FromMilliseconds(PlanRegex.PlanBudgetMilliseconds), cancellationToken); + + /// The same analysis with an explicit regex time budget for the whole plan; tests pass a tiny one. + internal static void Analyze( + ParsedPlan plan, + AnalyzerConfig? config, + ServerMetadata? serverMetadata, + TimeSpan regexBudget, CancellationToken cancellationToken) { var cfg = config ?? AnalyzerConfig.Default; + // One regex time budget for the whole plan; a check that is cut short is named in a note on its statement. + using var regexScope = PlanRegex.BeginPlan(regexBudget); + /* #4514: every statement, including the ones inside a stored procedure or UDF body. This used to walk batch.Statements alone, so an EXEC plan analyzed as a single statement with nothing to say about the statements actually doing the work. @@ -127,6 +139,7 @@ running the analyzer to completion against a plan nobody is waiting for. */ cancellationToken.ThrowIfCancellationRequested(); MarkLegacyWarnings(stmt); + NoteSkippedChecks(stmt, cfg); } cancellationToken.ThrowIfCancellationRequested(); @@ -134,6 +147,30 @@ running the analyzer to completion against a plan nobody is waiting for. */ ApplySeverityOverrides(plan, cfg); } + /// + /// Adds one statement-level note naming the regex checks that were cut short on this statement, by a match + /// timeout or because the plan's regex time budget was spent. Without it, a skipped check reads as a clean + /// result. It is rule 40, so every analyzer warning keeps carrying a rule number. + /// + private static void NoteSkippedChecks(PlanStatement stmt, AnalyzerConfig cfg) + { + // The record is drained even when rule 40 is disabled, so it cannot carry into the next statement. + var skipped = PlanRegex.TakeSkipped(); + if (skipped.Count == 0 || cfg.IsRuleDisabled(40)) + return; + + stmt.PlanWarnings.Add(new PlanWarning + { + RuleNumber = 40, + WarningType = SkippedChecksWarningType, + Message = PlanRegex.SkippedNote(skipped), + Severity = PlanWarningSeverity.Info + }); + } + + /// The of the note that names the regex checks a statement skipped. + public const string SkippedChecksWarningType = "Analysis Checks Skipped"; + /// /// #4535, mirroring erikdarlingdata/PerformanceStudio dev (85492a1) commit dcc06db /// src/PlanViewer.Core/Services/PlanAnalyzer.Helpers.cs: walks @@ -323,7 +360,10 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg, Ser if (stmt.NonParallelPlanReason == "MaxDOPSetToOne") { var text = MaskCommentsAndLiterals(stmt.StatementText); // #4524 - var hasMaxdop1InText = Regex.IsMatch(text, @"MAXDOP\s+1\b", RegexOptions.IgnoreCase); + // A regex that cannot finish hands the question to a plain text search, so the Serial Plan warning is + // kept or dropped on what the text says, whatever the time limit. + var hasMaxdop1InText = PlanRegex.IsMatch(text, @"MAXDOP\s+1\b", RegexOptions.IgnoreCase, + "MAXDOP 1 hint in the statement text", t => PlanTextSearch.ContainsWords(t, false, true, "MAXDOP", "1")); var isTruncated = stmt.IsTextTruncated; if (hasMaxdop1InText) @@ -506,7 +546,9 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg, Ser // Rule 27: OPTIMIZE FOR UNKNOWN in statement text if (!cfg.IsRuleDisabled(27) && !string.IsNullOrEmpty(stmt.StatementText) && - OptimizeForUnknownRegExp().IsMatch(MaskCommentsAndLiterals(stmt.StatementText))) // #4524 + PlanRegex.IsMatch(OptimizeForUnknownRegExp(), MaskCommentsAndLiterals(stmt.StatementText), // #4524 + "OPTIMIZE FOR UNKNOWN hint", // a regex that cannot finish gives way to a text search: the warning names a hint the text must hold + t => PlanTextSearch.ContainsWords(t, false, false, "OPTIMIZE", "FOR", "UNKNOWN"))) { stmt.PlanWarnings.Add(new PlanWarning { @@ -653,14 +695,18 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg, Ser // qualifier must be looked for between CURSOR and the FOR that introduces // the SELECT. Capturing tokens *before* CURSOR never sees LOCAL and would // fire on every cursor, including ones already declared LOCAL. - var cursorDeclMatch = Regex.Match( + // A declaration that cannot be read within the limit is recorded as a skipped check: with no + // declaration found there is nothing to say about its qualifiers. + var cursorDeclMatch = PlanRegex.Match( maskedText, @"\bDECLARE\s+\w+\s+(?:INSENSITIVE\s+|SCROLL\s+)*CURSOR\b(.*?)\bFOR\b", - RegexOptions.IgnoreCase | RegexOptions.Singleline); + RegexOptions.IgnoreCase | RegexOptions.Singleline, "cursor declaration", out _); if (cursorDeclMatch.Success) { var qualifiers = cursorDeclMatch.Groups[1].Value; - if (!Regex.IsMatch(qualifiers, @"\bLOCAL\b", RegexOptions.IgnoreCase)) + // A regex that cannot finish gives way to a text search: the warning says LOCAL is missing. + if (!PlanRegex.IsMatch(qualifiers, @"\bLOCAL\b", RegexOptions.IgnoreCase, "cursor LOCAL qualifier", + t => PlanTextSearch.ContainsWords(t, true, true, "LOCAL"))) { stmt.PlanWarnings.Add(new PlanWarning { @@ -682,7 +728,9 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg, Ser // Suppress when the user explicitly set MAXDOP 2 as a query hint — the DOP // cap is intentional, not the Standard Edition batch-mode limitation. var hasMaxdop2Hint = !string.IsNullOrEmpty(stmt.StatementText) - && Regex.IsMatch(MaskCommentsAndLiterals(stmt.StatementText), @"MAXDOP\s+2\b", RegexOptions.IgnoreCase); // #4524 + && PlanRegex.IsMatch(MaskCommentsAndLiterals(stmt.StatementText), @"MAXDOP\s+2\b", RegexOptions.IgnoreCase, + "MAXDOP 2 hint in the statement text", // #4524; a regex that cannot finish gives way to a text search + t => PlanTextSearch.ContainsWords(t, false, true, "MAXDOP", "2")); if (!hasMaxdop2Hint) { @@ -1083,6 +1131,8 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi "Leading wildcard LIKE prevents an index seek — SQL Server must scan every row. If substring search performance is critical, consider a full-text index or a trigram-based approach.", "CASE expression in predicate" => "CASE expression in a predicate prevents an index seek. Rewrite using separate WHERE clauses combined with OR, or split into multiple queries.", + UnverifiedNonSargableReason => + "The predicate could not be fully checked for patterns that prevent an index seek within the analysis time limit (a very long or complex predicate), so it may be non-SARGable. Review it by hand.", _ when nonSargableReason.StartsWith("Function call", StringComparison.OrdinalIgnoreCase) => $"{nonSargableReason} prevents an index seek. Remove the function from the column side — apply it to the parameter instead, or create a computed column with the expression and index that.", _ => @@ -1632,7 +1682,9 @@ private static bool HasNotInPattern(PlanNode spoolNode, PlanStatement stmt) { // Check statement text for NOT IN if (string.IsNullOrEmpty(stmt.StatementText) || - !NotInRegExp().IsMatch(MaskCommentsAndLiterals(stmt.StatementText))) // #4524 + !PlanRegex.IsMatch(NotInRegExp(), MaskCommentsAndLiterals(stmt.StatementText), // #4524 + "NOT IN in the statement text", // a regex that cannot finish gives way to a text search + t => PlanTextSearch.ContainsWords(t, true, true, "NOT", "IN"))) return false; // Walk up the tree checking ancestors and their children @@ -1683,11 +1735,17 @@ private static bool IsRowstoreScan(PlanNode node) private static bool IsProbeOnly(string predicate) { // Strip all PROBE(...) expressions — PROBE args can contain nested parens - var stripped = Regex.Replace(predicate, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", - RegexOptions.IgnoreCase).Trim(); + // A step that cannot finish gives way to a text search that decides the same question. + var stripped = PlanRegex.Replace(predicate, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", + RegexOptions.IgnoreCase, "PROBE-only predicate", out var probeDone, recordSkip: false).Trim(); + if (!probeDone) + return PlanTextSearch.IsProbeOnly(predicate); // Remove leftover AND/OR connectors and whitespace - stripped = Regex.Replace(stripped, @"\b(AND|OR)\b", "", RegexOptions.IgnoreCase).Trim(); + stripped = PlanRegex.Replace(stripped, @"\b(AND|OR)\b", "", RegexOptions.IgnoreCase, + "PROBE-only predicate", out var connectorsDone, recordSkip: false).Trim(); + if (!connectorsDone) + return PlanTextSearch.IsProbeOnly(predicate); // If nothing meaningful remains, it was PROBE-only return stripped.Length == 0; @@ -1699,12 +1757,13 @@ private static bool IsProbeOnly(string predicate) /// private static string StripProbeExpressions(string predicate) { - var stripped = Regex.Replace(predicate, @"\s*AND\s+PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", - RegexOptions.IgnoreCase); - stripped = Regex.Replace(stripped, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)\s*AND\s+", "", - RegexOptions.IgnoreCase); - stripped = Regex.Replace(stripped, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", - RegexOptions.IgnoreCase); + // Display only: a step that cannot finish leaves the PROBE text in the predicate shown. + var stripped = PlanRegex.Replace(predicate, @"\s*AND\s+PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", + RegexOptions.IgnoreCase, "PROBE text in the displayed predicate", out _); + stripped = PlanRegex.Replace(stripped, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)\s*AND\s+", "", + RegexOptions.IgnoreCase, "PROBE text in the displayed predicate", out _); + stripped = PlanRegex.Replace(stripped, @"PROBE\s*\([^()]*(?:\([^()]*\)[^()]*)*\)", "", + RegexOptions.IgnoreCase, "PROBE text in the displayed predicate", out _); return stripped.Trim(); } @@ -1848,23 +1907,38 @@ private static HashSet CollectBareOuterReferences(PlanNode node) { // CASE expression in predicate — check first because CASE bodies // often contain CONVERT_IMPLICIT that isn't the root cause - if (CaseInPredicateRegex.IsMatch(predicate)) - return "CASE expression in predicate"; + // A check that is cut short (a match timeout or the plan's spent regex budget) must not read as + // "this pattern is absent": if nothing is found, the predicate is reported as not fully checked. + var incomplete = false; + + if (PlanRegex.TryIsMatch(CaseInPredicateRegex, predicate, "non-SARGable CASE expression", out var hasCase)) + { + if (hasCase) + return "CASE expression in predicate"; + } + else + { + incomplete = true; + } // CONVERT_IMPLICIT — most common non-SARGable pattern, but only when it converts the // COLUMN. Converting the parameter up to the column's type costs nothing. - if (ConvertImplicitWrapsColumn(predicate, identity)) + if (ConvertImplicitWrapsColumn(predicate, identity, out var convertIncomplete)) return "Implicit conversion (CONVERT_IMPLICIT)"; + incomplete |= convertIncomplete; // ISNULL / COALESCE wrapping column — on the column side only. ISNULL(@p, 0) on the // parameter side is a runtime constant and seeks fine; flagging it contradicted this // warning's own "wrapping a column" message. col = ISNULL(@p, col) is still caught, // because the column sits inside the function, on its side of the comparison. - foreach (Match isnullMatch in IsNullCoalesceRegex.Matches(predicate)) + var isnullRun = PlanRegex.Matches(IsNullCoalesceRegex, predicate, "non-SARGable ISNULL/COALESCE"); + foreach (Match isnullMatch in isnullRun) { - if (IsFunctionOnColumnSide(predicate, isnullMatch, identity)) + if (IsFunctionOnColumnSide(predicate, isnullMatch, identity, out var undecided)) return "ISNULL/COALESCE wrapping column"; + incomplete |= undecided; // a side that could not be read is not reported as a column } + incomplete |= isnullRun.Incomplete; // Common function calls on columns — but only if the function wraps a column, // not a parameter/variable. Split on comparison operators to check which side @@ -1872,20 +1946,39 @@ private static HashSet CollectBareOuterReferences(PlanNode node) // Every match, not just the first: a parameter-side CONVERT_IMPLICIT now falls through to // here, and it is skipped below. Taking only the first match would let a benign conversion // sitting to the left of a real function-on-column hide it. - foreach (Match funcMatch in FunctionInPredicateRegex.Matches(predicate)) + var funcRun = PlanRegex.Matches(FunctionInPredicateRegex, predicate, "non-SARGable function call"); + foreach (Match funcMatch in funcRun) { var funcName = funcMatch.Groups[1].Value.ToUpperInvariant(); - if (funcName != "CONVERT_IMPLICIT" && IsFunctionOnColumnSide(predicate, funcMatch, identity)) + if (funcName == "CONVERT_IMPLICIT") + continue; + + if (IsFunctionOnColumnSide(predicate, funcMatch, identity, out var undecided)) return $"Function call ({funcName}) on column"; + incomplete |= undecided; } + incomplete |= funcRun.Incomplete; // Leading wildcard LIKE - if (LeadingWildcardLikeRegex.IsMatch(predicate)) - return "Leading wildcard LIKE pattern"; + if (PlanRegex.TryIsMatch(LeadingWildcardLikeRegex, predicate, "non-SARGable leading wildcard LIKE", out var hasLeadingWildcard)) + { + if (hasLeadingWildcard) + return "Leading wildcard LIKE pattern"; + } + else + { + incomplete = true; + } - return null; + return incomplete ? UnverifiedNonSargableReason : null; } + /// + /// The reason gives when a check ran past the analysis time limit and + /// nothing else was found: the rule-12 warning stays, worded as unverified, instead of vanishing. + /// + internal const string UnverifiedNonSargableReason = "Predicate not fully checked within the analysis time limit"; + /// /// True when a lookup branch under an OR expansion's Concatenation builds its seek value from /// another input. A join OR does: in ON u.Id = p.OwnerUserId OR u.Id = p.LastEditorUserId the @@ -1918,7 +2011,8 @@ private static bool LookupReadsAnotherInput(PlanNode branch) /// internal static bool ReadsAnotherInput(string scalarString) { - foreach (Match match in BracketedNameRegex.Matches(scalarString)) + var names = PlanRegex.Matches(BracketedNameRegex, scalarString, "lookup branch column references"); + foreach (Match match in names) { if (!match.Groups["name"].Success || match.Groups["call"].Success) continue; // a string literal, or the name of a function @@ -1931,6 +2025,8 @@ internal static bool ReadsAnotherInput(string scalarString) return true; } + // A pass cut short proved nothing about the rest of the text, so it proves no other input: the check is + // recorded as skipped and the warning is not raised on it. return false; } @@ -2683,14 +2779,14 @@ private static string IdentifyRowGoalCause(string stmtText) var text = MaskCommentsAndLiterals(stmtText).ToUpperInvariant(); var causes = new List(4); - if (Regex.IsMatch(text, @"\bTOP\b")) + if (PlanRegex.IsMatch(text, @"\bTOP\b", RegexOptions.None, "row goal cause: TOP")) causes.Add("TOP"); - if (Regex.IsMatch(text, @"\bEXISTS\b")) + if (PlanRegex.IsMatch(text, @"\bEXISTS\b", RegexOptions.None, "row goal cause: EXISTS")) causes.Add("EXISTS"); // IN with subquery — bare "IN (" followed by SELECT, not just "IN (1,2,3)" - if (Regex.IsMatch(text, @"\bIN\s*\(\s*SELECT\b")) + if (PlanRegex.IsMatch(text, @"\bIN\s*\(\s*SELECT\b", RegexOptions.None, "row goal cause: IN (subquery)")) causes.Add("IN (subquery)"); - if (Regex.IsMatch(text, @"\bFAST\b")) + if (PlanRegex.IsMatch(text, @"\bFAST\b", RegexOptions.None, "row goal cause: FAST hint")) causes.Add("FAST hint"); return causes.Count > 0 @@ -2774,13 +2870,23 @@ private static ScanImpact BuildScanImpactDetails(PlanNode node, PlanStatement st /// shape, e.g. abs([A]) where A is an outer reference and only X is this scan's own /// column in [X]=abs([A]). /// - private static bool IsFunctionOnColumnSide(string predicate, Match funcMatch, ScanIdentity? identity = null) + private static bool IsFunctionOnColumnSide(string predicate, Match funcMatch, ScanIdentity? identity, out bool undecided) { - var comparison = ComparisonContaining(predicate, funcMatch.Index, out var offset); + // True only when a column is shown on the function's side. A step that was cut short sets undecided and + // returns false: the caller reports the predicate as not fully checked, never as holding a column. + var comparison = ComparisonContaining(predicate, funcMatch.Index, out var offset, out undecided); + if (undecided) + return false; + + var compMatch = PlanRegex.Match(ComparisonOperatorRegex, comparison, "comparison operator in a predicate", out var compDone); + if (!compDone) + { + undecided = true; + return false; + } - var compMatch = ComparisonOperatorRegex.Match(comparison); if (!compMatch.Success) - return true; // No comparison found — can't determine side, assume worst case + return true; // No comparison in the text — can't determine side, assume worst case var compPos = compMatch.Index; var funcPos = funcMatch.Index - offset; @@ -2792,7 +2898,7 @@ private static bool IsFunctionOnColumnSide(string predicate, Match funcMatch, Sc // Same column-vs-variable distinction ConvertImplicitWrapsColumn needs, so it shares the // one helper rather than keeping a second copy of the logic in sync by hand. - return IsColumnReference(side, identity); + return IsColumnReference(side, identity, out undecided); } /// @@ -2806,12 +2912,14 @@ private static bool IsFunctionOnColumnSide(string predicate, Match funcMatch, Sc /// move a comparison operator or add a column, so they are harmless. No function in a /// ScalarString takes AND/OR inside its arguments; CASE does, and it is caught earlier. /// - private static string ComparisonContaining(string predicate, int position, out int offset) + private static string ComparisonContaining(string predicate, int position, out int offset, out bool incomplete) { var start = 0; var end = predicate.Length; - foreach (Match match in LogicalOperatorRegex.Matches(predicate)) + // Lazy, so the first AND/OR after the position ends the pass (the break below). + var operators = PlanRegex.Matches(LogicalOperatorRegex, predicate, "AND/OR split of a predicate"); + foreach (Match match in operators) { if (!match.Groups[1].Success) continue; // a string literal or bracketed name, skipped whole @@ -2827,6 +2935,9 @@ private static string ComparisonContaining(string predicate, int position, out i } } + // A pass cut short cannot say where the comparison ends: the caller treats the side as undecided. + incomplete = operators.Incomplete; + offset = start; return predicate[start..end]; } @@ -2856,19 +2967,50 @@ private static string ComparisonContaining(string predicate, int position, out i /// own bare columns apart from a bare outer reference off a different one; see /// . /// - internal static bool ConvertImplicitWrapsColumn(string predicate, ScanIdentity? identity = null) + internal static bool ConvertImplicitWrapsColumn(string predicate, ScanIdentity? identity = null) => + ConvertImplicitWrapsColumn(predicate, identity, out _); + + /// + /// The same check, telling a caller whether the pass was cut short (a match timeout or the plan's spent + /// regex budget). A cut-short pass that found no column returns false with true. + /// + private static bool ConvertImplicitWrapsColumn(string predicate, ScanIdentity? identity, out bool incomplete) { - foreach (Match match in ConvertImplicitRegex.Matches(predicate)) + const string check = "CONVERT_IMPLICIT in a predicate"; + incomplete = false; + var conversions = PlanRegex.Matches(ConvertImplicitRegex, predicate, check); + foreach (Match match in conversions) { - // The regex ends at the opening paren, so its last character is where the args start. - var arguments = ExtractBalancedArguments(predicate, match.Index + match.Length - 1); + // The scan to each closing parenthesis is not a regex call, so the plan's time limit is checked + // here, once per conversion and along the scan. + if (PlanRegex.TimeIsUp(check)) + { + incomplete = true; + return false; + } + + // The regex ends at the opening paren, so its last character is where the args start. The + // arguments are read in place, between the parentheses, with no copy of the text. + var open = match.Index + match.Length - 1; + var close = FindClosingParenthesis(predicate, open, check); + if (close == ScanTimedOut) + { + incomplete = true; + return false; + } // Unparseable means we cannot tell what is being converted. Assume the worst, matching // IsFunctionOnColumnSide, rather than silently dropping a real conversion. - if (arguments == null || IsColumnReference(arguments, identity)) + if (close == NoClosingParenthesis || IsColumnReference(predicate, open + 1, close - open - 1, identity, out var undecided)) + { + incomplete = false; return true; + } + + incomplete |= undecided; } + incomplete |= conversions.Incomplete; return false; } @@ -2911,14 +3053,30 @@ internal static bool ConvertImplicitWrapsColumn(string predicate, ScanIdentity? /// . /// /// - private static bool IsColumnReference(string text, ScanIdentity? identity) + private static bool IsColumnReference(string text, ScanIdentity? identity, out bool undecided) => + IsColumnReference(text, 0, text.Length, identity, out undecided); + + /// + /// The same question about text[start..(start + length)], read in place. True only when a column is shown. + /// A step that was cut short sets , and when no column was found by then the answer + /// is false: the caller words the result as unverified, and never claims a column that was not seen. + /// + private static bool IsColumnReference(string text, int start, int length, ScanIdentity? identity, out bool undecided) { + undecided = false; if (identity == null) - return ColumnReferenceRegex.IsMatch(text); + { + if (PlanRegex.TryIsMatch(ColumnReferenceRegex, text, start, length, "column reference in a predicate", out var hasColumn)) + return hasColumn; + + undecided = true; + return false; + } var scan = identity.Value; - foreach (Match match in BracketedNameRegex.Matches(text)) + var names = PlanRegex.Matches(BracketedNameRegex, text, start, length, "column reference in a predicate"); + foreach (Match match in names) { if (!match.Groups["name"].Success || match.Groups["call"].Success) continue; // a string literal, or the name of a function @@ -2931,7 +3089,14 @@ private static bool IsColumnReference(string text, ScanIdentity? identity) if (FollowedByAsBracket(text, match.Index + match.Length)) continue; - var parts = NamePartRegex.Matches(name); + var partRun = PlanRegex.Matches(NamePartRegex, name, "column reference in a predicate"); + var parts = partRun.ToList(); + if (partRun.Incomplete) + { + undecided = true; // the name could not be split: it is not read as a column + continue; + } + if (parts.Count >= 2) { var owner = StripBrackets(parts[^2].Value); @@ -2962,7 +3127,13 @@ private static bool IsColumnReference(string text, ScanIdentity? identity) if (name.StartsWith("[@", StringComparison.Ordinal)) continue; // a parameter or a variable: [@p1] - if (ExpressionColumnRegex.IsMatch(name)) + if (!PlanRegex.TryIsMatch(ExpressionColumnRegex, name, "column reference in a predicate", out var isExpression)) + { + undecided = true; // not known to be a column + continue; + } + + if (isExpression) continue; // an optimizer-generated expression, not an actual column: [Expr1003] if (scan.IsTableVariable && string.IsNullOrEmpty(scan.Alias) && @@ -2970,6 +3141,8 @@ private static bool IsColumnReference(string text, ScanIdentity? identity) return true; // a bare name — this unaliased table variable's own column } + // A pass cut short proved nothing about the rest of the text: undecided, not a column. + undecided |= names.Incomplete; return false; } @@ -2998,12 +3171,17 @@ private static bool PrecededByAs(string text, int index) => private static string StripBrackets(string bracketPart) => bracketPart.Replace("[", "").Replace("]", ""); + private const int NoClosingParenthesis = -1; + private const int ScanTimedOut = -2; + /// - /// Returns the text between the parenthesis at and its match, - /// or null if the parentheses do not balance. Needed because the target type of a conversion can - /// carry its own parentheses — numeric(18,0), varchar(50) — so the first ')' is not the end. + /// The index of the parenthesis that closes the one at , + /// if the parentheses do not balance, or if the + /// plan's time limit passed during the scan (recorded under ). Needed because the + /// target type of a conversion can carry its own parentheses — numeric(18,0), varchar(50) — so the first ')' + /// is not the end. The caller reads the arguments in place, between the two indexes. /// - private static string? ExtractBalancedArguments(string text, int openParenIndex) + private static int FindClosingParenthesis(string text, int openParenIndex, string check) { var depth = 0; for (var i = openParenIndex; i < text.Length; i++) @@ -3016,11 +3194,14 @@ private static string StripBrackets(string bracketPart) => { depth--; if (depth == 0) - return text[(openParenIndex + 1)..i]; + return i; } + + if ((i & 0xFFFF) == 0xFFFF && PlanRegex.TimeIsUp(check)) + return ScanTimedOut; } - return null; + return NoClosingParenthesis; } // Rule 33 only looks at tables with at least this many rows, and DetectCeGuess relies on that: @@ -3093,40 +3274,40 @@ private static string StripBrackets(string bracketPart) => }; } - [GeneratedRegex(@"\b(CONVERT_IMPLICIT|CONVERT|CAST|isnull|coalesce|datepart|datediff|dateadd|year|month|day|upper|lower|ltrim|rtrim|trim|substring|left|right|charindex|replace|len|datalength|abs|floor|ceiling|round|reverse|stuff|format)\s*\(", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"\b(CONVERT_IMPLICIT|CONVERT|CAST|isnull|coalesce|datepart|datediff|dateadd|year|month|day|upper|lower|ltrim|rtrim|trim|substring|left|right|charindex|replace|len|datalength|abs|floor|ceiling|round|reverse|stuff|format)\s*\(", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex FunctionInPredicateRegExp(); - [GeneratedRegex(@"\blike\b[^'""]*?N?'%", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"\blike\b[^'""]*?N?'%", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex LeadingWildcardLikeRegExp(); - [GeneratedRegex(@"\bCASE\s+(WHEN\b|$)", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"\bCASE\s+(WHEN\b|$)", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex CaseInPredicateRegExp(); - [GeneratedRegex(@"\b(isnull|coalesce)\s*\(", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"\b(isnull|coalesce)\s*\(", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex IsNullCoalesceRegExp(); - [GeneratedRegex(@"\bCONVERT_IMPLICIT\s*\(", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"\bCONVERT_IMPLICIT\s*\(", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex ConvertImplicitRegExp(); // A column reference in a ScalarString is multi-part bracket-qualified ([schema].[table]). // A variable is a single bracket pair with an @ prefix ([@0]), so excluding @ from the first // part is what separates the two. - [GeneratedRegex(@"\[[^\]@]+\]\.\[")] + [GeneratedRegex(@"\[[^\]@]+\]\.\[", RegexOptions.None, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex ColumnReferenceRegExp(); - [GeneratedRegex(@"^\[Expr\d+\]$")] + [GeneratedRegex(@"^\[Expr\d+\]$", RegexOptions.None, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex ExpressionColumnRegExp(); - [GeneratedRegex(@"\[(?:[^\]]|\]\])*\]")] + [GeneratedRegex(@"\[(?:[^\]]|\]\])*\]", RegexOptions.None, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex NamePartRegExp(); - [GeneratedRegex(@"'(?:[^']|'')*'|(?\[(?:[^\]]|\]\])*\](?:\.\[(?:[^\]]|\]\])*\])*)(?\s*\()?")] + [GeneratedRegex(@"'(?:[^']|'')*'|(?\[(?:[^\]]|\]\])*\](?:\.\[(?:[^\]]|\]\])*\])*)(?\s*\()?", RegexOptions.None, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex BracketedNameRegExp(); // The operator a comparison turns on in a ScalarString: >=, <=, <>, !=, >, <, = or like. // Without like, [col] like upper([@p]) had no operator at all, fell to the assume-the-worst // default, and a function on the pattern was reported as a function on the column. - [GeneratedRegex(@"(?])([<>=!]{1,2})(?![<>=])|\s(like)\s", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"(?])([<>=!]{1,2})(?![<>=])|\s(like)\s", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex ComparisonOperatorRegExp(); // What joins one comparison to the next in a compound predicate. String literals and // bracketed identifiers are matched first, so an AND inside one of them (N'Tom AND Jerry', // [Terms and Conditions]) is consumed whole and never reaches the capture group. Only a // Groups[1] match is a real operator. - [GeneratedRegex(@"'(?:[^']|'')*'|\[(?:[^\]]|\]\])*\]|\s(AND|OR)\s", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"'(?:[^']|'')*'|\[(?:[^\]]|\]\])*\]|\s(AND|OR)\s", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex LogicalOperatorRegExp(); - [GeneratedRegex(@"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex OptimizeForUnknownRegExp(); - [GeneratedRegex(@"\bNOT\s+IN\b", RegexOptions.IgnoreCase)] + [GeneratedRegex(@"\bNOT\s+IN\b", RegexOptions.IgnoreCase, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex NotInRegExp(); } diff --git a/PerformanceMonitor.PlanAnalysis/PlanRegex.cs b/PerformanceMonitor.PlanAnalysis/PlanRegex.cs new file mode 100644 index 0000000000..f75bf1f1d8 --- /dev/null +++ b/PerformanceMonitor.PlanAnalysis/PlanRegex.cs @@ -0,0 +1,312 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using System.Text.RegularExpressions; + +namespace PerformanceMonitor.PlanAnalysis; + +/// +/// Every regular expression the plan-analysis project runs on plan text goes through here. Statement text and +/// predicate strings come from plan XML that a person or an MCP client may have written, and a pattern +/// such as \s*AND\s+PROBE\s*\(... on a long run of spaces backtracks for minutes. Two limits apply: +/// +/// each call has a match timeout (); +/// inside , one plan has a wall-clock deadline +/// () that starts when the analysis starts and counts all the time, in +/// regex calls and out of them. The regex helpers check it, and so do the loops that run without a regex +/// (). Once it has passed, every further check is skipped. +/// +/// A timeout or a skipped call never throws and never quietly changes the answer: the check is recorded +/// (), and the analyzer adds one note to the statement that names the checks it +/// skipped. A skipped check is a missing result and never an invented one: a warning that states a fact about +/// the text is raised only when the text shows it. Each helper takes the check's name for that note, and each +/// call site picks the answer a timeout gives: +/// +/// IsMatch gives the caller's onTimeout value, or, in the overloads that take a +/// linear fallback, the answer of a plain text search that needs no regex and is not recorded as skipped; +/// Match and Replace report through completed that they did not finish; +/// Matches yields lazily, so a caller's early exit still ends the pass, and its +/// says the pass stopped before the end. +/// +/// The overloads that take a exist so a test can force a timeout in a few milliseconds. +/// The [GeneratedRegex] fields carry the same timeout in their attribute, and every use of one goes +/// through the overloads here. +/// +internal static class PlanRegex +{ + /// The match timeout in milliseconds: the default of every helper and of every generated regex. + internal const int TimeoutMilliseconds = 250; + + /// + /// The wall-clock time one plan's analysis may run before its remaining checks are skipped, in + /// milliseconds, counted from the start of the analysis. Analyzing a normal plan takes a few milliseconds, + /// so 2 seconds is far above any real plan, while the worst case per plan stays near 2 seconds plus one + /// regex call's timeout or one pass over a predicate, no matter how much of the time goes to loops that + /// run without a regex. The Darling service and Lite analyze the top plans of every cycle, so this + /// bounds the cycle's cost. + /// + internal const int PlanBudgetMilliseconds = 2000; + + internal static readonly TimeSpan DefaultTimeout = TimeSpan.FromMilliseconds(TimeoutMilliseconds); + + /// The note's wording for the checks returned. + internal static string SkippedNote(IReadOnlyList checks) => + "Some checks on this statement were skipped because the plan was too large or complex to finish within the analysis time limit: " + + string.Join(", ", checks) + ". Warnings that depend on those checks may be missing or less specific."; + + // ---- The per-plan budget ---- + + private sealed class Budget + { + private readonly TimeSpan _limit; + private readonly long _startStamp = Stopwatch.GetTimestamp(); + private readonly Func? _clock = ClockForTest; + private readonly TimeSpan _clockStart; + private readonly List _skipped = new(); + + internal Budget(TimeSpan limit) + { + _limit = limit; + _clockStart = _clock?.Invoke() ?? TimeSpan.Zero; + } + + private TimeSpan Elapsed => _clock != null ? _clock() - _clockStart : Stopwatch.GetElapsedTime(_startStamp); + + internal bool Exhausted => Elapsed >= _limit; + + internal TimeSpan Remaining + { + get + { + var left = _limit - Elapsed; + return left > TimeSpan.Zero ? left : TimeSpan.Zero; + } + } + + internal void Skip(string check) + { + if (!_skipped.Contains(check)) + _skipped.Add(check); + } + + internal string[] Take() + { + var taken = _skipped.ToArray(); + _skipped.Clear(); + return taken; + } + } + + /// + /// A test's own clock, read as the time since an origin of the test's choosing, on this thread only. A plan + /// that begins while it is set measures its deadline against it, so a test can spend the time limit + /// part-way through a loop without waiting. + /// + [ThreadStatic] + internal static Func? ClockForTest; + + // Analysis is synchronous, so the plan's budget lives on the thread that runs it. + [ThreadStatic] + private static Budget? t_budget; + + /// Ends the plan's budget when disposed. A scope opened inside another one shares the outer budget. + internal sealed class Scope : IDisposable + { + private readonly object? _owned; + + internal Scope(object? owned) => _owned = owned; + + public void Dispose() + { + if (_owned != null && ReferenceEquals(t_budget, _owned)) + t_budget = null; + } + } + + /// Starts one plan's time budget on this thread. + internal static Scope BeginPlan() => BeginPlan(TimeSpan.FromMilliseconds(PlanBudgetMilliseconds)); + + internal static Scope BeginPlan(TimeSpan budget) + { + if (t_budget != null) + return new Scope(null); + + var created = new Budget(budget); + t_budget = created; + return new Scope(created); + } + + /// The checks skipped (timeout or spent budget) since the last call, without repeats. Empty outside a plan. + internal static IReadOnlyList TakeSkipped() => t_budget?.Take() ?? Array.Empty(); + + /// False, with the check recorded as skipped when , once the plan's deadline has passed. + private static bool CanRun(string check, bool record = true) + { + var budget = t_budget; + if (budget == null || !budget.Exhausted) + return true; + + if (record) + budget.Skip(check); + return false; + } + + /// + /// For a loop that runs without a regex: true, with recorded as skipped, once the + /// plan's deadline has passed, and the loop must stop. Always false outside a plan. + /// + internal static bool TimeIsUp(string check) => !CanRun(check); + + private static void RecordTimeout(string check) => t_budget?.Skip(check); + + /// The timeout for one call: the given one, or what is left of the plan's budget when that is less. + private static TimeSpan Limit(TimeSpan timeout) + { + var remaining = t_budget?.Remaining; + if (remaining == null || remaining.Value >= timeout) + return timeout; + + return remaining.Value < TimeSpan.FromMilliseconds(1) ? TimeSpan.FromMilliseconds(1) : remaining.Value; + } + + /// Runs one regex call under the budget. False when it was skipped or timed out; the check is recorded. + private static bool Run(string check, Func call, out T result, bool record = true) + { + result = default!; + if (!CanRun(check, record)) + return false; + + try + { + result = call(); + return true; + } + catch (RegexMatchTimeoutException) + { + if (record) + RecordTimeout(check); + return false; + } + } + + // ---- Pattern strings ---- + + internal static bool IsMatch(string input, string pattern, RegexOptions options, string check, bool onTimeout = false) => + IsMatch(input, pattern, options, DefaultTimeout, check, onTimeout); + + internal static bool IsMatch(string input, string pattern, RegexOptions options, TimeSpan timeout, string check, bool onTimeout = false) => + Run(check, () => Regex.IsMatch(input, pattern, options, Limit(timeout)), out var found) ? found : onTimeout; + + /// + /// The regex's answer, or, when the call timed out or was skipped, 's answer + /// on the same text: a linear text search that decides the same question. Nothing is recorded as skipped, + /// because the question was answered. + /// + internal static bool IsMatch(string input, string pattern, RegexOptions options, string check, Func withoutRegex) => + Run(check, () => Regex.IsMatch(input, pattern, options, Limit(DefaultTimeout)), out var found, record: false) ? found : withoutRegex(input); + + /// The match, or with false. + internal static Match Match(string input, string pattern, RegexOptions options, string check, out bool completed) => + Match(input, pattern, options, DefaultTimeout, check, out completed); + + internal static Match Match(string input, string pattern, RegexOptions options, TimeSpan timeout, string check, out bool completed) + { + completed = Run(check, () => Regex.Match(input, pattern, options, Limit(timeout)), out var match); + return completed ? match : System.Text.RegularExpressions.Match.Empty; + } + + /// The replaced text, or the input unchanged with false. + /// False when the caller answers the question another way if the call does not finish, so nothing is recorded as skipped. + internal static string Replace(string input, string pattern, string replacement, RegexOptions options, string check, out bool completed, bool recordSkip = true) => + Replace(input, pattern, replacement, options, DefaultTimeout, check, out completed, recordSkip); + + internal static string Replace(string input, string pattern, string replacement, RegexOptions options, TimeSpan timeout, string check, out bool completed, bool recordSkip = true) + { + completed = Run(check, () => Regex.Replace(input, pattern, replacement, options, Limit(timeout)), out var replaced, recordSkip); + return completed ? replaced : input; + } + + // ---- Regex instances ([GeneratedRegex] and compiled fields): their own timeout applies ---- + + /// The answer when the call finished; when it timed out or was skipped. + internal static bool IsMatch(Regex regex, string input, string check, bool onTimeout = false) => + TryIsMatch(regex, input, check, out var found) ? found : onTimeout; + + /// The answer of , or 's when the call timed out or was skipped (nothing recorded). + internal static bool IsMatch(Regex regex, string input, string check, Func withoutRegex) => + Run(check, () => regex.IsMatch(input), out var found, record: false) ? found : withoutRegex(input); + + /// + /// on input[start..(start + length)], read in + /// place with no copy. The range is the whole input as far as the pattern's anchors go. + /// + internal static bool TryIsMatch(Regex regex, string input, int start, int length, string check, out bool matched) => + Run(check, () => regex.Match(input, start, length).Success, out matched); + + /// False when the call timed out or was skipped; is then false. + internal static bool TryIsMatch(Regex regex, string input, string check, out bool matched) => + Run(check, () => regex.IsMatch(input), out matched); + + /// The match, or with false. + internal static Match Match(Regex regex, string input, string check, out bool completed) + { + completed = Run(check, () => regex.Match(input), out var match); + return completed ? match : System.Text.RegularExpressions.Match.Empty; + } + + /// + /// The matches of , found lazily one at a time, so a caller that leaves its + /// foreach early stops the pass. Each step is timed against the plan's budget. A timeout or a + /// spent budget ends the list at the matches found so far and sets . + /// + internal static MatchRun Matches(Regex regex, string input, string check) => new(regex, input, check, 0, input.Length); + + /// over input[start..(start + length)], read in place with no copy. + internal static MatchRun Matches(Regex regex, string input, int start, int length, string check) => new(regex, input, check, start, length); + + /// One lazy pass over a regex's matches. Enumerate it once; read afterwards. + internal sealed class MatchRun : IEnumerable + { + private readonly Regex _regex; + private readonly string _input; + private readonly string _check; + private readonly int _start; + private readonly int _length; + + internal MatchRun(Regex regex, string input, string check, int start, int length) + { + _regex = regex; + _input = input; + _check = check; + _start = start; + _length = length; + } + + /// True when the pass stopped before the end of the input because of a timeout or the spent budget. + internal bool Incomplete { get; private set; } + + public IEnumerator GetEnumerator() + { + var current = Step(null); + while (current != null && current.Success) + { + yield return current; + current = Step(current); + } + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + private Match? Step(Match? previous) + { + if (!Run(_check, () => previous == null ? _regex.Match(_input, _start, _length) : previous.NextMatch(), out var next)) + { + Incomplete = true; + return null; + } + + return next; + } + } +} diff --git a/PerformanceMonitor.PlanAnalysis/PlanTextSearch.cs b/PerformanceMonitor.PlanAnalysis/PlanTextSearch.cs new file mode 100644 index 0000000000..018dd57fcc --- /dev/null +++ b/PerformanceMonitor.PlanAnalysis/PlanTextSearch.cs @@ -0,0 +1,124 @@ +using System; + +namespace PerformanceMonitor.PlanAnalysis; + +/// +/// Plain text searches that answer the same yes-or-no questions as a few of the analyzer's regexes, in one linear +/// pass and with no regex. When a regex is skipped (the plan's time limit has passed, or the call timed out), the +/// call site asks one of these instead, so a warning that states a fact about the statement text is raised only +/// when the text shows it. A word here is delimited as a regex's \b would: by a character that is not a +/// letter, a digit or an underscore. A gap between words is one or more whitespace characters, as \s+. +/// +internal static class PlanTextSearch +{ + private static bool IsWordChar(char c) => char.IsLetterOrDigit(c) || c == '_'; + + /// + /// True when holds in order, each pair separated by + /// whitespace, compared without regard to case. asks for a word boundary before the + /// first word, and for one after the last. + /// + internal static bool ContainsWords(string text, bool wordStart, bool wordEnd, params string[] words) + { + var span = text.AsSpan(); + var from = 0; + while (from <= span.Length - words[0].Length) + { + var found = span[from..].IndexOf(words[0], StringComparison.OrdinalIgnoreCase); + if (found < 0) + return false; + + var at = from + found; + from = at + 1; + if (wordStart && at > 0 && IsWordChar(span[at - 1])) + continue; + + var position = at + words[0].Length; + var matched = true; + for (var w = 1; w < words.Length && matched; w++) + { + var gapStart = position; + while (position < span.Length && char.IsWhiteSpace(span[position])) + position++; + + matched = position > gapStart + && span[position..].StartsWith(words[w], StringComparison.OrdinalIgnoreCase); + if (matched) + position += words[w].Length; + } + + if (!matched) + continue; + + if (wordEnd && position < span.Length && IsWordChar(span[position])) + continue; + + return true; + } + + return false; + } + + /// + /// True when a predicate holds nothing but PROBE(...) bitmap filters joined by AND or OR, as the regex pair that + /// strips them does. The arguments of a PROBE may nest parentheses to any depth here; an unbalanced one is not + /// PROBE-only. + /// + internal static bool IsProbeOnly(string predicate) + { + var position = 0; + while (true) + { + while (position < predicate.Length && char.IsWhiteSpace(predicate[position])) + position++; + + if (position >= predicate.Length) + return true; + + var rest = predicate.AsSpan(position); + if (StartsWithWord(rest, "AND")) + { + position += 3; + } + else if (StartsWithWord(rest, "OR")) + { + position += 2; + } + else if (rest.StartsWith("PROBE", StringComparison.OrdinalIgnoreCase)) + { + position += 5; + while (position < predicate.Length && char.IsWhiteSpace(predicate[position])) + position++; + + if (position >= predicate.Length || predicate[position] != '(') + return false; + + var depth = 0; + for (; position < predicate.Length; position++) + { + if (predicate[position] == '(') + { + depth++; + } + else if (predicate[position] == ')' && --depth == 0) + { + break; + } + } + + if (depth != 0) + return false; + + position++; // past the closing parenthesis + } + else + { + return false; + } + } + } + + private static bool StartsWithWord(ReadOnlySpan rest, string word) => + rest.StartsWith(word, StringComparison.OrdinalIgnoreCase) + && (rest.Length == word.Length || !IsWordChar(rest[word.Length])); +} diff --git a/PerformanceMonitor.PlanAnalysis/QueryModificationDetector.cs b/PerformanceMonitor.PlanAnalysis/QueryModificationDetector.cs index c5ef3b3063..6132ab76b7 100644 --- a/PerformanceMonitor.PlanAnalysis/QueryModificationDetector.cs +++ b/PerformanceMonitor.PlanAnalysis/QueryModificationDetector.cs @@ -88,7 +88,8 @@ not analyzed. */ /// private static bool TextRunsCodeThePlanDoesNotShow(string? queryText) => !string.IsNullOrWhiteSpace(queryText) - && (HasExecKeyword(queryText) || s_remoteRowset.IsMatch(queryText)); + && (HasExecKeyword(queryText) + || PlanRegex.IsMatch(s_remoteRowset, queryText, "remote rowset functions in the query text", onTimeout: true)); /// True when a Showplan StatementType is an EXECUTE one (EXECUTE PROC). private static bool IsExecuteStatementType(string? statementType) @@ -100,12 +101,14 @@ column reached through a dot or a variable or temp-table name. A closing bracket is fully wrapped in a matching pair (`[exec]`, `"exec"`) is an identifier. */ private static readonly Regex s_execStatement = new( @"(?True when the text has an EXEC / EXECUTE keyword that is not a bracketed or double-quoted identifier. private static bool HasExecKeyword(string queryText) { - foreach (Match match in s_execStatement.Matches(queryText)) + var statements = PlanRegex.Matches(s_execStatement, queryText, "EXEC keyword in the query text"); + foreach (Match match in statements) { var before = match.Index > 0 ? queryText[match.Index - 1] : '\0'; var afterIndex = match.Index + match.Length; @@ -117,12 +120,14 @@ private static bool HasExecKeyword(string queryText) } } - return false; + // A pass cut short cannot rule EXEC out: the re-execution gate takes the safe direction. + return statements.Incomplete; } private static readonly Regex s_remoteRowset = new( @"\b(?:OPENQUERY|OPENROWSET|OPENDATASOURCE)\b", - RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled); + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled, + PlanRegex.DefaultTimeout); /// /// True when a Showplan StatementType value denotes a data- or schema-modifying statement. Public so @@ -193,7 +198,8 @@ private static IReadOnlyList ScanTextForStatementTypes(string? queryText var found = new List(); foreach (var keyword in s_textKeywords) { - if (Regex.IsMatch(queryText, $@"\b{keyword}\b", RegexOptions.IgnoreCase)) + // Display only: a keyword that cannot be checked in time is listed, the safe direction. + if (PlanRegex.IsMatch(queryText, $@"\b{keyword}\b", RegexOptions.IgnoreCase, "statement keyword scan", onTimeout: true)) { found.Add(keyword); } diff --git a/PerformanceMonitor.PlanAnalysis/ReproScriptBuilder.cs b/PerformanceMonitor.PlanAnalysis/ReproScriptBuilder.cs index dc5b61a503..554e3c61b1 100644 --- a/PerformanceMonitor.PlanAnalysis/ReproScriptBuilder.cs +++ b/PerformanceMonitor.PlanAnalysis/ReproScriptBuilder.cs @@ -152,7 +152,7 @@ rather than leaving an unexplained placeholder. */ separator, so the USE line is skipped entirely rather than emitted with an embedded line break that a line-based batch splitter (unlike the bracket-aware ones this project ships) would read as its own GO. Decided before the header is built so the warning lands in it. */ - var skipUseForControlChars = !string.IsNullOrEmpty(databaseName) && DatabaseNameHasControlCharsRegex().IsMatch(databaseName); + var skipUseForControlChars = !string.IsNullOrEmpty(databaseName) && PlanRegex.IsMatch(DatabaseNameHasControlCharsRegex(), databaseName, "control characters in the database name", onTimeout: true); if (skipUseForControlChars) { warnings.Add("Database name contains control or line-separator characters — USE statement omitted for safety."); @@ -292,7 +292,7 @@ private static List DetectTempTablesAndTableVariables(string queryText) foreach (var pattern in tempTablePatterns) { - if (Regex.IsMatch(queryText, pattern, RegexOptions.IgnoreCase)) + if (PlanRegex.IsMatch(queryText, pattern, RegexOptions.IgnoreCase, "temp table references", onTimeout: true)) { warnings.Add("Query references temp table(s) — reproduction may require creating the temp table first"); break; @@ -314,7 +314,7 @@ private static List DetectTempTablesAndTableVariables(string queryText) foreach (var pattern in tableVarPatterns) { - if (Regex.IsMatch(queryText, pattern, RegexOptions.IgnoreCase)) + if (PlanRegex.IsMatch(queryText, pattern, RegexOptions.IgnoreCase, "table variable references", onTimeout: true)) { warnings.Add("Query may reference table variable(s) — reproduction may require declaring and populating the table variable first"); break; @@ -505,7 +505,9 @@ private static string CommentSafe(string? text) { /* \p{Cc} covers CR, LF, tab and NEL; U+2028 and U+2029 are the Unicode line and paragraph separators, which some editors also treat as line breaks. */ - return Regex.Replace(text ?? "", @"[\p{Cc}\u2028\u2029]", " ") + // Char by char rather than a regex: a replacement that could time out and hand the text back unchanged + // would leave the line breaks this exists to remove. + return string.Concat((text ?? "").Select(c => char.IsControl(c) || c == '\u2028' || c == '\u2029' ? ' ' : c)) .Replace("*/", "* /") .Replace("/*", "/ *"); } @@ -517,7 +519,8 @@ private static string CommentSafe(string? text) /// private static bool IsValidParameterName(string name) { - return Regex.IsMatch(name, @"\A@[\p{L}\p{Nd}_@#$]+\z"); + // A name that cannot be checked in time is not valid: it is dropped from the script. + return PlanRegex.IsMatch(name, @"\A@[\p{L}\p{Nd}_@#$]+\z", RegexOptions.None, "parameter name check", onTimeout: false); } /// @@ -532,10 +535,11 @@ private static bool IsValidParameterName(string name) private static bool IsValidDataType(string dataType) { const string name = @"(?:\[[\p{L}\p{Nd}_ ]+\]|[\p{L}_][\p{L}\p{Nd}_]*)"; - return Regex.IsMatch( + // A type that cannot be checked in time is not valid: it is dropped with the existing warning. + return PlanRegex.IsMatch( dataType, $@"\A{name}(?:\.{name}){{0,2}}(?: *\( *(?:max|[0-9]+(?: *, *(?:[0-9]+|{name}))?) *\))?\z", - RegexOptions.IgnoreCase); + RegexOptions.IgnoreCase, "parameter data type check", onTimeout: false); } /// @@ -550,15 +554,15 @@ private static bool IsSafeLiteral(string value) /* Integer/decimal/float/money forms: -12, 3.14, 1.5E+3, $9.99. [0-9], not \d, which also matches digits from other scripts that T-SQL doesn't read as numbers. */ - if (Regex.IsMatch(value, @"\A-?\$?[0-9]+(\.[0-9]+)?([eE][+-]?[0-9]+)?\z")) + if (PlanRegex.IsMatch(value, @"\A-?\$?[0-9]+(\.[0-9]+)?([eE][+-]?[0-9]+)?\z", RegexOptions.None, "parameter value check", onTimeout: false)) return true; /* Binary literal */ - if (Regex.IsMatch(value, @"\A0x[0-9A-Fa-f]*\z")) + if (PlanRegex.IsMatch(value, @"\A0x[0-9A-Fa-f]*\z", RegexOptions.None, "parameter value check", onTimeout: false)) return true; /* One complete string literal — every embedded quote must be doubled */ - if (Regex.IsMatch(value, @"\AN?'([^']|'')*'\z")) + if (PlanRegex.IsMatch(value, @"\AN?'([^']|'')*'\z", RegexOptions.None, "parameter value check", onTimeout: false)) return true; return false; @@ -574,7 +578,7 @@ private static List FindUnresolvedVariables(string queryText, List(parameters.Select(p => p.Name), StringComparer.OrdinalIgnoreCase); /* Find all @variable references in the query text */ - var matches = AtVariableRegExp().Matches(queryText); + var matches = PlanRegex.Matches(AtVariableRegExp(), queryText, "unresolved variables in the script"); var seenVars = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (Match match in matches) @@ -605,7 +609,7 @@ private static List FindUnresolvedVariables(string queryText, List @@ -613,7 +617,7 @@ private static List FindUnresolvedVariables(string queryText, List - [GeneratedRegex(@"[\p{Cc}\u2028\u2029]")] + [GeneratedRegex(@"[\p{Cc}\u2028\u2029]", RegexOptions.None, matchTimeoutMilliseconds: PlanRegex.TimeoutMilliseconds)] private static partial Regex DatabaseNameHasControlCharsRegex(); } diff --git a/PerformanceMonitor.Ui/CallerPlanXmlPrompt.cs b/PerformanceMonitor.Ui/CallerPlanXmlPrompt.cs new file mode 100644 index 0000000000..1aed5e48d9 --- /dev/null +++ b/PerformanceMonitor.Ui/CallerPlanXmlPrompt.cs @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System.Windows; +using PerformanceMonitor.PlanAnalysis; + +namespace PerformanceMonitor.Ui; + +/// +/// The viewer side of : the open, drop and paste paths call it on the text a person +/// supplied, right after they read it and before anything is loaded. It shows the reason and tells the caller to +/// stop. Plans the collectors stored reach the viewer through LoadPlan directly and never come here. +/// +internal static class CallerPlanXmlPrompt +{ + /// True when the text was refused (the dialog has been shown and nothing may be loaded). + internal static bool Refuse(string planXml) + { + var reason = CallerPlanXml.RefusalReason(planXml); + if (reason == null) return false; + + MessageBox.Show( + reason, + "Plan Not Opened", + MessageBoxButton.OK, + MessageBoxImage.Warning); + return true; + } +} diff --git a/PerformanceMonitor.Ui/DataGridClipboardBehavior.cs b/PerformanceMonitor.Ui/DataGridClipboardBehavior.cs index af03c07233..8bf52ae4af 100644 --- a/PerformanceMonitor.Ui/DataGridClipboardBehavior.cs +++ b/PerformanceMonitor.Ui/DataGridClipboardBehavior.cs @@ -52,15 +52,30 @@ public static string GetHeaderText(DataGridColumn column) } /// - /// Event handler that can be wired directly to DataGrid.CopyingRowClipboardContent. + /// Event handler that can be wired directly to DataGrid.CopyingRowClipboardContent. It fixes the header + /// row (see the class summary) and, on every data row, passes text cells through + /// : tabs and line breaks inside a cell become one space, + /// and a cell that starts with a formula character is pasted into a spreadsheet as text. Cells that are not strings (numbers, dates) are left as they are. /// public static void FixHeaderCopy(object? sender, DataGridRowClipboardEventArgs e) { - if (!e.IsColumnHeadersRow) return; - for (int i = 0; i < e.ClipboardRowContent.Count; i++) { var cell = e.ClipboardRowContent[i]; + if (!e.IsColumnHeadersRow) + { + if (cell.Content is string text) + { + var safe = SpreadsheetText.NeutralizeClipboardCell(text); + if (!ReferenceEquals(safe, text)) + { + e.ClipboardRowContent[i] = new DataGridClipboardCellContent(cell.Item, cell.Column, safe); + } + } + + continue; + } + if (cell.Column?.Header is StackPanel sp) { var tb = sp.Children.OfType().FirstOrDefault(); diff --git a/PerformanceMonitor.Ui/DataGridExport.cs b/PerformanceMonitor.Ui/DataGridExport.cs index 2962546e7c..11ec8cd920 100644 --- a/PerformanceMonitor.Ui/DataGridExport.cs +++ b/PerformanceMonitor.Ui/DataGridExport.cs @@ -37,7 +37,10 @@ public static class DataGridExport { /* ---- side-effecting wrappers (wired to the context-menu MenuItem handlers) ---- */ - /// Copies the current cell's value to the clipboard. + /// + /// Copies the current cell's value to the clipboard exactly as it is (see ): a + /// single cell is often query text on its way into SSMS, where a leading apostrophe would break it. + /// public static void CopyCell(object sender) { var grid = FindDataGrid(sender); @@ -46,7 +49,7 @@ public static void CopyCell(object sender) var cell = grid.CurrentCell; if (cell.Column == null || cell.Item == null) return; - var value = GetCellValue(cell.Column, cell.Item); + var value = GetRawCellValue(cell.Column, cell.Item); if (value.Length > 0) ClipboardText.TrySetText(value); } @@ -63,7 +66,7 @@ public static void CopyRow(object sender) var item = FindRowItem(sender) ?? grid.CurrentItem ?? grid.SelectedItem; if (item == null) return; - ClipboardText.TrySetText(string.Join("\t", GetRowValues(grid, item))); + ClipboardText.TrySetText(string.Join("\t", GetClipboardRowValues(grid, item))); } /// Copies the whole grid (header + all rows, tab-delimited) to the clipboard. @@ -116,7 +119,7 @@ public static string BuildClipboardText(DataGrid grid) sb.AppendLine(string.Join("\t", grid.Columns.Select(DataGridClipboardBehavior.GetHeaderText))); foreach (var item in grid.Items) { - sb.AppendLine(string.Join("\t", GetRowValues(grid, item))); + sb.AppendLine(string.Join("\t", GetClipboardRowValues(grid, item))); } return sb.ToString(); } @@ -146,19 +149,48 @@ public static List GetRowValues(DataGrid grid, object item) return values; } + /// + /// Gets one string per column for a row as the tab-separated copies write it: like , + /// but a tab or a line break inside a cell becomes a space, so one cell stays one field when it is pasted. + /// + public static List GetClipboardRowValues(DataGrid grid, object item) + { + var values = new List(grid.Columns.Count); + foreach (var column in grid.Columns) + { + values.Add(GetClipboardCellValue(column, item)); + } + return values; + } + + /// One cell's text for Copy Row and Copy All Rows (see ). + public static string GetClipboardCellValue(DataGridColumn column, object item) => + ReadCell(column, item, FormatForClipboard); + /// /// Extracts a cell value for any column type. Bound columns read their binding path directly; /// template columns are instantiated and inspected for a TextBlock binding. Unhandled column /// types yield an empty string so the value count always matches the column count. /// - public static string GetCellValue(DataGridColumn column, object item) + public static string GetCellValue(DataGridColumn column, object item) => + ReadCell(column, item, FormatForExport); + + /// + /// One cell's text exactly as it is, with no apostrophe added: what Copy Cell puts on the clipboard. Copy Row, + /// Copy All Rows, the keyboard copy on selected rows and the CSV export still read through + /// , because those land in a spreadsheet. + /// + public static string GetRawCellValue(DataGridColumn column, object item) => + ReadCell(column, item, FormatRaw); + + private static string ReadCell(DataGridColumn column, object item, Func format) { if (column is DataGridBoundColumn boundColumn && boundColumn.Binding is Binding binding && binding.Path != null) { var property = item.GetType().GetProperty(binding.Path.Path); - return FormatForExport(property?.GetValue(item)); + return format(property?.GetValue(item)); } if (column is DataGridTemplateColumn templateColumn && templateColumn.CellTemplate != null) @@ -170,7 +202,7 @@ public static string GetCellValue(DataGridColumn column, object item) if (textBinding != null && textBinding.Path != null) { var property = item.GetType().GetProperty(textBinding.Path.Path); - return FormatForExport(property?.GetValue(item)); + return format(property?.GetValue(item)); } } } @@ -178,22 +210,45 @@ public static string GetCellValue(DataGridColumn column, object item) return string.Empty; } - /// Formats a value for export: invariant culture for IFormattable, else ToString. - public static string FormatForExport(object? value) + /// + /// Formats a value for export: invariant culture for IFormattable (numbers and dates, which stay as they + /// are), else ToString. Text goes through , so a cell that starts + /// with a formula character is written as text. Every grid copy that lands in a spreadsheet and every CSV + /// path reads its cells here; only the single-cell Copy Cell reads them raw (). + /// + public static string FormatForExport(object? value) => + value == null || value is IFormattable + ? FormatRaw(value) + : SpreadsheetText.Neutralize(FormatRaw(value)); + + /// + /// for the tab-separated copies: text also has its tabs and line breaks + /// collapsed to a space before the formula rule runs. The CSV export keeps its line breaks (the field is + /// quoted), so it reads . + /// + public static string FormatForClipboard(object? value) => + value == null || value is IFormattable + ? FormatRaw(value) + : SpreadsheetText.NeutralizeClipboardCell(FormatRaw(value)); + + /// The value as text with no neutralizing: invariant culture for IFormattable, else ToString. + public static string FormatRaw(object? value) { if (value == null) return string.Empty; if (value is IFormattable formattable) return formattable.ToString(null, CultureInfo.InvariantCulture); return value.ToString() ?? string.Empty; } - /// Escapes a value for CSV: quote when it contains the separator, a quote, or a newline. + /// + /// Escapes a value for CSV: quote when it contains the separator, a quote, a line break, or a comma, + /// semicolon or tab. A spreadsheet splits on its own list separator, which need not be the one the file was + /// written with, so those three are quoted whatever the configured separator is. + /// public static string CsvEscape(string value, string separator) { if (string.IsNullOrEmpty(value)) return string.Empty; if (value.Contains(separator, StringComparison.Ordinal) - || value.Contains('"') - || value.Contains('\n') - || value.Contains('\r')) + || value.AsSpan().IndexOfAny(",;\t\"\r\n".AsSpan()) >= 0) { return "\"" + value.Replace("\"", "\"\"") + "\""; } diff --git a/PerformanceMonitor.Ui/PlanViewerControl.Interaction.cs b/PerformanceMonitor.Ui/PlanViewerControl.Interaction.cs index 2275ba8ccb..e582b897f7 100644 --- a/PerformanceMonitor.Ui/PlanViewerControl.Interaction.cs +++ b/PerformanceMonitor.Ui/PlanViewerControl.Interaction.cs @@ -227,7 +227,7 @@ private async void PlanViewerControl_PreviewKeyDown(object sender, KeyEventArgs try { var (ok, text) = await ClipboardText.TryReadAsync(); - if (ok && !string.IsNullOrWhiteSpace(text)) + if (ok && !string.IsNullOrWhiteSpace(text) && !CallerPlanXmlPrompt.Refuse(text)) { try { diff --git a/PerformanceMonitor.Ui/SpreadsheetText.cs b/PerformanceMonitor.Ui/SpreadsheetText.cs new file mode 100644 index 0000000000..23c33031f7 --- /dev/null +++ b/PerformanceMonitor.Ui/SpreadsheetText.cs @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; + +namespace PerformanceMonitor.Ui; + +/// +/// Keeps exported and copied text from being read as a spreadsheet formula. Collected text (query text, object +/// and login names, wait resources) can start with =, +, - or @, or with a tab or a +/// carriage return, and a spreadsheet that opens the CSV or receives the pasted cells evaluates such a cell +/// instead of showing it. The fix is the usual one: a text cell that starts with one of those characters gets a +/// leading apostrophe. +/// +/// Text only. A number stays a number: a negative number starts with - too, and an apostrophe in +/// front of it would turn a value into text. Numbers are told apart two ways. A typed value that is not a string +/// (an int, a double, a date) is left alone by . A string that is nothing but a plain +/// number (-5, -1,234.5, -1.5E-05) is left alone by , because +/// digits, separators and an exponent cannot form a formula. +/// +/// Every grid copy, grid CSV export and chart CSV export goes through this one class, so the rule cannot +/// drift between them. It is idempotent: the apostrophe it adds is not itself a trigger character. +/// +public static class SpreadsheetText +{ + /// + /// Returns with a leading apostrophe when it starts with a tab or a carriage return, + /// or when its first character that is not white space or a control character is =, +, + /// - or @, and the text is not a plain number; otherwise returns it unchanged. A spreadsheet + /// (and the HTML form of a paste) drops leading white space, so the rule looks past it. Null becomes the + /// empty string. + /// + public static string Neutralize(string? text) + { + if (string.IsNullOrEmpty(text)) + { + return string.Empty; + } + + if (text[0] is '\t' or '\r') + { + return "'" + text; + } + + var start = 0; + while (start < text.Length && IsLeadingSkipped(text[start])) + { + start++; + } + + if (start == text.Length || !IsFormulaStart(text[start])) + { + return text; + } + + var lead = start == 0 ? text : text.Substring(start); + return IsPlainNumber(lead) ? text : "'" + text; + } + + /// White space and control characters that a spreadsheet ignores in front of a cell's text. + private static bool IsLeadingSkipped(char c) => char.IsWhiteSpace(c) || char.IsControl(c) || c == '\uFEFF'; + + /// + /// A text cell as the tab-separated copies (Copy Row, Copy All Rows, the keyboard copy) write it: every run of + /// tabs and line breaks becomes one space, then the cell goes through , which looks + /// past leading white space. A tab or a line + /// break inside a cell would otherwise start a new cell or a new row when the text is pasted into a + /// spreadsheet, and the new piece would not carry the leading apostrophe. Mirrors the web grid's Copy row. + /// + public static string NeutralizeClipboardCell(string? text) => Neutralize(CollapseLineBreaks(text)); + + /// + /// Replaces every run of tabs, carriage returns and line feeds with one space. Text with none of them comes + /// back as the same instance; null becomes the empty string. + /// + public static string CollapseLineBreaks(string? text) + { + if (string.IsNullOrEmpty(text)) + { + return string.Empty; + } + + if (text.AsSpan().IndexOfAny('\t', '\r', '\n') < 0) + { + return text; + } + + var sb = new System.Text.StringBuilder(text.Length); + var inRun = false; + foreach (var c in text) + { + if (c is '\t' or '\r' or '\n') + { + if (!inRun) + { + sb.Append(' '); + inRun = true; + } + + continue; + } + + inRun = false; + sb.Append(c); + } + + return sb.ToString(); + } + + /// + /// Neutralizes a typed cell value: a string is passed through ; anything else (a + /// number, a date, a null) is returned as it is, so a numeric cell is never turned into text. + /// + public static object? NeutralizeCell(object? value) => value is string text ? Neutralize(text) : value; + + private static bool IsFormulaStart(char c) => c is '=' or '+' or '-' or '@'; + + /// + /// True for an optional sign followed by digits with optional . and , separators and an + /// optional exponent (e or E, an optional sign, digits), and nothing else. + /// + internal static bool IsPlainNumber(string text) + { + var i = 0; + if (text.Length > 0 && text[0] is '+' or '-') + { + i++; + } + + var digits = 0; + for (; i < text.Length && (char.IsAsciiDigit(text[i]) || text[i] is '.' or ','); i++) + { + if (char.IsAsciiDigit(text[i])) + { + digits++; + } + } + + if (digits == 0) + { + return false; + } + + if (i < text.Length && text[i] is 'e' or 'E') + { + i++; + if (i < text.Length && text[i] is '+' or '-') + { + i++; + } + + var exponentDigits = 0; + for (; i < text.Length && char.IsAsciiDigit(text[i]); i++) + { + exponentDigits++; + } + + if (exponentDigits == 0) + { + return false; + } + } + + return i == text.Length; + } +} diff --git a/PerformanceMonitor.Ui/StandalonePlanViewerController.cs b/PerformanceMonitor.Ui/StandalonePlanViewerController.cs index 79cf41eaf0..a06950170d 100644 --- a/PerformanceMonitor.Ui/StandalonePlanViewerController.cs +++ b/PerformanceMonitor.Ui/StandalonePlanViewerController.cs @@ -301,6 +301,8 @@ public TabItem AddNewEmptyPlanSubTab() try { var xml = System.IO.File.ReadAllText(fileName); + // A refused file opens nothing, so the next file still takes this (empty) tab. + if (CallerPlanXmlPrompt.Refuse(xml)) continue; var targetTab = isFirst ? subTab : AddNewEmptyPlanSubTab(); // Fire-and-forget (open-file, not paste): discard the Task (CS4014 is an error here). _ = LoadPlanIntoSubTab(targetTab, xml, System.IO.Path.GetFileName(fileName)); @@ -332,7 +334,8 @@ public TabItem AddNewEmptyPlanSubTab() if (!string.IsNullOrWhiteSpace(xml)) { // Await so the guard spans the off-thread parse, not just the clipboard read (#2870). - await LoadPlanIntoSubTab(subTab, xml, "Pasted Plan"); + if (!CallerPlanXmlPrompt.Refuse(xml)) + await LoadPlanIntoSubTab(subTab, xml, "Pasted Plan"); return; } MessageBox.Show("The clipboard does not contain any text.", "Paste Plan XML", @@ -475,6 +478,7 @@ public void HandleDrop(DragEventArgs e) try { var xml = System.IO.File.ReadAllText(planFiles[i]); + if (CallerPlanXmlPrompt.Refuse(xml)) continue; // Fire-and-forget (drag-drop, not paste): discard the Task (CS4014 is an error here). _ = LoadPlanIntoSubTab(newTab, xml, System.IO.Path.GetFileName(planFiles[i])); } @@ -508,7 +512,7 @@ public async void HandleKeyDown(KeyEventArgs e) try { var (ok, xml) = await ClipboardText.TryReadAsync(); - if (ok && !string.IsNullOrWhiteSpace(xml)) + if (ok && !string.IsNullOrWhiteSpace(xml) && !CallerPlanXmlPrompt.Refuse(xml)) { // Await so the guard spans the off-thread parse, not just the clipboard read (#2870). await LoadPlanIntoActivePlanSubTab(xml, "Pasted Plan"); @@ -523,6 +527,7 @@ private void LoadPlanFromFileIntoActiveTab(string path) try { var xml = System.IO.File.ReadAllText(path); + if (CallerPlanXmlPrompt.Refuse(xml)) return; // Fire-and-forget (drag-drop, not paste): discard the Task (CS4014 is an error here). _ = LoadPlanIntoActivePlanSubTab(xml, System.IO.Path.GetFileName(path)); } diff --git a/deprecated/Dashboard.Tests/DataGridExportTests.cs b/deprecated/Dashboard.Tests/DataGridExportTests.cs index 6cc38d7cc1..828454a21a 100644 --- a/deprecated/Dashboard.Tests/DataGridExportTests.cs +++ b/deprecated/Dashboard.Tests/DataGridExportTests.cs @@ -102,8 +102,9 @@ public void CsvEscape_QuotesWhenNeeded(string input, string expected) [Fact] public void CsvEscape_QuotesWhenValueContainsSeparator() { - // With a semicolon separator a comma is NOT special, but a semicolon is. - Assert.Equal("a,b", DataGridExport.CsvEscape("a,b", ";")); + // A comma, semicolon or tab is quoted whatever the separator, so a spreadsheet whose list separator + // differs cannot split the value into a new cell. + Assert.Equal("\"a,b\"", DataGridExport.CsvEscape("a,b", ";")); Assert.Equal("\"a;b\"", DataGridExport.CsvEscape("a;b", ";")); }