runtime: Serializer should throw NotSupportedException when collections can be instantiated but not populated
Consider (head at https://github.com/dotnet/corefx/commit/4fbef813c6e3c1ce1a23e5dc1528e65abb1b8c0a)
using System;
using System.Text.Json;
using Microsoft.Extensions.Primitives;
namespace stringvaluestest
{
class Program
{
static void Main(string[] args)
{
string json = @"[""My Val""]";
JsonSerializer.Deserialize<StringValues>(json);
}
}
}
Output
Unhandled exception. System.NotSupportedException: Specified method is not supported.
at Microsoft.Extensions.Primitives.StringValues.System.Collections.Generic.ICollection<System.String>.Add(String item)
at System.Text.Json.JsonPropertyInfoCommon`4.CreateDerivedEnumerableInstance(JsonPropertyInfo collectionPropertyInfo, IList sourceList, String jsonPath, JsonSerializerOptions options)
at System.Text.Json.Serialization.Converters.DefaultDerivedEnumerableConverter.CreateFromList(ReadStack& state, IList sourceList, JsonSerializerOptions options)
at System.Text.Json.JsonSerializer.HandleEndArray(JsonSerializerOptions options, Utf8JsonReader& reader, ReadStack& state)
at System.Text.Json.JsonSerializer.ReadCore(JsonSerializerOptions options, Utf8JsonReader& reader, ReadStack& readStack)
at System.Text.Json.JsonSerializer.ReadCore(Type returnType, JsonSerializerOptions options, Utf8JsonReader& reader)
at System.Text.Json.JsonSerializer.ParseCore(String json, Type returnType, JsonSerializerOptions options)
at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
at stringvaluestest.Program.Main(String[] args) in D:\console_apps\stringvaluestest\Program.cs:line 14
https://github.com/dotnet/corefx/pull/39001 adds support for types that implement BCL enumerables that are natively supported in the serializer.
For cases like StringValue where we cannot invoke the add method of the implemented type (ICollection<string>.Add in this case), we should throw a JsonException instead of a NotSupportedException:
For enumerables that are not supported: readonly collections like StringValues (implements ICollection<string>), and collections that do not implement any of:
IListICollection<T>Stack<T>Queue<T>IDictionaryIDictionary<string, TValue>, the serializer should throw aNotSupportedException.
These collections can be supported in the future.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 19 (19 by maintainers)
@ahsonkhan Filed https://github.com/dotnet/docs/issues/14005. Let me know if there’s anything you’d like to change/add 😄
What does this mean? You can’t support
StringValuesin the future, using theAddmethod. It throwsNotSupportedException(because it’sIsReadOnly).As I see it, supporting
StringValuesand this issue is two different things. If you want to supportStringValuesin the future, you have to add support for calling the proper constructor, not using theAddmethod, like @JamesNK mentioned:Also, this is about much more than
StringValues. Any collection that hasIsReadOnlyprobably doesn’t want you to call itsAddmethod:https://github.com/dotnet/corefx/blob/66273ed9944c7c964d3d51771ed8d4b4ddce3e8c/src/System.Collections.Specialized/src/System/Collections/Specialized/NameValueCollection.cs#L265-L266
https://github.com/dotnet/corefx/blob/66273ed9944c7c964d3d51771ed8d4b4ddce3e8c/src/Common/src/CoreLib/System/Collections/ObjectModel/ReadOnlyCollection.cs#L65-L68
https://github.com/dotnet/corefx/blob/66273ed9944c7c964d3d51771ed8d4b4ddce3e8c/src/Common/src/CoreLib/System/Collections/ObjectModel/ReadOnlyCollection.cs#L79-L82
Why rely on exceptions for flow control? If you ask me, that’s more of an anti-pattern than wrapping execptions with a clear message and a documented exception type.
I’ll wait for a decision on whether
JsonException,NotSupportedExceptionor not throwing at all is the best option, before updating https://github.com/dotnet/corefx/pull/40268.@martincostello That’s a good point. Added to dotnet/corefx#40268.