From 25cbe5cd3ec2e63bd635be76fcb730083d148d90 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 17:10:58 -0400 Subject: [PATCH] fix(worker): guard timestamp-format derivation against pathological culture patterns --- .../MxAccess/MxAccessEventMapperTests.cs | 49 +++++++++++ .../MxAccess/MxAccessEventMapper.cs | 82 +++++++++++++++++-- 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs index fba52ec..1194d0f 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs @@ -303,6 +303,55 @@ public sealed class MxAccessEventMapperTests Assert.Equal(DateTime.SpecifyKind(localWall, DateTimeKind.Local).ToUniversalTime(), utc); } + /// + /// Verifies format derivation survives a culture whose long-time pattern + /// hides an "ss" inside quoted literal text — the case where blindly + /// inserting ".fff" at the first "ss" produces a format that mangles the + /// value or is outright malformed, and a malformed format makes exact + /// parsing throw rather than report failure. Derivation must reject such + /// a candidate at build time, so parsing here stays exception-free and + /// the culture's own published shape still reads as local wall-clock + /// time. Uses a culture no other test touches, because the format cache + /// compares cultures by name. + /// + [Fact] + public void TryParseSourceTimestamp_WithLiteralSecondsInCulturePattern_ParsesWithoutThrowing() + { + CultureInfo pathological = new("en-ZA") + { + DateTimeFormat = + { + ShortDatePattern = "M/d/yyyy", + LongTimePattern = "'sss' HH:mm:ss", + }, + }; + + CultureInfo original = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = pathological; + DateTime localWall = new(2026, 3, 26, 13, 38, 22, DateTimeKind.Unspecified); + + // The culture's published shape: "3/26/2026 sss 13:38:22". + string published = localWall.ToString( + pathological.DateTimeFormat.ShortDatePattern + " " + pathological.DateTimeFormat.LongTimePattern, + pathological); + + Assert.True(MxAccessEventMapper.TryParseSourceTimestamp(published, out DateTime utc)); + Assert.Equal(DateTimeKind.Utc, utc.Kind); + Assert.Equal(DateTime.SpecifyKind(localWall, DateTimeKind.Local).ToUniversalTime(), utc); + + // Shapes this culture cannot describe must still fail quietly rather + // than throw out of the derived formats. + Assert.False(MxAccessEventMapper.TryParseSourceTimestamp("not a timestamp", out _)); + Assert.False(MxAccessEventMapper.TryParseSourceTimestamp("sss sss sss", out _)); + } + finally + { + CultureInfo.CurrentCulture = original; + } + } + /// /// The parse chain exactly as it stood before PERF-20 added the /// exact-format stage, used as the parity oracle above. diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs index d1958cf..a045bfb 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Globalization; using Google.Protobuf.WellKnownTypes; using ZB.MOM.WW.MxGateway.Contracts.Proto; @@ -15,7 +16,13 @@ public sealed class MxAccessEventMapper /// tries before the general parse /// chain. Keyed by because the format set is /// derived from the culture's own patterns and the worker host's culture - /// is fixed in practice, so the derivation runs once. + /// is fixed in practice, so the derivation runs once. The cache assumes + /// patterns are not customized or + /// mutated at runtime — compares by name, so a + /// culture carrying custom patterns is served the formats derived for its + /// stock namesake. That direction is safe: a stale format simply stops + /// matching, and the unchanged general chain below still parses the + /// string. /// private static readonly ConcurrentDictionary TimestampFormatCache = new(); @@ -397,8 +404,12 @@ public sealed class MxAccessEventMapper const DateTimeStyles styles = DateTimeStyles.AssumeLocal | DateTimeStyles.AdjustToUniversal; CultureInfo culture = CultureInfo.CurrentCulture; + + // Empty when the culture's patterns yielded no format that survived + // validation; that culture just gets no fast path. string[] formats = TimestampFormatCache.GetOrAdd(culture, TimestampFormatFactory); - if (DateTime.TryParseExact(text, formats, culture, styles, out DateTime parsed) + DateTime parsed; + if ((formats.Length > 0 && DateTime.TryParseExact(text, formats, culture, styles, out parsed)) || DateTime.TryParse(text, culture, styles, out parsed) || DateTime.TryParse(text, CultureInfo.InvariantCulture, styles, out parsed)) { @@ -419,25 +430,78 @@ public sealed class MxAccessEventMapper /// .fff after the seconds specifier; a pattern without a /// two-digit seconds specifier, or one that already carries a fraction, /// gets only the published pattern. + /// + /// Both candidates are validated by round-tripping a probe instant + /// before they are handed to the event path, because the derivation + /// reads patterns it does not control: the first ss in a + /// pattern could in principle sit inside quoted literal text, and + /// inserting into it would produce a format that either mangles the + /// value or is outright malformed — and a malformed format makes + /// + /// and its siblings throw rather than + /// report failure. Validation happens here, once per culture, so the + /// event path never carries that risk; a candidate that does not + /// round-trip is dropped, and a culture that loses both simply gets + /// no fast path. + /// /// /// Culture whose patterns the formats are derived from. - /// The formats to try, most likely first. + /// The validated formats to try, most likely first; empty when none survived. private static string[] BuildTimestampFormats(CultureInfo culture) { DateTimeFormatInfo formatInfo = culture.DateTimeFormat; string shortDate = formatInfo.ShortDatePattern; string longTime = formatInfo.LongTimePattern; + List formats = new(2); + int secondsIndex = longTime.IndexOf("ss", StringComparison.Ordinal); if (secondsIndex >= 0 && longTime.IndexOf('f') < 0 && longTime.IndexOf('F') < 0) { - return - [ - shortDate + " " + longTime.Insert(secondsIndex + 2, ".fff"), - shortDate + " " + longTime, - ]; + string withMilliseconds = shortDate + " " + longTime.Insert(secondsIndex + 2, ".fff"); + if (RoundTripsProbe(withMilliseconds, culture, new DateTime(2026, 3, 26, 13, 38, 22, 907))) + { + formats.Add(withMilliseconds); + } } - return [shortDate + " " + longTime]; + string published = shortDate + " " + longTime; + if (RoundTripsProbe(published, culture, new DateTime(2026, 3, 26, 13, 38, 22))) + { + formats.Add(published); + } + + return formats.ToArray(); + } + + /// + /// Reports whether a derived format both formats and re-parses a probe + /// instant back to itself under the given culture. A format that throws, + /// fails to parse its own output, or loses part of the instant is + /// rejected — the fast path may only ever be a shortcut. + /// + /// Candidate format to validate. + /// Culture the format was derived from. + /// Instant to round-trip; carries the precision the format must preserve. + /// when the format is safe to use on the event path. + private static bool RoundTripsProbe( + string format, + CultureInfo culture, + DateTime probe) + { + try + { + string probeText = probe.ToString(format, culture); + return DateTime.TryParseExact(probeText, format, culture, DateTimeStyles.None, out DateTime roundTripped) + && roundTripped == probe; + } + catch (FormatException) + { + return false; + } + catch (ArgumentException) + { + return false; + } } private MxArray ConvertBufferedArray(