Skip to content

Commit 34c3fee

Browse files
Copilotstephentoubeiriktsarpalis
authored
Fix JsonSerializer.Serialize producing invalid JSON for [JsonExtensionData] with JsonObject (#122838)
# Description `JsonSerializer.Serialize` was producing invalid JSON when serializing objects with `[JsonExtensionData]` properties of type `JsonObject`. The output was missing the property name for extension data entries: ```csharp var mix = new Mix { Id = 1, Extra = new JsonObject { ["nested"] = true } }; JsonSerializer.Serialize(mix); // Before: {"Id":1,{"nested":true}} ← invalid JSON // After: {"Id":1,"nested":true} ← correct ``` **Root cause**: `TryWriteDataExtensionProperty` in `JsonConverterOfT.cs` called `TryWrite` for `JsonObject`, which writes the full object including braces. Extension data should write only the contents (key-value pairs) without wrapping braces. **Fix**: - Added a new internal `WriteContentsTo` method to `JsonObject` that writes properties without wrapping braces, handling both dictionary-backed and `JsonElement`-backed cases with proper optimization. - Added a `WriteExtensionDataValue` virtual method to `JsonConverter<T>` with a generic `T value` parameter that throws by default, overridden in `JsonObjectConverter` to call `WriteContentsTo`. This follows the same pattern as `ReadElementAndSetProperty` and avoids direct type references to `JsonObject` in `JsonConverterOfT.cs` to maintain trimming support. - Updated `TryWriteDataExtensionProperty` to call the virtual method instead of directly referencing `JsonObject`. - Retained a `Debug.Assert` for the `JsonObject` type check using a qualified `Nodes.JsonObject` reference (debug assertions are removed in release builds, so this doesn't affect trimming). # Customer Impact Serialization of any class with `[JsonExtensionData]` of type `JsonObject` produces malformed JSON that cannot be parsed. # Regression No. Reproducible from .NET 6 through .NET 8+. Deserialization works correctly; only serialization was affected. # Testing - Added 4 targeted tests covering: valid JSON output, round-trip, empty `JsonObject`, and null `JsonObject` - All 49,808 existing `System.Text.Json.Tests` pass - All 7,626 source generation tests pass - Registered new test class in source generator contexts (`ExtensionDataTestsContext_Metadata` and `ExtensionDataTestsContext_Default`) # Risk Low. Change is minimal and isolated to the `JsonObject` branch of extension data serialization. Dictionary extension data path is unchanged. The virtual method pattern maintains trimming support by avoiding direct type references in non-debug code paths. # Package authoring no longer needed in .NET 9 IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>JsonSerializer.Serialize produces invalid JSON for [JsonExtensionData] property</issue_title> > <issue_description>### Description > > When serializing object containing property marked with `[JsonExtensionData]` attribute, serializer produces invalid JSON like this one: `{"Id":1,{"nested":true}}` > > ### Reproduction Steps > > Run the following code > > ```C# > var mix = new Mix > { > Id = 1, > Extra = new() { ["nested"] = true, } > }; > > var text = System.Text.Json.JsonSerializer.Serialize(mix); > Console.WriteLine(text); > // output {"Id":1,{"nested":true}} > > > public class Mix > { > public int Id { get; set; } > > [System.Text.Json.Serialization.JsonExtensionData] > public System.Text.Json.Nodes.JsonObject? Extra { get; set; } > } > > ``` > > ### Expected behavior > > Correct JSON like `{"Id":1,"nested":true}` or at least valid JSON as if there was not `[JsonExtensionData]` attribute (`{"Id":1,"Extra":{"nested":true}}` ). > > ### Actual behavior > > Invalid JSON `{"Id":1,{"nested":true}}` > > ### Regression? > > Reproducible at least on .Net 6 to .Net 8 > > ### Known Workarounds > > _No response_ > > ### Configuration > > .Net 8 > Windows 11 > x64 > > ### Other information > > _No response_</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@</author><body> > Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis > See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed. > <details> > <summary>Issue Details</summary> > <hr /> > > ### Description > > When serializing object containing property marked with `[JsonExtensionData]` attribute, serializer produces invalid JSON like this one: `{"Id":1,{"nested":true}}` > > ### Reproduction Steps > > Run the following code > > ```C# > var mix = new Mix > { > Id = 1, > Extra = new() { ["nested"] = true, } > }; > > var text = System.Text.Json.JsonSerializer.Serialize(mix); > Console.WriteLine(text); > // output {"Id":1,{"nested":true}} > > > public class Mix > { > public int Id { get; set; } > > [System.Text.Json.Serialization.JsonExtensionData] > public System.Text.Json.Nodes.JsonObject? Extra { get; set; } > } > > ``` > > ### Expected behavior > > Correct JSON like `{"Id":1,"nested":true}` or at least valid JSON as if there was not `[JsonExtensionData]` attribute (`{"Id":1,"Extra":{"nested":true}}` ). > > ### Actual behavior > > Invalid JSON `{"Id":1,{"nested":true}}` > > ### Regression? > > Reproducible at least on .Net 6 to .Net 8 > > ### Known Workarounds > > _No response_ > > ### Configuration > > .Net 8 > Windows 11 > x64 > > ### Other information > > _No response_ > > <table> > <tr> > <th align="left">Author:</th> > <td>KalininAndreyVictorovich</td> > </tr> > <tr> > <th align="left">Assignees:</th> > <td>-</td> > </tr> > <tr> > <th align="left">Labels:</th> > <td> > > `area-System.Text.Json` > > </td> > </tr> > <tr> > <th align="left">Milestone:</th> > <td>-</td> > </tr> > </table> > </details></body></comment_new> > <comment_new><author>@eiriktsarpalis</author><body> > Can confirm that this occurs. It seems we never added testing for the serialization scenario -- deserialization appears to be working as expected.</body></comment_new> > <comment_new><author>@eiriktsarpalis</author><body> > The documentation is correctly stating that `JsonObject` is one of the supported types. However there is a bug specifically impacting serialization for the particular type (FWIW `JsonExtensionData` is a feature primarily oriented towards dserialization).</body></comment_new> > <comment_new><author>@eiriktsarpalis</author><body> > You're right, I misread the documentation which appears to be out of date. The correct statement on supported types can actually be found in the error messages of the implementation itself: > > https://github.com/dotnet/runtime/blob/2d751cac7311b344c237df3b3c63b33434b52217/src/libraries/System.Text.Json/gen/Resources/Strings.resx#L150-L152 > > In other words, what I mentioned earlier holds. `JsonObject` _is_ supported and there is a bug specifically concerning serialization.</body></comment_new> > <comment_new><author>@eiriktsarpalis</author><body> > It would help if you could file a separate issue in dotnet-api-docs. Thanks!</body></comment_new> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #97225 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> Co-authored-by: Stephen Toub <stoub@microsoft.com> Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
1 parent 1492e43 commit 34c3fee

5 files changed

Lines changed: 142 additions & 3 deletions

File tree

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,30 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio
172172
else
173173
{
174174
writer.WriteStartObject();
175+
WriteContentsTo(writer, options);
176+
writer.WriteEndObject();
177+
}
178+
}
175179

