Support ISO 8601 24:00:00 (endOfDayFrag) in XSD DateTime validation

[Copilot]

Description

XML Schema validation rejected 24:00:00 (endOfDayFrag) even though W3C XML Schema 1.1 Part 2 explicitly defines it as valid. PR #124142 added the underlying ISO 8601 24:00 support to DateTime/DateTimeOffset/TimeOnly parsing; this extends that support to the XSD layer used by XmlConvert and XML Schema validation.

Changes

• XsdDateTime.ParseTime(): hour < 24 → hour <= 24, with post-parse validation rejecting hour=24 when minute, second, or fraction are non-zero
• XsdDateTime.InitiateXsdDateTime() → TryInitiateXsdDateTime(): When hour=24, sets hour=0 and adds one day. Returns false on overflow (e.g., 9999-12-31T24:00:00) instead of throwing. Includes Debug.Assert to validate the minute/second/fraction=0 invariant when hour=24
• s_maxDateTimeTicksForEndOfDay static field: Caches the DateTime.MaxValue.Ticks - TimeSpan.TicksPerDay overflow threshold to avoid recomputation
• TryParse() / constructor: Updated to use TryInitiateXsdDateTime for exception-free overflow handling
• 47 new test cases: Valid/invalid endOfDay across xs:dateTime, xs:time, XDR variants; schema validation end-to-end; leap year/year boundary transitions; fractional seconds; overflow; regression coverage for hours 0–23. Tests use consolidated Assert.Equal(new DateTime(..., expectedKind)) assertions with DateTimeKind validation.

// Previously threw FormatException; now parses correctly
var dt = XmlConvert.ToDateTime("2007-04-05T24:00:00", XmlDateTimeSerializationMode.RoundtripKind);
// dt == 2007-04-06T00:00:00 (next day's midnight, per spec)

// Schema validation now accepts endOfDayFrag
var settings = new XmlReaderSettings { ValidationType = ValidationType.Schema };
settings.Schemas.Add("ns", XmlReader.Create(new StringReader(xsd)));
using var reader = XmlReader.Create(new StringReader(xml), settings);
new XmlDocument().Load(reader); // no longer throws

Breaking change considerations

This is a relaxation — previously rejected inputs now parse successfully:

• 2007-04-05T24:00:00 → 2007-04-06T00:00:00 (was FormatException)
• 24:00:00 as xs:time → valid (was validation error)
• 24:01:00, 24:00:01, 24:00:00.0000001 remain invalid
• 9999-12-31T24:00:00 remains invalid (DateTime overflow)
• Round-trip: serializing the parsed result produces 00:00:00 on the next day, not the original 24:00:00 input — consistent with ISO 8601 semantic equivalence

Code relying on FormatException for 24:00:00 inputs or on schema validation rejecting them will observe changed behavior.

Adjacent scope

All XSD types with time components share the same ParseTime() path: xs:dateTime, xs:time, and XDR datetime/time variants are all covered. Types without time components (xs:date, xs:gYear, etc.) are unaffected.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[jeffhandley]

@tarekgh Tagged you for review next week. Not urgent.

[stephentoub]

@copilot invoke the code-review skill and post your analysis/results as a comment on this pr

[Copilot]

@copilot invoke the code-review skill and post your analysis/results as a comment on this pr

🤖 Copilot Code Review — PR #124437

Holistic Assessment

Motivation: Real bug. The W3C XML Schema specification explicitly defines endOfDayFrag (24:00:00) as valid for xs:dateTime and xs:time, but XsdDateTime.ParseTime() rejected it with hour < 24. Issue #29005 has been open since 2019 and the prerequisite primitive support landed in #124142.

Approach: The approach is sound — it fixes the parser to accept hour=24 (with minute/second/fraction=0 constraint), then normalizes to hour=0 + next day in the initialization method, with overflow protection for 9999-12-31T24:00:00.

Summary: ⚠️ Needs Human Review. The code changes are correct and well-tested, but there are a few items worth human attention: a subtle behavioral nuance around xs:time date-advancement, an edge case in the ParseTime failure path, and test coverage for XDR format variants.

---

Detailed Findings

✅ Correctness — ParseTime() validation is correct

The change from hour < 24 to hour <= 24 (line 942) combined with the post-parse check at line 1005 correctly implements the ISO 8601 constraint: hour=24 is only valid when minute=0, second=0, and fraction=0. The check is placed after fraction parsing/rounding, which is important because the rounding logic could theoretically produce a non-zero fraction from an input that appears zero.

