dotnet / dotnet/runtime

XmlSerializer: reflection-based reader cannot deserialize into any readonly collection field

Open
#132,357 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`'s reflection-based reader (`ReflectionXmlSerializationReader`) throws when a serializable type declares a collection member as a `readonly` **field**. The IL-emitting reader handles the same type correctly, so this is a parity gap between the two backends rather than an intended restriction.

This is not specific to immutable or read-only collections. It reproduces with a plain `readonly List` field, and it fails whether the corresponding element is present in the XML or absent entirely, which makes such a type wholly undeserializable on that backend.

## Repro

```csharp
public class Holder
{
public readonly List Items = new List();
}
```

Deserializing `1` with the reflection-based reader throws:

```
System.ArgumentException: Expression must be writeable (Parameter 'left')
```

The IL-emitting reader returns `Items` populated with `[1]`, and returns it as an empty list when the element is absent.

Measured matrix (same document, same type shape, both backends):

| Field | XML | ILGen | Reflection |
|---|---|---|---|
| `readonly List F = new()` | present | `[1]` | **throws** |
| `readonly List F = new()` | absent | `[]` | **throws** |
| `readonly List F` (no initializer) | present | `null` | **throws** |
| `readonly ImmutableList F` | present | `null` | **throws** |

## Root cause

`ReflectionXmlSerializationReader.GetSetMemberValueDelegate` builds a setter delegate for every member. When the declaring type is a reference type and dynamic code is available, it takes the expression-tree branch, which eagerly constructs an assignment for the member:

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs#L656-L672

`Expression.Assign` rejects an `initonly` field outright, so the delegate fails to build at all. Note that the failure happens while *constructing* the setter, before anything decides whether the member actually needs to be assigned, which is why an absent element fails too.

The IL-emitting reader never hits this because it does not assign read-only collection members at all. It populates them in place through the getter, which is sufficient: `Models.cs` only admits a read-only member when its kind is `Collection` or `Enumerable`, so any read-only member that reaches a mapping is by construction a collection that can be populated in place.

## Suggested fix

The same method already has a `FieldInfo.SetValue` branch, used when the declaring type is a value type or dynamic code is unavailable. `FieldInfo.SetValue` succeeds on an `initonly` instance field (verified on .NET 10), so routing read-only fields to that branch looks like a small fix.

Whether that is the right behavior is worth a moment's thought, since it would make the reflection reader *assign* where the IL reader only populates in place. Matching the IL reader more literally, by populating through the getter and skipping the assignment, may be the better parity target.

## Notes

- This does **not** reproduce under NativeAOT. `Mode` returns `ReflectionOnly` when `!RuntimeFeature.IsDynamicCodeSupported`, but that same condition sends `GetSetMemberValueDelegate` down the `SetValue` branch, which works.
- It likewise does not reproduce when the declaring type is a struct, for the same reason.
- Found while working on #66264 (read-only collection deserialization support). It is pre-existing and unrelated to that change, so it was deliberately left out of scope there.

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

Contributor guide

Open the contributing guide

Research direction

Start in src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs at GetSetMemberValueDelegate and compare its expression-tree path with the IL-emitting reader. Review Models.cs for the collection-member constraints, then reproduce the readonly List case with present and absent XML; done means the reflection reader no longer throws and matches the IL reader's behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.