180+
/// <summary>
181+
/// Writes the properties of this JsonObject to the writer without the surrounding braces.
182+
/// This is used for extension data serialization where the properties should be flattened
183+
/// into the parent object.
184+
/// </summary>
185+
internal void WriteContentsTo(Utf8JsonWriter writer, JsonSerializerOptions? options)
186+
{
187+
GetUnderlyingRepresentation(out OrderedDictionary<string, JsonNode?>? dictionary, out JsonElement? jsonElement);
188+
189+
if (dictionary is null && jsonElement.HasValue)
190+
{
191+
// Write properties from the underlying JsonElement without converting to nodes.
192+
foreach (JsonProperty property in jsonElement.Value.EnumerateObject())
193+
{
194+
property.WriteTo(writer);
195+
}
196+
}
197+
else
198+
{
176199
foreach (KeyValuePair<string, JsonNode?> entry in Dictionary)
177200
{
178201
writer.WritePropertyName(entry.Key);
@@ -186,8 +209,6 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio
186209
entry.Value.WriteTo(writer, options);
187210
}
188211
}
189-
190-
writer.WriteEndObject();
191212
}
192213
}
193214

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ internal override void ReadElementAndSetProperty(
4141
}
4242
}
4343

44+
internal override void WriteExtensionDataValue(Utf8JsonWriter writer, JsonObject? value, JsonSerializerOptions options)
45+
{
46+
Debug.Assert(value is not null);
47+
value.WriteContentsTo(writer, options);
48+
}
49+
4450
public override void Write(Utf8JsonWriter writer, JsonObject? value, JsonSerializerOptions options)
4551
{
4652
if (value is null)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,11 @@ internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, Json
458458
{
459459
// If not JsonDictionaryConverter<T> then we are JsonObject.
460460
// Avoid a type reference to JsonObject and its converter to support trimming.
461+
// The WriteExtensionDataValue virtual method is overridden by the JsonObject converter.
461462
Debug.Assert(Type == typeof(Nodes.JsonObject));
462-
return TryWrite(writer, value, options, ref state);
463+
WriteExtensionDataValue(writer, value!, options);
464+
465+
return true;
463466
}
464467

465468
if (writer.CurrentDepth >= options.EffectiveMaxDepth)
@@ -493,6 +496,19 @@ internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, Json
493496
return success;
494497
}
495498

499+
/// <summary>
500+
/// Used to support JsonObject as an extension property in a loosely-typed, trimmable manner.
501+
/// </summary>
502+
/// <remarks>
503+
/// Writes the extension data contents without wrapping object braces.
504+
/// </remarks>
505+
internal virtual void WriteExtensionDataValue(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
506+
{
507+
Debug.Fail("Should not be reachable.");
508+
509+
throw new InvalidOperationException();
510+
}
511+
496512
/// <inheritdoc/>
497513
public sealed override Type Type { get; } = typeof(T);
498514

src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,100 @@ public async Task DeserializeIntoJsonObjectProperty()
815815
Assert.Equal(1, obj.MyOverflow["MyDict"]["Property1"].GetValue<int>());
816816
}
817817

