Skip to content

Commit f008252

Browse files
json: 🐛Fix stack overflow deserializing empty JSON (#1293)
Fixes #1292 The call to "token.ToObject<IQuantity>(serializer)" with an empty or invalid json calls `JsonConvert.DeserializeObject` again and results in infinite recursion and stack overflow. ### Changes - Throw `JsonSerializationException` on invalid JSON in `UnitsNetIQuantityJsonConverter` - Include helpful info in exception, link to wiki, truncated JSON token in message and full JSON token in `Data["JsonToken"]` property. - Add tests for new behavior --------- Co-authored-by: Andreas Gullberg Larsen <[email protected]>
1 parent 0e979a6 commit f008252

File tree

6 files changed

+72
-22
lines changed

6 files changed

+72
-22
lines changed

UnitsNet.Serialization.JsonNet.Tests/UnitsNetIComparableJsonConverterTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace UnitsNet.Serialization.JsonNet.Tests
1313
{
1414
public sealed class UnitsNetIComparableJsonConverterTest
1515
{
16-
private UnitsNetIComparableJsonConverter _sut;
16+
private readonly UnitsNetIComparableJsonConverter _sut;
1717

1818
public UnitsNetIComparableJsonConverterTest()
1919
{

UnitsNet.Serialization.JsonNet.Tests/UnitsNetJsonDeserializationTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,24 @@ public void NullValue_ExpectNullReturned()
9898
Assert.Null(deserializedNullMass);
9999
}
100100

101+
[Fact]
102+
public void EmptyJsonValue_NullableType_ExpectJsonSerializationException()
103+
{
104+
var ex = Assert.Throws<JsonSerializationException>(() => DeserializeObject<Mass?>("{}"));
105+
Assert.Equal("Failed to deserialize IQuantity for target type System.Nullable`1[UnitsNet.Mass] from JSON '{}', expected properties Unit and Value.", ex.Message);
106+
Assert.Equal("https://github.com/angularsen/UnitsNet/wiki/Serializing-to-JSON,-XML-and-more#unitsnetserializationjsonnet-with-jsonnet-newtonsoft", ex.HelpLink);
107+
Assert.Equal("{}", ex.Data["JsonToken"]);
108+
}
109+
110+
[Fact]
111+
public void EmptyJsonValue_NonNullableType_ExpectJsonSerializationException()
112+
{
113+
var ex = Assert.Throws<JsonSerializationException>(() => DeserializeObject<Mass>("{}"));
114+
Assert.Equal("Failed to deserialize IQuantity for target type UnitsNet.Mass from JSON '{}', expected properties Unit and Value.", ex.Message);
115+
Assert.Equal("https://github.com/angularsen/UnitsNet/wiki/Serializing-to-JSON,-XML-and-more#unitsnetserializationjsonnet-with-jsonnet-newtonsoft", ex.HelpLink);
116+
Assert.Equal("{}", ex.Data["JsonToken"]);
117+
}
118+
101119
[Fact]
102120
public void NullValueNestedInObject_ExpectValueDeserializedToNullCorrectly()
103121
{
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Licensed under MIT No Attribution, see LICENSE file at the root.
2+
// Copyright 2013 Andreas Gullberg Larsen ([email protected]). Maintained at https://github.com/angularsen/UnitsNet.
3+
4+
using System.Diagnostics.CodeAnalysis;
5+
6+
namespace UnitsNet.Serialization.JsonNet
7+
{
8+
internal static class StringExtensions
9+
{
10+
/// <summary>
11+
/// Truncates a string to the given length, with truncation suffix.
12+
/// </summary>
13+
/// <param name="value">The string.</param>
14+
/// <param name="maxLength">The max length, including <paramref name="truncationSuffix"/>.</param>
15+
/// <param name="truncationSuffix">Suffix to append if truncated, defaults to "…".</param>
16+
/// <returns>The truncated string if longer, otherwise the original string.</returns>
17+
[return: NotNullIfNotNull("value")]
18+
public static string? Truncate(this string? value, int maxLength, string truncationSuffix = "…")
19+
{
20+
return value?.Length > maxLength
21+
? value.Substring(0, maxLength - truncationSuffix.Length) + truncationSuffix
22+
: value;
23+
}
24+
}
25+
}

UnitsNet.Serialization.JsonNet/UnitsNetBaseJsonConverter.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public void RegisterCustomType(Type quantity, Type unit)
4747
/// <returns>A <see cref="ValueUnit"/></returns>
4848
protected ValueUnit? ReadValueUnit(JToken jsonToken)
4949
{
50+
// Empty JSON "{}"
5051
if (!jsonToken.HasValues)
5152
{
5253
return null;

UnitsNet.Serialization.JsonNet/UnitsNetIComparableJsonConverter.cs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,28 +50,30 @@ public override void WriteJson(JsonWriter writer, IComparable? value, JsonSerial
5050
public override IComparable? ReadJson(JsonReader reader, Type objectType, IComparable? existingValue, bool hasExistingValue,
5151
JsonSerializer serializer)
5252
{
53-
reader = reader ?? throw new ArgumentNullException(nameof(reader));
54-
serializer = serializer ?? throw new ArgumentNullException(nameof(serializer));
53+
if (reader == null) throw new ArgumentNullException(nameof(reader));
54+
if (serializer == null) throw new ArgumentNullException(nameof(serializer));
5555

56-
if (reader.TokenType == JsonToken.Null)
56+
var token = JToken.Load(reader);
57+
58+
if (token.Type is JTokenType.Null)
5759
{
5860
return existingValue;
5961
}
6062

61-
var localSerializer = CreateLocalSerializer(serializer, this);
62-
63-
var token = JToken.Load(reader);
64-
6563
// If objectType is not IComparable but a type that implements IComparable, deserialize directly as this type instead.
6664
if (objectType != typeof(IComparable))
6765
{
66+
// Remove the current converter from the list of converters to avoid infinite recursion.
67+
JsonSerializer localSerializer = CreateLocalSerializer(serializer, this);
6868
return (IComparable)token.ToObject(objectType, localSerializer)!;
6969
}
7070

71-
var valueUnit = ReadValueUnit(token);
71+
ValueUnit? valueUnit = ReadValueUnit(token);
7272

7373
if (valueUnit == null)
7474
{
75+
// Remove the current converter from the list of converters to avoid infinite recursion.
76+
JsonSerializer localSerializer = CreateLocalSerializer(serializer, this);
7577
return token.ToObject<IComparable>(localSerializer);
7678
}
7779

UnitsNet.Serialization.JsonNet/UnitsNetIQuantityJsonConverter.cs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public override void WriteJson(JsonWriter writer, IQuantity? value, JsonSerializ
3131
return;
3232
}
3333

34-
var valueUnit = ConvertIQuantity(value);
34+
ValueUnit valueUnit = ConvertIQuantity(value);
3535

3636
serializer.Serialize(writer, valueUnit);
3737
}
@@ -51,24 +51,28 @@ public override void WriteJson(JsonWriter writer, IQuantity? value, JsonSerializ
5151
public override IQuantity? ReadJson(JsonReader reader, Type objectType, IQuantity? existingValue, bool hasExistingValue,
5252
JsonSerializer serializer)
5353
{
54-
reader = reader ?? throw new ArgumentNullException(nameof(reader));
55-
serializer = serializer ?? throw new ArgumentNullException(nameof(serializer));
56-
57-
if (reader.TokenType == JsonToken.Null)
58-
{
59-
return existingValue;
60-
}
54+
if (reader == null) throw new ArgumentNullException(nameof(reader));
55+
if (serializer == null) throw new ArgumentNullException(nameof(serializer));
6156

6257
var token = JToken.Load(reader);
6358

64-
var valueUnit = ReadValueUnit(token);
65-
66-
if (valueUnit == null)
59+
if (token.Type is JTokenType.Null)
6760
{
68-
return token.ToObject<IQuantity>(serializer);
61+
return existingValue;
6962
}
7063

71-
return ConvertValueUnit(valueUnit);
64+
// Try to read value and unit from JSON, otherwise throw.
65+
ValueUnit? valueUnit = ReadValueUnit(token);
66+
67+
return valueUnit != null
68+
? ConvertValueUnit(valueUnit)
69+
: throw new JsonSerializationException(
70+
$"Failed to deserialize IQuantity for target type {objectType} from JSON '{token.ToString().Truncate(100)}', expected properties Unit and Value.")
71+
{
72+
HelpLink =
73+
"https://github.com/angularsen/UnitsNet/wiki/Serializing-to-JSON,-XML-and-more#unitsnetserializationjsonnet-with-jsonnet-newtonsoft",
74+
Data = { { "JsonToken", token.ToString() }, }
75+
};
7276
}
7377
}
7478
}

0 commit comments

Comments
 (0)