Skip to content

Commit beb7336

Browse files
committed
fix(xml): XmlConvert's TimeSpan pair uses the XSD duration form (#2080, SR-AUD-354)
XmlConvert::ToString(TimeSpan) emitted .NET's NATIVE colon form -- "1.00:00:00.0000000" for one day -- and ToTimeSpan parsed it. .NET's are `new XsdDuration(value).ToString()` and `new XsdDuration(s).ToTimeSpan()`, and never look at the colon form. The deferral named three sub-questions with no repository-contained answer. The reference answers all three. (1) The year/month factors: XsdDuration ESTIMATES -- 365 days to the year, 30 to the month -- and says so in as many words. Reproduced rather than improved on: a "better" estimate would disagree with every .NET-produced value. P12M rounds to a year, because the algorithm is (years + months/12)*365 + (months%12)*30. (2) Does the colon form stay accepted: no. The parser requires a leading P. Accepting both would make the contract "either grammar", which no reference or schema defines. (3) The exception: FormatException, always, because XmlConvert.ToTimeSpan wraps every XsdDuration failure -- including its own OverflowException -- in one. Two things surprised the first cut and are recorded rather than smoothed over. "PT.5S" is VALID: .NET's '.' branch is the one component that does not check for preceding digits, and XML Schema's duSecondFrag admits ('.' fracFrag) -- the test was wrong, not the parser. And one line of the reference is DEAD: XsdDuration.TryParse's trailing `if (numDigits != 0)`, which every reaching path has already made unreachable. The port omits it, and a mutation confirmed that changes nothing. ToString and ToTimeSpan round-trip, asserted over eight values including negatives, zero and a single tick. Two #1836 tests are rewritten in the new grammar rather than deleted, because the property they pin -- an out-of-range day count raises rather than wrapping negative -- is unchanged. +3 net tests. Five mutations: three caught directly, one re-run after an invalid first cut, one that led to deleting the dead line. Gate: 17,265 run, 0 failed, 38 executables -- green. Downstream, measured: zero XmlConvert sites in either consumer. SR-AUD-354 -> remediated. Record: docs/Migration-XmlConvertXsdDuration.md.
1 parent b9346ee commit beb7336

7 files changed

Lines changed: 437 additions & 47 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

audit/AUDIT_FINDINGS_INDEX.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — `XmlConvert` TimeSpan uses the XSD `duration` form (ticket #2080)
5+
6+
*2026-08-17.* `XmlConvert::ToString(TimeSpan)` emitted .NET's **native colon form** and
7+
`XmlConvert::ToTimeSpan` parsed it. .NET's use the XML Schema `duration` lexical form and never
8+
look at the colon form at all.
9+
10+
Landed under `docs/StandingApprovals.md` SA-5. No public signature, layout, vtable or `noexcept`
11+
change. **This changes emitted text and narrows accepted input** — read §1.
12+
13+
---
14+
15+
## 1. What changed
16+
17+
| Call | Was | Is |
18+
|---|---|---|
19+
| `ToString(TimeSpan::FromDays(1))` | `"1.00:00:00.0000000"` | `"P1D"` |
20+
| `ToString(TimeSpan::FromTicks(0))` | `"00:00:00"` | `"PT0S"` |
21+
| `ToString(TimeSpan::FromMinutes(90))` | `"01:30:00"` | `"PT1H30M"` |
22+
| `ToString(TimeSpan::FromDays(-1))` | `"-1.00:00:00"` | `"-P1D"` |
23+
| `ToTimeSpan("P1D")`, `ToTimeSpan("PT1H30M")` | `FormatException` | parsed |
24+
| `ToTimeSpan("1.00:00:00")` | parsed | **`FormatException`** |
25+
| `ToTimeSpan("PT.5S")` | `FormatException` | 0.5 s — see §3 |
26+
27+
`ToString` and `ToTimeSpan` round-trip, and a test asserts that over eight values including
28+
negatives, zero and a single tick.
29+
30+
## 2. Why, and the three questions the deferral could not answer
31+
32+
The deferral listed exactly three sub-questions with no repository-contained answer. The
33+
reference answers all three:
34+
35+
* **The year/month conversion factors.** `XsdDuration` *estimates* — 365 days to the year, 30 to
36+
the month — and says so in as many words (`XsdDuration.cs:243-244`). Reproduced rather than
37+
improved on: a "better" estimate would disagree with every .NET-produced value. Note `P12M`
38+
rounds to a year, because the algorithm is `(years + months/12) * 365 + (months % 12) * 30`.
39+
* **Whether the native colon form stays accepted.** It does **not**. `XsdDuration`'s parser
40+
requires a leading `P`, and `XmlConvert.ToTimeSpan` goes straight to it
41+
(`XmlConvert.cs:1109-1127`). Accepting both would make the method's contract "either
42+
grammar", which no reference or schema defines.
43+
* **The exception identity.** `FormatException`, always. `XmlConvert.ToTimeSpan` wraps *every*
44+
`XsdDuration` failure — including its `OverflowException` — in one, with the comment "Remap
45+
exception for v1 compatibility" (`:1118-1122`).
46+
47+
## 3. Two details that surprised the first cut
48+
49+
**`PT.5S` is valid.** .NET's `'.'` branch is the one component that does **not** check whether
50+
any digits preceded it, and XML Schema agrees: `duSecondFrag` admits `('.' fracFrag)` with no
51+
leading digit. The first cut of the test listed `PT.5S` as malformed; the expectation was wrong,
52+
not the parser, and there is now a row recording that so it is not "fixed" later.
53+
54+
**One line of the reference is dead.** `XsdDuration.TryParse` ends with
55+
`if (numDigits != 0) goto InvalidFormat;` before its "no trailing characters" check. Every path
56+
that reaches it has already rejected `pos >= length`, so `pos < length` holds and the next check
57+
rejects regardless. The port omits it, and a mutation confirmed the omission changes nothing.
58+
59+
## 4. Two tests from ticket #1836 are rewritten, not deleted
60+
61+
`#1836` pinned that an out-of-range day count *raises* rather than wrapping to a negative
62+
duration. That property is unchanged and still asserted — in the new grammar, and with
63+
`FormatException` rather than `OverflowException`, which is .NET's own remapping.
64+
65+
## 5. To migrate
66+
67+
If you write XML, this is the fix: `xs:duration` is what a schema-validating consumer expects,
68+
and the colon form was never valid there.
69+
70+
If you were round-tripping through these two methods, you still are — the pair is consistent.
71+
72+
If you were feeding `XmlConvert::ToTimeSpan` a colon-form string from elsewhere, use
73+
`System::TimeSpan::Parse`, which is the method for that grammar and is unchanged.
74+
75+
## 6. Downstream, measured
76+
77+
Neither `cna` nor `mobile-eggbert` references `XmlConvert` or `System::Xml`**zero sites in
78+
both**. Neither repository was modified.

0 commit comments

Comments
 (0)