818+
[Fact]
819+
public async Task SerializeJsonObjectExtensionData_ProducesValidJson()
820+
{
821+
// JsonSerializer.Serialize was producing invalid JSON for [JsonExtensionData] property of type JsonObject
822+
// Output was: {"Id":1,{"nested":true}} instead of {"Id":1,"nested":true}
823+
824+
var obj = new ClassWithJsonObjectExtensionDataAndProperty
825+
{
826+
Id = 1,
827+
Extra = new JsonObject { ["nested"] = true }
828+
};
829+
830+
string json = await Serializer.SerializeWrapper(obj);
831+
832+
// Verify the JSON is valid by parsing it
833+
using JsonDocument doc = JsonDocument.Parse(json);
834+
835+
// Verify the structure is correct: extension data should be flattened, not nested
836+
Assert.True(doc.RootElement.TryGetProperty("Id", out JsonElement idElement));
837+
Assert.Equal(1, idElement.GetInt32());
838+
839+
Assert.True(doc.RootElement.TryGetProperty("nested", out JsonElement nestedElement));
840+
Assert.True(nestedElement.GetBoolean());
841+
842+
// Extension data property name should NOT appear in the output
843+
Assert.False(doc.RootElement.TryGetProperty("Extra", out _));
844+
845+
// Verify expected JSON format
846+
Assert.Equal(@"{""Id"":1,""nested"":true}", json);
847+
}
848+
849+
[Fact]
850+
public async Task SerializeJsonObjectExtensionData_RoundTrip()
851+
{
852+
var original = new ClassWithJsonObjectExtensionDataAndProperty
853+
{
854+
Id = 42,
855+
Extra = new JsonObject
856+
{
857+
["string"] = "value",
858+
["number"] = 123,
859+
["boolean"] = false,
860+
["nested"] = new JsonObject { ["inner"] = "data" }
861+
}
862+
};
863+
864+
string json = await Serializer.SerializeWrapper(original);
865+
866+
// Verify round-trip
867+
var deserialized = await Serializer.DeserializeWrapper<ClassWithJsonObjectExtensionDataAndProperty>(json);
868+
869+
Assert.Equal(original.Id, deserialized.Id);
870+
Assert.NotNull(deserialized.Extra);
871+
Assert.Equal(4, deserialized.Extra.Count);
872+
Assert.Equal("value", deserialized.Extra["string"]!.GetValue<string>());
873+
Assert.Equal(123, deserialized.Extra["number"]!.GetValue<int>());
874+
Assert.False(deserialized.Extra["boolean"]!.GetValue<bool>());
875+
Assert.Equal("data", deserialized.Extra["nested"]!["inner"]!.GetValue<string>());
876+
}
877+
878+
[Fact]
879+
public async Task SerializeJsonObjectExtensionData_EmptyJsonObject()
880+
{
881+
var obj = new ClassWithJsonObjectExtensionDataAndProperty
882+
{
883+
Id = 1,
884+
Extra = new JsonObject()
885+
};
886+
887+
string json = await Serializer.SerializeWrapper(obj);
888+
Assert.Equal(@"{""Id"":1}", json);
889+
}
890+
891+
[Fact]
892+
public async Task SerializeJsonObjectExtensionData_NullJsonObject()
893+
{
894+
var obj = new ClassWithJsonObjectExtensionDataAndProperty
895+
{
896+
Id = 1,
897+
Extra = null
898+
};
899+
900+
string json = await Serializer.SerializeWrapper(obj);
901+
Assert.Equal(@"{""Id"":1}", json);
902+
}
903+
904+
public class ClassWithJsonObjectExtensionDataAndProperty
905+
{
906+
public int Id { get; set; }
907+
908+
[JsonExtensionData]
909+
public JsonObject? Extra { get; set; }
910+
}
911+
818912
[Fact]
819913
#if BUILDING_SOURCE_GENERATOR_TESTS
820914
[ActiveIssue("https://github.com/dotnet/runtime/issues/58945")]

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ public ExtensionDataTests_Metadata()
7474
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))]
7575
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryAlreadyInstantiated))]
7676
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryJsonElementAlreadyInstantiated))]
77+
[JsonSerializable(typeof(ClassWithJsonObjectExtensionDataAndProperty))]
7778
internal sealed partial class ExtensionDataTestsContext_Metadata : JsonSerializerContext
7879
{
7980
}
@@ -144,6 +145,7 @@ public ExtensionDataTests_Default()
144145
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))]
145146
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryAlreadyInstantiated))]
146147
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryJsonElementAlreadyInstantiated))]
148+
[JsonSerializable(typeof(ClassWithJsonObjectExtensionDataAndProperty))]
147149
internal sealed partial class ExtensionDataTestsContext_Default : JsonSerializerContext
148150
{
149151
}

0 commit comments

Comments
 (0)