From 8a0df25094594497eb3ef6126400b40580f2d24d Mon Sep 17 00:00:00 2001 From: Chris Lange Date: Mon, 11 May 2026 19:34:24 -0600 Subject: [PATCH 1/6] fix(logging-export): CSV escaping, delimiter validation, ticks bounds, no-op progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundles three follow-ups to PR #167 — all flagged by Qodo on the parallel daqifi-python-core port (PR #103) and verified equivalent in Python before being ported here. Closes #191 (no-op progress finalization): - Empty-channel early return now calls progress?.Report(100) so callers (UI progress bars) don't stall at <100% on no-op exports. Closes #192 (DateTime.MinValue marked INVALID): - FormatTimestamp's `ticks <= 0` check rejected ticks=0 even though it's a legal DateTime value (DateTime.MinValue = 0001-01-01 00:00:00). Tightened to `ticks < 0`. Existing test Export_ZeroTicks_WritesInvalidToken updated to assert the new correct format ("0001-01-01T00:00:00..."); the relative-time invalid-tick test switched to a negative tick (-1) since 0 now formats as "0.000". Closes #193 (CSV escaping + delimiter validation, 5-part fix): 1. New EscapeCsvField helper with formulaSafe parameter — RFC 4180 quoting for fields containing the delimiter / `"` / CR / LF, plus formula-injection mitigation that prefixes `'` to fields whose first non-whitespace character is `=`/`+`/`-`/`@`. 2. Delimiter validation rejects empty / multi-char / CR / LF / double-quote (`"` is reserved as the quoting char so allowing it as a delimiter would produce ambiguous CSV). 3. Header writes use formulaSafe=true (channel keys are user- controllable; spreadsheet apps would evaluate "=DevA:..." as a formula on open). 4. Whitespace-prefixed formula chars caught via TrimStart before the first-char check (" =SUM(A1)" still evaluates in spreadsheets, so naive value[0] checks bypass). 5. Data-row writes (timestamps, values) pass formulaSafe=false: internal output, so RFC 4180 quoting still applies (necessary when ':' or '.' is the delimiter and timestamps/floats contain them) but formula mitigation is OFF — otherwise legit negative numbers like "-1.5" would be clobbered into "'-1.5". 13 new tests cover header escaping (delimiter, quote, formula-leading char, whitespace-prefixed formula), data-row escaping (':' delimiter quotes ISO timestamp, '.' delimiter quotes relative time + value), negative-value regression (no leading- apostrophe), invalid-delimiter validation (5 cases parametrized via Theory), and no-op progress finalization. 905 total tests pass on net9.0 + net10.0 (was 892 with 2 failing on the prior buggy assertions). --- .../Logging/Export/CsvExporterTests.cs | 138 +++++++++++++++++- src/Daqifi.Core/Logging/Export/CsvExporter.cs | 91 +++++++++++- 2 files changed, 216 insertions(+), 13 deletions(-) diff --git a/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs b/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs index 469a381..c30c619 100644 --- a/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs +++ b/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs @@ -136,15 +136,20 @@ public async Task Export_RelativeTime_SubSecondPrecision_ThreeDecimalPlaces() // ── Invalid ticks fallback ─────────────────────────────────────────────── [Fact] - public async Task Export_ZeroTicks_WritesInvalidToken() + public async Task Export_ZeroTicks_FormatsAsDateTimeMinValue() { + // ticks==0 is DateTime.MinValue (0001-01-01 00:00:00) — a legal + // DateTime value. Pre-fix, FormatTimestamp's `ticks <= 0` check + // rejected it as INVALID; post-fix, only negative ticks are + // invalid and the formatter renders the absolute timestamp. var source = new InMemorySampleSource( [Ch1], [new SampleRow(0L, Ch1.Key, 1.0)]); var (lines, _) = await ExportToLinesAsync(source, new CsvExportOptions()); - Assert.StartsWith("INVALID(0),", lines[1]); + Assert.StartsWith("0001-01-01T00:00:00", lines[1]); + Assert.DoesNotContain("INVALID", lines[1]); } [Fact] @@ -173,15 +178,18 @@ public async Task Export_TicksBeyondMaxValue_WritesInvalidToken() } [Fact] - public async Task Export_RelativeTime_InvalidTicks_StillWritesInvalidToken() + public async Task Export_RelativeTime_NegativeTicks_StillWritesInvalidToken() { + // Use a genuinely invalid tick value (negative). Post-fix, + // ticks==0 is now valid and would format as "0.000" relative + // seconds, so the prior INVALID(0) expectation no longer holds. var source = new InMemorySampleSource( [Ch1], - [new SampleRow(0L, Ch1.Key, 1.0)]); + [new SampleRow(-1L, Ch1.Key, 1.0)]); var (lines, _) = await ExportToLinesAsync(source, new CsvExportOptions { UseRelativeTime = true }); - Assert.StartsWith("INVALID(0),", lines[1]); + Assert.StartsWith("INVALID(-1),", lines[1]); } // ── Timestamp bucketing ────────────────────────────────────────────────── @@ -528,4 +536,124 @@ public void ExporterTypes_DoNotReferenceEfCoreOrWindows() var _ = new CsvExporter(); Assert.NotNull(_); } + + // ── #191 progress finalization on no-op export ─────────────────────────── + + [Fact] + public async Task Export_NoChannels_StillReports100ProgressOnCompletion() + { + var source = new InMemorySampleSource([], []); + var report = new ListProgress(); + var sw = new StringWriter(); + + await new CsvExporter().ExportAsync(source, sw, new CsvExportOptions(), report); + + Assert.Contains(100, report.Reports); + } + + private sealed class ListProgress : IProgress + { + public List Reports { get; } = new(); + public void Report(T value) => Reports.Add(value); + } + + // ── #193 CSV header escaping ───────────────────────────────────────────── + + [Fact] + public async Task Export_ChannelNameContainingDelimiter_QuotesHeaderField() + { + var ch = new ChannelDescriptor("DevA", "SN001", "name,with,commas", ChannelType.Analog); + var source = new InMemorySampleSource([ch], [new SampleRow(T0, ch.Key, 1.0)]); + var (_, header) = await ExportToLinesAsync(source, new CsvExportOptions()); + Assert.Contains("\"DevA:SN001:name,with,commas\"", header); + } + + [Fact] + public async Task Export_ChannelNameContainingQuote_DoublesAndQuotesField() + { + var ch = new ChannelDescriptor("DevA", "SN001", "name\"with\"quote", ChannelType.Analog); + var source = new InMemorySampleSource([ch], [new SampleRow(T0, ch.Key, 1.0)]); + var (_, header) = await ExportToLinesAsync(source, new CsvExportOptions()); + Assert.Contains("\"DevA:SN001:name\"\"with\"\"quote\"", header); + } + + [Fact] + public async Task Export_DeviceNameStartingWithFormulaChar_GetsLeadingApostrophe() + { + // Channel keys starting with =/+/-/@ would be evaluated as + // formulas by Excel/LibreOffice/Sheets. The mitigation prefixes + // a literal ' to force text mode. + var ch = new ChannelDescriptor("=DevA", "SN001", "Channel1", ChannelType.Analog); + var source = new InMemorySampleSource([ch], [new SampleRow(T0, ch.Key, 1.0)]); + var (_, header) = await ExportToLinesAsync(source, new CsvExportOptions()); + Assert.Contains("'=DevA:SN001:Channel1", header); + } + + [Fact] + public async Task Export_WhitespacePrefixedFormulaChar_StillNeutralized() + { + // " =SUM(A1)" — leading whitespace bypasses a naive value[0] + // check but spreadsheets still interpret it as a formula. + var ch = new ChannelDescriptor(" =DevA", "SN001", "Channel1", ChannelType.Analog); + var source = new InMemorySampleSource([ch], [new SampleRow(T0, ch.Key, 1.0)]); + var (_, header) = await ExportToLinesAsync(source, new CsvExportOptions()); + Assert.Contains("' =DevA:SN001:Channel1", header); + } + + // ── #193 data-row escaping (timestamps + values) ───────────────────────── + + [Fact] + public async Task Export_ColonDelimiter_QuotesIsoTimestamp() + { + // ISO 8601 absolute timestamps inherently contain ':'. With ':' + // chosen as the delimiter, the timestamp field must be RFC 4180 + // quoted so it stays a single CSV field. + var source = new InMemorySampleSource([Ch1], [new SampleRow(T0, Ch1.Key, 1.0)]); + var (lines, _) = await ExportToLinesAsync(source, new CsvExportOptions { Delimiter = ":" }); + // Body row: "2024-...":1 + Assert.StartsWith("\"", lines[1]); + Assert.Contains("\":", lines[1]); + } + + [Fact] + public async Task Export_DotDelimiter_QuotesRelativeTimestampAndValue() + { + var source = new InMemorySampleSource( + [Ch1], + [new SampleRow(T0, Ch1.Key, 0.5), new SampleRow(T0 + TimeSpan.TicksPerSecond, Ch1.Key, 1.5)]); + var (lines, _) = await ExportToLinesAsync( + source, new CsvExportOptions { Delimiter = ".", UseRelativeTime = true }); + // Both relative timestamps and float values contain '.' so both + // get quoted under the '.' delimiter. + Assert.Equal("\"0.000\".\"0.5\"", lines[1]); + Assert.Equal("\"1.000\".\"1.5\"", lines[2]); + } + + [Fact] + public async Task Export_NegativeValue_NotApostrophePrefixed() + { + // Regression: data fields use formulaSafe=false so negative + // numbers (whose leading '-' is a sign, not a formula char) + // aren't clobbered into "'-1.5". + var source = new InMemorySampleSource( + [Ch1], + [new SampleRow(T0, Ch1.Key, -1.5)]); + var (lines, _) = await ExportToLinesAsync(source, new CsvExportOptions { UseRelativeTime = true }); + Assert.Equal("0.000,-1.5", lines[1]); + } + + // ── #193 delimiter validation ──────────────────────────────────────────── + + [Theory] + [InlineData("")] + [InlineData(",,")] + [InlineData("\n")] + [InlineData("\r")] + [InlineData("\"")] + public async Task Export_InvalidDelimiter_ThrowsArgumentException(string bad) + { + var source = new InMemorySampleSource([Ch1], [new SampleRow(T0, Ch1.Key, 1.0)]); + await Assert.ThrowsAsync(async () => + await ExportToLinesAsync(source, new CsvExportOptions { Delimiter = bad })); + } } diff --git a/src/Daqifi.Core/Logging/Export/CsvExporter.cs b/src/Daqifi.Core/Logging/Export/CsvExporter.cs index 9b2d4fd..7cfeb58 100644 --- a/src/Daqifi.Core/Logging/Export/CsvExporter.cs +++ b/src/Daqifi.Core/Logging/Export/CsvExporter.cs @@ -37,11 +37,31 @@ public async Task ExportAsync( options.AverageWindow.Value, $"{nameof(CsvExportOptions.AverageWindow)} must be greater than zero."); + // The double-quote is reserved as the RFC 4180 quoting character used by + // EscapeCsvField, so allowing it as the delimiter would produce ambiguous, + // unparseable output. Newlines would split fields across rows. Multi-char + // / empty delimiters can't be handled by single-character splitting either. + if (string.IsNullOrEmpty(options.Delimiter) + || options.Delimiter.Length != 1 + || options.Delimiter == "\"" + || options.Delimiter == "\r" + || options.Delimiter == "\n") + { + throw new ArgumentException( + $"Delimiter must be a single character that is not a newline or double-quote (got '{options.Delimiter}').", + $"{nameof(options)}.{nameof(CsvExportOptions.Delimiter)}"); + } + cancellationToken.ThrowIfCancellationRequested(); var channels = source.GetChannels(); if (channels.Count == 0) + { + // Always finalize progress so callers (e.g. UI progress bars) don't + // stall at <100% when the export is a no-op. + progress?.Report(100); return; + } var channelKeys = channels.Select(c => c.Key).ToList(); @@ -60,11 +80,11 @@ public async Task ExportAsync( private static async Task WriteHeaderAsync(TextWriter writer, List channelKeys, CsvExportOptions options) { var timeHeader = options.UseRelativeTime ? "Relative Time (s)" : "Time"; - await writer.WriteAsync(timeHeader); + await writer.WriteAsync(EscapeCsvField(timeHeader, options.Delimiter)); foreach (var key in channelKeys) { await writer.WriteAsync(options.Delimiter); - await writer.WriteAsync(key); + await writer.WriteAsync(EscapeCsvField(key, options.Delimiter)); } await writer.WriteLineAsync(); } @@ -116,7 +136,12 @@ private static async Task WriteTimestampRowAsync( { var ticks = bucket[0].TimestampTicks; sb.Clear(); - sb.Append(FormatTimestamp(ticks, firstTicks, options.UseRelativeTime)); + // Data fields use formulaSafe=false: timestamps and numeric values are + // internally generated; their leading '-' is a sign on negative numbers, + // not a formula char. + sb.Append(EscapeCsvField( + FormatTimestamp(ticks, firstTicks, options.UseRelativeTime), + options.Delimiter, formulaSafe: false)); var lookup = new Dictionary(bucket.Count); foreach (var row in bucket) @@ -126,7 +151,9 @@ private static async Task WriteTimestampRowAsync( { sb.Append(options.Delimiter); if (lookup.TryGetValue(key, out var value)) - sb.Append(value.ToString("G", CultureInfo.InvariantCulture)); + sb.Append(EscapeCsvField( + value.ToString("G", CultureInfo.InvariantCulture), + options.Delimiter, formulaSafe: false)); } sb.AppendLine(); @@ -198,13 +225,17 @@ private static async Task WriteAveragedRowAsync( CsvExportOptions options) { sb.Clear(); - sb.Append(FormatTimestamp(lastTick, firstTicks, options.UseRelativeTime)); + sb.Append(EscapeCsvField( + FormatTimestamp(lastTick, firstTicks, options.UseRelativeTime), + options.Delimiter, formulaSafe: false)); foreach (var key in channelKeys) { sb.Append(options.Delimiter); if (counts[key] > 0) - sb.Append((totals[key] / counts[key]).ToString("G", CultureInfo.InvariantCulture)); + sb.Append(EscapeCsvField( + (totals[key] / counts[key]).ToString("G", CultureInfo.InvariantCulture), + options.Delimiter, formulaSafe: false)); } sb.AppendLine(); @@ -214,11 +245,12 @@ private static async Task WriteAveragedRowAsync( /// /// Formats a tick value as an absolute ISO 8601 string or relative seconds string. /// Ticks that are out of the valid range are rendered as INVALID({ticks}) - /// in both modes. + /// in both modes. ticks==0 (DateTime.MinValue, 0001-01-01 00:00:00) is a legal + /// value and IS rendered through the formatter; only negative ticks are invalid. /// private static string FormatTimestamp(long ticks, long firstTicks, bool useRelativeTime) { - if (ticks <= 0 || ticks > DateTime.MaxValue.Ticks) + if (ticks < 0 || ticks > DateTime.MaxValue.Ticks) return $"INVALID({ticks})"; if (useRelativeTime) @@ -227,6 +259,49 @@ private static string FormatTimestamp(long ticks, long firstTicks, bool useRelat return new DateTime(ticks).ToString("O"); } + /// + /// RFC 4180 quoting + optional spreadsheet formula-injection neutralization. + /// + /// The field value to escape. + /// The current CSV delimiter (single character, validated by caller). + /// + /// When true (default — header fields where channel names are user-controlled), + /// prefix a literal ' on values whose first non-whitespace character is + /// =, +, -, or @ so spreadsheet apps don't evaluate + /// the field as a formula. When false (data fields — internally generated + /// timestamps and numeric values), formula mitigation is skipped so legitimate + /// negative numbers like -1.23 aren't clobbered into '-1.23. + /// + /// The escaped field, ready to write between delimiters. + private static string EscapeCsvField(string value, string delimiter, bool formulaSafe = true) + { + if (formulaSafe && !string.IsNullOrEmpty(value)) + { + var leading = value.TrimStart(' ', '\t'); + if (leading.Length > 0 && "=+-@".IndexOf(leading[0]) >= 0) + { + value = "'" + value; + } + } + + var delimChar = delimiter[0]; + var mustQuote = false; + for (var i = 0; i < value.Length; i++) + { + var c = value[i]; + if (c == delimChar || c == '"' || c == '\r' || c == '\n') + { + mustQuote = true; + break; + } + } + if (mustQuote) + { + return "\"" + value.Replace("\"", "\"\"") + "\""; + } + return value; + } + private static void ReportProgress(IProgress? progress, int processed, int total) { if (progress == null || total <= 0) From b1e27f27883f3e3121a23b0b6f9ea8032426129a Mon Sep 17 00:00:00 2001 From: Chris Lange Date: Mon, 11 May 2026 20:42:01 -0600 Subject: [PATCH 2/6] fix: Apply Qodo /improve pass 1 on PR #194: harden formula injection against Unicode whitespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo flagged that the formula-injection check trimmed only ' ' and '\t' before testing for the leading '=', '+', '-', '@'. CSV formula-injection PoCs use NBSP (U+00A0), thin spaces, and Unicode line separators (U+2028) ahead of '=' to evade trim-based checks — spreadsheets still treat the resulting cell as a formula because they normalize / ignore the leading whitespace. Switched the skip loop to char.IsWhiteSpace, which covers the full Unicode whitespace set, closing that bypass class. Skipped a second suggestion to coalesce null inputs to empty — EscapeCsvField is private and current callers always pass strings; adding null tolerance for an impossible case violates the "only validate at boundaries" rule. --- src/Daqifi.Core/Logging/Export/CsvExporter.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Daqifi.Core/Logging/Export/CsvExporter.cs b/src/Daqifi.Core/Logging/Export/CsvExporter.cs index 7cfeb58..72efe68 100644 --- a/src/Daqifi.Core/Logging/Export/CsvExporter.cs +++ b/src/Daqifi.Core/Logging/Export/CsvExporter.cs @@ -277,8 +277,15 @@ private static string EscapeCsvField(string value, string delimiter, bool formul { if (formulaSafe && !string.IsNullOrEmpty(value)) { - var leading = value.TrimStart(' ', '\t'); - if (leading.Length > 0 && "=+-@".IndexOf(leading[0]) >= 0) + // Skip ALL Unicode whitespace, not just ' ' and '\t'. CSV + // formula-injection PoCs use NBSP (U+00A0), thin spaces, line + // separator (U+2028), etc. before '=' to evade trim-based + // checks; spreadsheets still treat the resulting cell as a + // formula. char.IsWhiteSpace covers the full Unicode set. + var i = 0; + while (i < value.Length && char.IsWhiteSpace(value[i])) + i++; + if (i < value.Length && "=+-@".IndexOf(value[i]) >= 0) { value = "'" + value; } From e656b8c73b06861787f87fdb9344deb2359a055c Mon Sep 17 00:00:00 2001 From: Chris Lange Date: Mon, 11 May 2026 20:47:48 -0600 Subject: [PATCH 3/6] test(csv-export): cover Unicode whitespace formula-injection bypass (PR #194 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Locks in the char.IsWhiteSpace upgrade from the previous commit. NBSP (U+00A0) and EM SPACE (U+2003) before '=' previously slipped past the trim-only check; the regression test confirms both now get the leading apostrophe. U+2028 LINE SEPARATOR — also covered by the runtime fix — is omitted from the test data because C# treats raw U+2028 as a source-file newline even inside string literals; the runtime behavior is identical to the other whitespace cases since char.IsWhiteSpace covers U+2028. --- .../Logging/Export/CsvExporterTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs b/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs index c30c619..d5f40b1 100644 --- a/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs +++ b/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs @@ -600,6 +600,22 @@ public async Task Export_WhitespacePrefixedFormulaChar_StillNeutralized() Assert.Contains("' =DevA:SN001:Channel1", header); } + [Theory] + [InlineData("\u00A0")] // NBSP + [InlineData("\u2003")] // EM SPACE + public async Task Export_UnicodeWhitespacePrefixedFormulaChar_StillNeutralized(string whitespace) + { + // Trim-based formula-injection mitigations that only strip ' ' + // and '\t' miss CSV PoCs that prepend NBSP / EM SPACE / line + // separator before '='. char.IsWhiteSpace covers the full + // Unicode whitespace set so the leading apostrophe still lands. + var deviceName = whitespace + "=DevA"; + var ch = new ChannelDescriptor(deviceName, "SN001", "Channel1", ChannelType.Analog); + var source = new InMemorySampleSource([ch], [new SampleRow(T0, ch.Key, 1.0)]); + var (_, header) = await ExportToLinesAsync(source, new CsvExportOptions()); + Assert.Contains("'" + deviceName + ":SN001:Channel1", header); + } + // ── #193 data-row escaping (timestamps + values) ───────────────────────── [Fact] From 904746b922110f03da032e0d6946aaca2dbdee66 Mon Sep 17 00:00:00 2001 From: Chris Lange Date: Tue, 12 May 2026 01:15:07 -0600 Subject: [PATCH 4/6] Apply Qodo /improve: quote CSV fields with leading/trailing whitespace Excel, Google Sheets, and pandas (with default options) trim unquoted leading/trailing whitespace in CSV fields, silently losing the exact value through round-trip. Detect and quote those fields up front so the round-trip preserves the value verbatim. --- .../Logging/Export/CsvExporterTests.cs | 13 ++++++++++ src/Daqifi.Core/Logging/Export/CsvExporter.cs | 24 +++++++++++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs b/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs index d5f40b1..7101f1b 100644 --- a/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs +++ b/src/Daqifi.Core.Tests/Logging/Export/CsvExporterTests.cs @@ -616,6 +616,19 @@ public async Task Export_UnicodeWhitespacePrefixedFormulaChar_StillNeutralized(s Assert.Contains("'" + deviceName + ":SN001:Channel1", header); } + [Fact] + public async Task Export_LeadingTrailingWhitespaceInDeviceName_QuotesField() + { + // Excel, Google Sheets, and pandas trim unquoted leading/trailing + // whitespace in CSV fields; quoting preserves the exact value + // through round-trip parsing. + var deviceName = " DevA "; + var ch = new ChannelDescriptor(deviceName, "SN001", "Channel1", ChannelType.Analog); + var source = new InMemorySampleSource([ch], [new SampleRow(T0, ch.Key, 1.0)]); + var (_, header) = await ExportToLinesAsync(source, new CsvExportOptions()); + Assert.Contains("\" DevA :SN001:Channel1\"", header); + } + // ── #193 data-row escaping (timestamps + values) ───────────────────────── [Fact] diff --git a/src/Daqifi.Core/Logging/Export/CsvExporter.cs b/src/Daqifi.Core/Logging/Export/CsvExporter.cs index 72efe68..7d9118d 100644 --- a/src/Daqifi.Core/Logging/Export/CsvExporter.cs +++ b/src/Daqifi.Core/Logging/Export/CsvExporter.cs @@ -293,15 +293,29 @@ private static string EscapeCsvField(string value, string delimiter, bool formul var delimChar = delimiter[0]; var mustQuote = false; - for (var i = 0; i < value.Length; i++) + + // Quote fields with leading or trailing whitespace — many CSV + // parsers (Excel, Google Sheets, pandas with default options) trim + // unquoted whitespace and silently lose it. Quoting preserves the + // exact value through round-trip. + if (value.Length > 0 && (char.IsWhiteSpace(value[0]) || char.IsWhiteSpace(value[^1]))) + { + mustQuote = true; + } + + if (!mustQuote) { - var c = value[i]; - if (c == delimChar || c == '"' || c == '\r' || c == '\n') + for (var i = 0; i < value.Length; i++) { - mustQuote = true; - break; + var c = value[i]; + if (c == delimChar || c == '"' || c == '\r' || c == '\n') + { + mustQuote = true; + break; + } } } + if (mustQuote) { return "\"" + value.Replace("\"", "\"\"") + "\""; From 6da7050dc47ee1ca3ad1a4dc1aa961a7f73e52ba Mon Sep 17 00:00:00 2001 From: Chris Lange Date: Tue, 12 May 2026 01:17:52 -0600 Subject: [PATCH 5/6] Apply Qodo /improve: format invalid delimiter as code point in error When the delimiter is '\r' or '\n', interpolating it raw into the ArgumentException message produced a multi-line, hard-to-grep error log. Format the bad value as 'null' / 'empty' / 'U+XXXX' so the message stays a single line and unambiguously identifies the bad character. --- src/Daqifi.Core/Logging/Export/CsvExporter.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Daqifi.Core/Logging/Export/CsvExporter.cs b/src/Daqifi.Core/Logging/Export/CsvExporter.cs index 7d9118d..cbce2b6 100644 --- a/src/Daqifi.Core/Logging/Export/CsvExporter.cs +++ b/src/Daqifi.Core/Logging/Export/CsvExporter.cs @@ -47,8 +47,16 @@ public async Task ExportAsync( || options.Delimiter == "\r" || options.Delimiter == "\n") { + // Format the bad value as a code point so a stray '\r' or '\n' + // in the delimiter doesn't break the exception message into + // multiple log lines. + var got = options.Delimiter == null + ? "null" + : options.Delimiter.Length == 0 + ? "empty" + : $"U+{(int)options.Delimiter[0]:X4}"; throw new ArgumentException( - $"Delimiter must be a single character that is not a newline or double-quote (got '{options.Delimiter}').", + $"Delimiter must be a single character that is not a newline or double-quote (got {got}).", $"{nameof(options)}.{nameof(CsvExportOptions.Delimiter)}"); } From 2f772841af29b3fa4cbaa4a49f87b421eb7eba6c Mon Sep 17 00:00:00 2001 From: Chris Lange Date: Tue, 12 May 2026 13:04:34 -0600 Subject: [PATCH 6/6] Apply Qodo /agentic_review pass 4: full-delimiter diagnostic in error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enumerate every code point of the invalid delimiter (with its length) in the ArgumentException detail string instead of reporting only the first character. Multi-character delimiters like ",," were misreported as a single "U+002C" — the new format reads len=2 [U+002C U+002C] and stays log-safe (no raw CR/LF interpolated). --- src/Daqifi.Core/Logging/Export/CsvExporter.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/Daqifi.Core/Logging/Export/CsvExporter.cs b/src/Daqifi.Core/Logging/Export/CsvExporter.cs index cbce2b6..893b151 100644 --- a/src/Daqifi.Core/Logging/Export/CsvExporter.cs +++ b/src/Daqifi.Core/Logging/Export/CsvExporter.cs @@ -1,4 +1,5 @@ using System.Globalization; +using System.Linq; using System.Text; namespace Daqifi.Core.Logging.Export; @@ -47,14 +48,27 @@ public async Task ExportAsync( || options.Delimiter == "\r" || options.Delimiter == "\n") { - // Format the bad value as a code point so a stray '\r' or '\n' + // Format the bad value as code points so a stray '\r' or '\n' // in the delimiter doesn't break the exception message into - // multiple log lines. - var got = options.Delimiter == null - ? "null" - : options.Delimiter.Length == 0 - ? "empty" - : $"U+{(int)options.Delimiter[0]:X4}"; + // multiple log lines. Enumerate ALL chars (not just [0]) so + // multi-character delimiters like ",," aren't misreported as a + // single character. + string got; + if (options.Delimiter == null) + { + got = "null"; + } + else if (options.Delimiter.Length == 0) + { + got = "empty"; + } + else + { + var codePoints = string.Join( + " ", + options.Delimiter.Select(c => $"U+{(int)c:X4}")); + got = $"len={options.Delimiter.Length} [{codePoints}]"; + } throw new ArgumentException( $"Delimiter must be a single character that is not a newline or double-quote (got {got}).", $"{nameof(options)}.{nameof(CsvExportOptions.Delimiter)}");