Skip to content

Commit d59f143

Browse files
stevehartersteveharter
authored
Merge pull request #32669 from steveharter/NullConvertersAndNotSupportedException
Don't allow null converters and add Path support to NotSupportedException
2 parents 3d2f172 + 417254e commit d59f143

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1078
-579
lines changed

‎src/libraries/System.Text.Json/ref/System.Text.Json.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ public JsonSerializerOptions() { }
465465
public System.Text.Json.JsonCommentHandling ReadCommentHandling { get { throw null; } set { } }
466466
public System.Text.Json.Serialization.ReferenceHandling ReferenceHandling { get { throw null; } set { } }
467467
public bool WriteIndented { get { throw null; } set { } }
468-
public System.Text.Json.Serialization.JsonConverter? GetConverter(System.Type typeToConvert) { throw null; }
468+
public System.Text.Json.Serialization.JsonConverter GetConverter(System.Type typeToConvert) { throw null; }
469469
}
470470
public sealed partial class JsonString : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonString>
471471
{

‎src/libraries/System.Text.Json/src/Resources/Strings.resx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,6 @@
357357
<data name="SerializationNotSupportedType" xml:space="preserve">
358358
<value>The type '{0}' is not supported.</value>
359359
</data>
360-
<data name="SerializationNotSupported" xml:space="preserve">
361-
<value>The type '{0}' on '{1}' is not supported.</value>
362-
</data>
363360
<data name="InvalidCharacterAtStartOfComment" xml:space="preserve">
364361
<value>'{0}' is invalid after '/' at the beginning of the comment. Expected either '/' or '*'.</value>
365362
</data>
@@ -485,4 +482,10 @@
485482
<data name="MetadataInvalidPropertyWithLeadingDollarSign" xml:space="preserve">
486483
<value>Properties that start with '$' are not allowed on preserve mode, either escape the character or turn off preserve references by setting ReferenceHandling to ReferenceHandling.Default.</value>
487484
</data>
485+
<data name="SerializerConverterFactoryReturnsNull" xml:space="preserve">
486+
<value>The converter '{0}' cannot return a null value.</value>
487+
</data>
488+
<data name="SerializationNotSupportedParentType" xml:space="preserve">
489+
<value>The unsupported member type is located on type '{0}'.</value>
490+
</data>
488491
</root>

‎src/libraries/System.Text.Json/src/System.Text.Json.csproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@
119119
<Compile Include="System\Text\Json\Serialization\JsonConverterAttribute.cs" />
120120
<Compile Include="System\Text\Json\Serialization\JsonConverterFactory.cs" />
121121
<Compile Include="System\Text\Json\Serialization\JsonConverterOfT.cs" />
122+
<Compile Include="System\Text\Json\Serialization\JsonConverterOfT.ReadCore.cs" />
123+
<Compile Include="System\Text\Json\Serialization\JsonConverterOfT.WriteCore.cs" />
122124
<Compile Include="System\Text\Json\Serialization\JsonDefaultNamingPolicy.cs" />
123125
<Compile Include="System\Text\Json\Serialization\JsonDictionaryConverter.cs" />
124126
<Compile Include="System\Text\Json\Serialization\JsonExtensionDataAttribute.cs" />
@@ -129,7 +131,6 @@
129131
<Compile Include="System\Text\Json\Serialization\JsonPropertyInfoOfTTypeToConvert.cs" />
130132
<Compile Include="System\Text\Json\Serialization\JsonPropertyNameAttribute.cs" />
131133
<Compile Include="System\Text\Json\Serialization\JsonResumableConverterOfT.cs" />
132-
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.cs" />
133134
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.HandleMetadata.cs" />
134135
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.HandlePropertyName.cs" />
135136
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.Helpers.cs" />
@@ -138,7 +139,6 @@
138139
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.String.cs" />
139140
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.Utf8JsonReader.cs" />
140141
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Write.ByteArray.cs" />
141-
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Write.cs" />
142142
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Write.HandleMetadata.cs" />
143143
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Write.Helpers.cs" />
144144
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Write.Stream.cs" />

‎src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.Unescaping.cs‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,13 @@ public static bool TryGetUnescapedBase64Bytes(ReadOnlySpan<byte> utf8Source, int
4242
// TODO: Similar to escaping, replace the unescaping logic with publicly shipping APIs from https://github.com/dotnet/runtime/issues/27919
4343
public static string GetUnescapedString(ReadOnlySpan<byte> utf8Source, int idx)
4444
{
45-
byte[]? unescapedArray = null;
45+
// The escaped name is always >= than the unescaped, so it is safe to use escaped name for the buffer length.
46+
int length = utf8Source.Length;
47+
byte[]? pooledName = null;
4648

47-
Span<byte> utf8Unescaped = utf8Source.Length <= JsonConstants.StackallocThreshold ?
48-
stackalloc byte[utf8Source.Length] :
49-
(unescapedArray = ArrayPool<byte>.Shared.Rent(utf8Source.Length));
49+
Span<byte> utf8Unescaped = length <= JsonConstants.StackallocThreshold ?
50+
stackalloc byte[length] :
51+
(pooledName = ArrayPool<byte>.Shared.Rent(length));
5052

5153
Unescape(utf8Source, utf8Unescaped, idx, out int written);
5254
Debug.Assert(written > 0);
@@ -56,15 +58,40 @@ public static string GetUnescapedString(ReadOnlySpan<byte> utf8Source, int idx)
5658

5759
string utf8String = TranscodeHelper(utf8Unescaped);
5860

59-
if (unescapedArray != null)
61+
if (pooledName != null)
6062
{
6163
utf8Unescaped.Clear();
62-
ArrayPool<byte>.Shared.Return(unescapedArray);
64+
ArrayPool<byte>.Shared.Return(pooledName);
6365
}
6466

6567
return utf8String;
6668
}
6769

70+
public static ReadOnlySpan<byte> GetUnescapedSpan(ReadOnlySpan<byte> utf8Source, int idx)
71+
{
72+
// The escaped name is always >= than the unescaped, so it is safe to use escaped name for the buffer length.
73+
int length = utf8Source.Length;
74+
byte[]? pooledName = null;
75+
76+
Span<byte> utf8Unescaped = length <= JsonConstants.StackallocThreshold ?
77+
stackalloc byte[length] :
78+
(pooledName = ArrayPool<byte>.Shared.Rent(length));
79+
80+
Unescape(utf8Source, utf8Unescaped, idx, out int written);
81+
Debug.Assert(written > 0);
82+
83+
ReadOnlySpan<byte> propertyName = utf8Unescaped.Slice(0, written).ToArray();
84+
Debug.Assert(!propertyName.IsEmpty);
85+
86+
if (pooledName != null)
87+
{
88+
new Span<byte>(pooledName, 0, written).Clear();
89+
ArrayPool<byte>.Shared.Return(pooledName);
90+
}
91+
92+
return propertyName;
93+
}
94+
6895
public static bool UnescapeAndCompare(ReadOnlySpan<byte> utf8Source, ReadOnlySpan<byte> other)
6996
{
7097
Debug.Assert(utf8Source.Length >= other.Length && utf8Source.Length / JsonConstants.MaxExpansionFactorWhileEscaping <= other.Length);

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ClassType.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ namespace System.Text.Json
1313
/// </remarks>
1414
internal enum ClassType : byte
1515
{
16+
// Default - no class type.
17+
None = 0x0,
1618
// JsonObjectConverter<> - objects with properties.
1719
Object = 0x1,
1820
// JsonConverter<> - simple values.
@@ -23,7 +25,5 @@ internal enum ClassType : byte
2325
Enumerable = 0x8,
2426
// JsonDictionaryConverter<,> - dictionary types.
2527
Dictionary = 0x10,
26-
// Invalid (not used directly for serialization)
27-
Invalid = 0x20
2828
}
2929
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableConverterFactory.cs‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace System.Text.Json.Serialization.Converters
1414
/// <summary>
1515
/// Converter factory for all IEnumerable types.
1616
/// </summary>
17-
internal class JsonIEnumerableConverterFactory : JsonConverterFactory
17+
internal class IEnumerableConverterFactory : JsonConverterFactory
1818
{
1919
private static readonly IDictionaryConverter<IDictionary> s_converterForIDictionary = new IDictionaryConverter<IDictionary>();
2020
private static readonly IEnumerableConverter<IEnumerable> s_converterForIEnumerable = new IEnumerableConverter<IEnumerable>();
@@ -43,10 +43,9 @@ public override bool CanConvert(Type typeToConvert)
4343
[PreserveDependency(".ctor", "System.Text.Json.Serialization.Converters.ListOfTConverter`2")]
4444
[PreserveDependency(".ctor", "System.Text.Json.Serialization.Converters.QueueOfTConverter`2")]
4545
[PreserveDependency(".ctor", "System.Text.Json.Serialization.Converters.StackOfTConverter`2")]
46-
public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
46+
public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
4747
{
48-
JsonConverter? converter = null;
49-
Type converterType;
48+
Type converterType = null!;
5049
Type[] genericArgs;
5150
Type? elementType = null;
5251
Type? actualTypeToConvert;
@@ -57,7 +56,7 @@ public override bool CanConvert(Type typeToConvert)
5756
// Verify that we don't have a multidimensional array.
5857
if (typeToConvert.GetArrayRank() > 1)
5958
{
60-
return null;
59+
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(typeToConvert);
6160
}
6261

6362
converterType = typeof(ArrayConverter<,>);
@@ -80,7 +79,7 @@ public override bool CanConvert(Type typeToConvert)
8079
}
8180
else
8281
{
83-
return null;
82+
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(typeToConvert);
8483
}
8584
}
8685
// Immutable dictionaries from System.Collections.Immutable, e.g. ImmutableDictionary<string, TValue>
@@ -94,7 +93,7 @@ public override bool CanConvert(Type typeToConvert)
9493
}
9594
else
9695
{
97-
return null;
96+
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(typeToConvert);
9897
}
9998
}
10099
// IDictionary<string,> or deriving from IDictionary<string,>
@@ -108,7 +107,7 @@ public override bool CanConvert(Type typeToConvert)
108107
}
109108
else
110109
{
111-
return null;
110+
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(typeToConvert);
112111
}
113112
}
114113
// IReadOnlyDictionary<string,> or deriving from IReadOnlyDictionary<string,>
@@ -122,7 +121,7 @@ public override bool CanConvert(Type typeToConvert)
122121
}
123122
else
124123
{
125-
return null;
124+
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(typeToConvert);
126125
}
127126
}
128127
// Immutable non-dictionaries from System.Collections.Immutable, e.g. ImmutableStack<T>
@@ -213,25 +212,24 @@ public override bool CanConvert(Type typeToConvert)
213212
converterType = typeof(IEnumerableConverter<>);
214213
}
215214

