Round-trip TimeZoneInfo.ToSerializedString on Unix (#19794)#131352
Draft
tarekgh wants to merge 1 commit into
Draft
Round-trip TimeZoneInfo.ToSerializedString on Unix (#19794)#131352tarekgh wants to merge 1 commit into
tarekgh wants to merge 1 commit into
Conversation
TimeZoneInfo.ToSerializedString/FromSerializedString did not round-trip on Unix because the custom string format only represents the Windows-shaped public projection of the adjustment rules. Unix rules can carry information the legacy format cannot express (NoDaylightTransitions rules, UTC sub-day boundaries, sub-minute BaseUtcOffsetDelta, or multi-year rules that get split), so the deserialized instance did not match the original. Append an optional full-fidelity copy of the internal adjustment rules after the separator that terminates the legacy rule list. Older readers stop at that separator and never see the trailer, so existing serialized strings and Windows output are unchanged and remain valid. A modern reader consumes the trailer and reconstructs the exact internal rules; when the trailer is absent or written with an unknown future version, it falls back to the legacy rules. Also make the legacy portion self-consistent when the trailer is ignored: order the whole-day legacy boundaries so consecutive rules never overlap, and write BaseUtcOffsetDelta in whole minutes since the legacy reader rejects a fractional value. The trailer count and rule ticks are untrusted input, so bound the rule count against the remaining input length and translate out-of-range or overflowing tick values into SerializationException.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates TimeZoneInfo string serialization to support full-fidelity round-tripping of Unix time zone adjustment rules by appending an optional, versioned trailer after the legacy rule-list terminator, while keeping the legacy portion backward-compatible for older readers.
Changes:
- Extend
TimeZoneInfo.ToSerializedStringto optionally emit a full-fidelity trailer of internal adjustment rules when the legacy (Windows-shaped) projection would be lossy. - Extend
TimeZoneInfo.FromSerializedStringto detect and consume the trailer (when present and recognized) and fall back to legacy rules otherwise. - Re-enable and expand
TimeZoneInfoTestscoverage for Unix round-tripping and trailer parsing/validation scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.StringSerializer.cs | Adds versioned full-fidelity trailer serialization/deserialization and keeps legacy output parseable for older runtimes. |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs | Re-enables Unix round-trip test and adds targeted tests for trailer emission, parsing, and malformed input handling. |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #19794
Problem
TimeZoneInfo.ToSerializedString/FromSerializedStringdid not round-trip on Unix. The custom string format only represents the Windows-shaped public projection of the adjustment rules (the output ofGetAdjustmentRules()). Unix rules can carry information that projection cannot express, so the deserialized instance did not equal the original. Lossy cases include:NoDaylightTransitionsset,DateStart/DateEndwithDateTimeKind.Utcand sub-day boundaries,BaseUtcOffsetDelta,Fix
Append an optional full-fidelity copy of the internal adjustment rules after the separator that terminates the legacy rule list. This placement is backward compatible: older readers stop at that separator and never see the trailer, so previously serialized strings and Windows output are unchanged and remain valid.
The legacy portion is also kept self-consistent for readers that ignore the trailer:
BaseUtcOffsetDeltais written in whole minutes, because the legacy reader rejects a fractional value.The trailer count and rule ticks are untrusted input, so the rule count is bounded against the remaining input length, and out-of-range or overflowing tick values are translated into
SerializationException.Testing
Added and re-enabled tests in
TimeZoneInfoTests:BaseUtcOffsetDelta,SerializationException,Validated on Windows (618 tests) and Linux/WSL (2044 tests); all pass.