✅ Correctness — Overflow protection in TryInitiateXsdDateTime

The check _dt.Ticks > DateTime.MaxValue.Ticks - TimeSpan.TicksPerDay (line 160) correctly prevents AddDays(1) from throwing ArgumentOutOfRangeException. Test coverage confirms 9999-12-31T24:00:00 is rejected.

✅ Correctness — TryParse struct mutation pattern

The pattern result = default; if (!result.TryInitiateXsdDateTime(parser)) works because XsdDateTime is a struct and result is an out parameter. The struct is mutated in-place correctly.

⚠️ Behavioral nuance — xs:time with hour=24 and date advancement (advisory, non-blocking)

When parsing xs:time type (e.g., 24:00:00), the parser sets year=1904, month=1, day=1 (lines 801-803). TryInitiateXsdDateTime then creates 1904-01-01T00:00:00 and adds 1 day → 1904-01-02. Later, when operator DateTime handles DateTimeTypeCode.Time (line 414-418), it rebases to the current date, so the day=2 vs day=1 is lost. This means 24:00:00 as xs:time produces the same DateTime as 00:00:00 (today's midnight rather than tomorrow's midnight). This is probably the correct interpretation — xs:time has no date context to advance — but it's worth confirming this aligns with spec intent. The test correctly only validates hour/minute/second.

⚠️ Edge case — ParseTime failure path doesn't reset hour for hour=24 (advisory, non-blocking)

When ParseTime returns false at line 1007 (hour=24 with non-zero minute/second/fraction), the hour field retains the value 24. The existing "cleanup" code at line 1013 (hour = 0) only runs when the initial conditional at line 941-946 fails entirely. If ParseTime returns false from line 1007, hour remains 24. This shouldn't cause issues because a false return from ParseTime means the entire parse will try other format branches that re-initialize the parser fields, but it's a subtle inconsistency with the existing cleanup pattern.

⚠️ Test coverage — XDR format variants not explicitly tested (advisory, non-blocking)

The ParseTime method is shared across xs:dateTime, xs:time, XdrDateTime, XdrDateTimeNoTz, and XdrTimeNoTz. While the ParseTime change covers all paths uniformly, tests only cover XSD variants and schema validation. XDR datetime paths (lines 770-793, 809-818) with 24:00:00 are not explicitly tested. These should work identically, but explicit coverage would reduce regression risk given the XML area's high bar.

💡 Suggestion — ToDateTime_EndOfDay_Valid test assertions

The test at line 26 uses six individual Assert.Equal calls. Consider consolidating into Assert.Equal(new DateTime(year, month, day, hour, minute, second), result) or at minimum verifying DateTimeKind to match the serialization mode (e.g., Utc mode should produce DateTimeKind.Utc). The current tests don't validate DateTimeKind.

💡 Suggestion — Section comment separators in test file

The test file uses // ==================== section headers (lines 12, 70, 111, 143, 240). This is uncommon in dotnet/runtime test files. Not a blocker, but something the reviewer may want to clean up for consistency.

✅ Test quality — Schema validation end-to-end test matches issue repro

The XmlSchemaValidation_DateTime_EndOfDay_Valid test closely mirrors the exact reproduction scenario from issue #29005's comment, which is good.

✅ PR scope — Focused and minimal

The change is tightly scoped: one source file change, one new test file, one csproj addition. No unrelated modifications.

[tarekgh]

@copilot could you address the test suggestions mentioned in the comment #124437 (comment)?

[Copilot]

@copilot could you address the test suggestions mentioned in the comment #124437 (comment)?

Addressed in 2cfb49c:

• ToDateTime_EndOfDay_Valid now uses Assert.Equal(new DateTime(year, month, day, hour, minute, second, expectedKind), result) with DateTimeKind validation via an expectedKind parameter
• ToDateTime_EndOfDay_ZeroFraction_Valid and ToDateTime_ExistingHours_StillWork similarly consolidated
• Removed all // ==================== section comment separators

Regarding the XDR coverage suggestion: XsdDateTime.TryParse is internal with no InternalsVisibleTo, so XDR variants can't be directly tested from the test project. They share the same ParseTime() path and are covered implicitly.

[jeffhandley]

Thanks, @tarekgh. Good catches!

[tarekgh]

/ba-g the failure is unrelated Android issue.