dotnet / dotnet/runtime

XmlSerializer: LIFO collections would round-trip inverted; element order needs deciding before support lands

Open
#132,761 0 comments 0 reactions 0 assignees View on GitHub
area-Serialization
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

(Found when investigating PR #132356 for issue #66264 ...)

## Description

`XmlSerializer` writes a collection by enumerating it, and reads one back by appending elements in document order. Those two operations are inverses only when enumeration order matches insertion order. For a LIFO collection it does not: a stack hands its newest element to an enumerator first, so writing a stack out and reading it back returns an inverted stack.

There is no way today for `XmlSerializer` to know that a collection has LIFO semantics, and no decision on record about what order such a collection should be written in.

## Nothing is broken today, which is exactly why this is worth deciding now

No LIFO collection in the BCL is serializable by `XmlSerializer` at all:

| Type | Why it is rejected |
| --- | --- |
| `Stack` | implements non-generic `ICollection` but has no default indexer, so `XmlNoDefaultAccessors` |
| `ConcurrentStack` | same |
| `ImmutableStack` | not `ICollection`, and has no `Add`, so `XmlNoAddMethod` |

Each of those rejections is an accident of the type's shape rather than a deliberate stance on ordering. The useful consequence is a free hand: because no LIFO type round-trips today, whichever order is chosen when one first becomes serializable is a greenfield decision with no compatibility cost. That window closes the moment support lands.

## How it came up

#132356 adds deserialization support for read-only collections through `[CollectionBuilder]`. `ImmutableStack` carries that attribute (on `IImmutableStack`), so it would have become serializable as a side effect: `ImmutableStack.Create(1, 2, 3)` writes `3,2,1`, and reading that back produces a stack with 1 on top. Rather than ship an order that could not be corrected later, that PR deliberately refuses `IImmutableStack` so these types stay unsupported. **That block should be removed as part of resolving this issue.**

## The two candidate fixes

**Reverse the accumulated elements on read.** This was implemented in #132356 and then rejected. It makes the same document mean two different things depending on which direction it is read: `321` means "3, then 2, then 1" when written but "1 ends up on top" when read. It also requires detecting LIFO-ness, and the only available signal is an interface name; a wrong match silently reorders user data instead of failing.

**Write LIFO collections in construction order instead.** This is the preferred direction. `ImmutableStack.Create(1, 2, 3)` puts 3 on top, so writing `1,2,3` and reading it back through that same builder reconstructs an identical stack, and the document order carries the same meaning in both directions. It also matches what a C# collection expression or an initializer array would look like for the same value.

## What is needed

A way for a type to declare that its enumeration order is the reverse of its construction order. Options worth weighing:

- an attribute on the collection type
- a curated set of known types held inside `System.Private.Xml`
- ordering metadata on `[CollectionBuilder]` itself, if that is something the broader collection-builder feature would want

A third-party collection whose builder reverses its input cannot be detected under any of these unless it opts in, and would round-trip inverted. That seems acceptable so long as opting in is possible.

## Prior art

#83086 is the same defect in `System.Text.Json`, for `Stack`. It is open with milestone Future because STJ shipped the inversion and is now constrained by compatibility. Avoiding that outcome is the main argument for settling this before support lands rather than after.

## Other information

Related: #66264 (the read-only collection feature request), #132356 (the implementation, on hold for this release).

> [!NOTE]
> This issue was drafted with GitHub Copilot.

Contributor guide

Open the contributing guide

Research direction

Start with PR #132356 and the XmlSerializer collection-handling entry points, focusing on its [CollectionBuilder] support and the deliberate IImmutableStack rejection. Compare the proposed ordering options, decide how LIFO construction order is declared, remove the block that keeps IImmutableStack unsupported, and verify that ImmutableStack.Create(1, 2, 3) round-trips without inversion.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.