216-
if (converterType != null)
217-
{
218-
Type genericType;
219-
if (converterType.GetGenericArguments().Length == 1)
220-
{
221-
genericType = converterType.MakeGenericType(typeToConvert);
222-
}
223-
else
224-
{
225-
genericType = converterType.MakeGenericType(typeToConvert, elementType!);
226-
}
215+
Debug.Assert(converterType != null);
227216

228-
converter = (JsonConverter)Activator.CreateInstance(
229-
genericType,
230-
BindingFlags.Instance | BindingFlags.Public,
231-
binder: null,
232-
args: null,
233-
culture: null)!;
217+
Type genericType;
218+
if (converterType.GetGenericArguments().Length == 1)
219+
{
220+
genericType = converterType.MakeGenericType(typeToConvert);
234221
}
222+
else
223+
{
224+
genericType = converterType.MakeGenericType(typeToConvert, elementType!);
225+
}
226+
227+
JsonConverter converter = (JsonConverter)Activator.CreateInstance(
228+
genericType,
229+
BindingFlags.Instance | BindingFlags.Public,
230+
binder: null,
231+
args: null,
232+
culture: null)!;
235233

236234
return converter;
237235
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace System.Text.Json.Serialization.Converters
1010
/// <summary>
1111
/// Default base class implementation of <cref>JsonObjectConverter{T}</cref>.
1212
/// </summary>
13-
internal sealed class ObjectDefaultConverter<T> : JsonObjectConverter<T>
13+
internal sealed class ObjectDefaultConverter<T> : JsonObjectConverter<T> where T : notnull
1414
{
1515
internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, [MaybeNullWhen(false)] out T value)
1616
{
@@ -273,16 +273,10 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
273273
internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, ref WriteStack state)
274274
{
275275
// Minimize boxing for structs by only boxing once here
276-
object? objectValue = value;
276+
object objectValue = value!;
277277

278278
if (!state.SupportContinuation)
279279
{
280-
if (objectValue == null)
281-
{
282-
writer.WriteNullValue();
283-
return true;
284-
}
285-
286280
writer.WriteStartObject();
287281

288282
if (options.ReferenceHandling.ShouldWritePreservedReferences())
@@ -338,12 +332,6 @@ internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializer
338332
{
339333
if (!state.Current.ProcessedStartToken)
340334
{
341-
if (objectValue == null)
342-
{
343-
writer.WriteNullValue();
344-
return true;
345-
}
346-
347335
writer.WriteStartObject();
348336

349337
if (options.ReferenceHandling.ShouldWritePreservedReferences())

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,8 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer
2020

2121
Type valueTypeToConvert = typeToConvert.GetGenericArguments()[0];
2222

23-
JsonConverter? valueConverter = options.GetConverter(valueTypeToConvert);
24-
if (valueConverter == null)
25-
{
26-
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(valueTypeToConvert);
27-
}
23+
JsonConverter valueConverter = options.GetConverter(valueTypeToConvert);
24+
Debug.Assert(valueConverter != null);
2825

2926
return CreateValueConverter(valueTypeToConvert, valueConverter);
3027
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.AddProperty.cs‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,19 @@ private JsonPropertyInfo AddProperty(Type propertyType, PropertyInfo propertyInf
1717
return JsonPropertyInfo.CreateIgnoredPropertyPlaceholder(propertyInfo, options);
1818
}
1919

20-
JsonConverter? converter;
21-
ClassType classType = GetClassType(
20+
JsonConverter converter = GetConverter(
2221
propertyType,
2322
parentClassType,
2423
propertyInfo,
25-
out Type? runtimeType,
26-
out Type? _,
27-
out converter,
24+
out Type runtimeType,
2825
options);
2926

30-
if (converter == null)
31-
{
32-
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(propertyType, parentClassType, propertyInfo);
33-
}
34-
3527
return CreateProperty(
3628
declaredPropertyType: propertyType,
3729
runtimePropertyType: runtimeType,
3830
propertyInfo,
3931
parentClassType,
4032
converter,
41-
classType,
4233
options);
4334
}
4435

@@ -48,7 +39,6 @@ internal static JsonPropertyInfo CreateProperty(
4839
PropertyInfo? propertyInfo,
4940
Type parentClassType,
5041
JsonConverter converter,
51-
ClassType classType,
5242
JsonSerializerOptions options)
5343
{
5444
// Create the JsonPropertyInfo instance.
@@ -58,7 +48,7 @@ internal static JsonPropertyInfo CreateProperty(
5848
parentClassType,
5949
declaredPropertyType,
6050
runtimePropertyType,
61-
runtimeClassType: classType,
51+
runtimeClassType: converter.ClassType,
6252
propertyInfo,
6353
converter,
6454
options);
@@ -74,9 +64,8 @@ internal static JsonPropertyInfo CreateProperty(
7464
/// </summary>
7565
internal static JsonPropertyInfo CreatePolicyProperty(
7666
Type declaredPropertyType,
77-
Type? runtimePropertyType,
67+
Type runtimePropertyType,
7868
JsonConverter converter,
79-
ClassType classType,
8069
JsonSerializerOptions options)
8170
{
8271
return CreateProperty(
@@ -85,7 +74,6 @@ internal static JsonPropertyInfo CreatePolicyProperty(
8574
propertyInfo: null, // Not a real property so this is null.
8675
parentClassType: typeof(object), // a dummy value (not used)
8776
converter : converter,
88-
classType : classType,
8977
options);
9078
}
9179
}

0 commit comments

Comments
 (0)