Fix AnyType serialization with empty-string dictionary keys#9775
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes AnyType JSON serialization for dictionaries that contain an empty-string key by allowing empty property names to flow through ResultElement and preventing a writer crash when attempting to write a zero-length byte span.
Changes:
- Allow empty-string JSON property names by removing the
Length == 0exception guards inResultElement.SetPropertyNameoverloads. - Add an early-return in
ResultDocument.WriteDataCorefordata.Length == 0to avoid exceptions when writing empty property names. - Add a regression test validating output when a resolver returns a dictionary with an empty-string key.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/HotChocolate/Core/test/Types.Tests/Types/Scalars/AnyTypeTests.cs | Adds a regression test for empty-string dictionary keys in AnyType output. |
| src/HotChocolate/Core/src/Types/Text/Json/ResultElement.cs | Removes empty-name guards so empty JSON property names are permitted. |
| src/HotChocolate/Core/src/Types/Text/Json/ResultDocument.cs | Avoids exceptions by no-op’ing WriteDataCore on zero-length writes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
ResultDocument.WriteDataCoreearly-returns on a zero-length span, fixing anArgumentOutOfRangeExceptionwhen writing an empty property name.ResultDocument.ReadLocalDataearly-returns on a zero-length read, so properties withSizeOrLength == 0whoseLocationwas never backed by a rented chunk (e.g.{ "": null }with no other local data) don't trip_data[startChunkIndex].propertyName.Length == 0throws from bothResultElement.SetPropertyNameoverloads — empty strings are valid JSON property names and the previous guard was masking the writer bug above.AnyTyperegression test covering a resolver returning aDictionary<string, object?>with an empty-string key and anullvalue (the minimal case that exercises both the zero-length write and zero-length read paths).Test plan
dotnet test src/HotChocolate/Core/test/Types.Tests/HotChocolate.Types.Tests.csproj --filter "FullyQualifiedName~AnyTypeTests.Output_Return_Dictionary_With_Empty_Key"— green on net9.0.dotnet test src/HotChocolate/Core/test/Types.Tests/HotChocolate.Types.Tests.csproj --filter "FullyQualifiedName~AnyType"— fullAnyTypesuite 75/75 on net9.0.Closes #9774