CSHARP-6043: NullReferenceException in IEnumerableSerializerBase.Serialize when projecting nullable array with conditional Select#2012
Conversation
…alize when projecting nullable array with conditional Select
There was a problem hiding this comment.
Pull request overview
Fixes null handling in Linq3 enumerable serialization to prevent NullReferenceException when serializing projections that can evaluate to null (e.g., nullable arrays under conditional Select), and adds targeted regression tests.
Changes:
- Handle BSON
nullinIEnumerableSerializerBase.Deserializeby consumingnulland returningdefault. - Handle
nullvalues inIEnumerableSerializerBase.Serializeby writing BSONnullinstead of iterating. - Add unit tests verifying
nullserialize/deserialize behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Serializers/IEnumerableSerializerBaseTests.cs | Adds regression tests ensuring null enumerables round-trip as BSON null. |
| src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/IEnumerableSerializerBase.cs | Adds explicit BSON null handling for serialize/deserialize paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public override void Serialize(BsonSerializationContext context, BsonSerializationArgs args, TEnumerable value) | ||
| { | ||
| var writer = context.Writer; | ||
| if (value == null) | ||
| { | ||
| writer.WriteNull(); | ||
| return; | ||
| } |
BorisDog
left a comment
BorisDog
left a comment
There was a problem hiding this comment.
LGTM + question about testing.
| var result = serializer.Deserialize(context, new BsonDeserializationArgs { NominalType = typeof(IEnumerable<int>) }); | ||
|
|
||
| result.Should().BeNull(); | ||
| } |
There was a problem hiding this comment.
Would be good to have some limited end-to-end testing. By "e2e" here I mean serializing and deserializing a real POCO, using a real expression (without DB).
But not sure where we place such tests now, any ideas @sanych-sun?
There was a problem hiding this comment.
It probably should be a Theory with bytes on one side and expected value on the another. We might need such for all serializers.
The NRE happened during LINQ3 query translation when the translator serialized the null constant from the ternary's true branch through a
IEnumerableSerializerBase.Serializethat didn't expect null.The BSON-layer
EnumerableSerializerBasealready handles null but the LINQ path uses its own dedicatedIEnumerableSerializerBaseand that one was missing the null guards. So the fix is to bring the LINQ path serializer in par with the BSON layer one.