Skip to content

Added guard to throw InvalidOperrationException. #95723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Added guard to throw InvalidOperrationException. #95723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ internal virtual bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, J

internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, scoped ref ReadStack state, out T? value, out bool isPopulatedValue)
{
if (typeToConvert != typeof(T))
Copy link
Member

@huoyaoyuan huoyaoyuan Dec 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the newly exposed CanConvert(Type typeToConvert) method be used here?
Does TryWrite method need similar guard?

{
ThrowHelper.ThrowInvalidOperationException($"Type '{typeToConvert}' is not compatible with converter '{GetType()}'.");
Copy link
Contributor

@karakasa karakasa Dec 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception message needs to be localizable. Besides afaic you should add a throw method in the ThrowHelper class like others.

}

// For perf and converter simplicity, handle null here instead of forwarding to the converter.
if (reader.TokenType == JsonTokenType.Null && !HandleNullOnRead && !state.IsContinuation)
{
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1416,10 +1416,10 @@ public static void CannotSet_DefaultIgnoreCondition_To_Always()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/36605")]
public static void ConverterRead_VerifyInvalidTypeToConvertFails()
{
var options = new JsonSerializerOptions();
var options = new JsonSerializerOptions(JsonSerializerOptions.Default);
options.MakeReadOnly();
Type typeToConvert = typeof(KeyValuePair<int, int>);
byte[] bytes = Encoding.UTF8.GetBytes(@"{""Key"":1,""Value"":2}");

Expand Down