Skip to content

Commit

Permalink
Fix StackOverflow exception when NewtonsoftJsonSerializer tries t…
Browse files Browse the repository at this point in the history
…o deserialize a `JObject` inside an `object` field (#6503)

* Reproduction for  #6502

* Fix JObject inside object property/field overflow

---------

Co-authored-by: Aaron Stannard <aaron@petabridge.com>
  • Loading branch information
Arkatufus and Aaronontheweb committed Mar 15, 2023
1 parent 3fbb7a2 commit 64c6eff
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 4 deletions.
61 changes: 61 additions & 0 deletions src/core/Akka.Tests/Serialization/SerializationSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Reflection;
using System.Runtime.Serialization;
Expand All @@ -21,6 +22,7 @@
using Akka.Util;
using Akka.Util.Reflection;
using FluentAssertions;
using Newtonsoft.Json.Linq;
using Xunit;

namespace Akka.Tests.Serialization
Expand Down Expand Up @@ -608,6 +610,60 @@ public void Missing_custom_serializer_id_should_append_help_message()
.Where(ex => ex.Message.Contains("Serializer Id [101] is not one of the internal Akka.NET serializer."));
}

[Fact(DisplayName = "Should be able to serialize object property with JObject value")]
public void ObjectPropertyJObjectTest()
{
var serializer = (NewtonSoftJsonSerializer) Sys.Serialization.FindSerializerForType(typeof(object));
var obj = JObject.FromObject(new
{
FormattedMessage = "We are apple 20 points above value 10.01 ms",
Message = "We are {0} {1} points above value {2} ms",
Parameters = new List<object> { "apple", 20, 10.01F, 50L, (decimal) 9.9 },
MessageType = 200
});
var instance = new ObjectTestClass { MyObject = obj};

var serialized = serializer.ToBinary(instance);

// Stack overflowed in the original bug
var deserialized = serializer.FromBinary<ObjectTestClass>(serialized);
deserialized.MyObject.Should().BeOfType<JObject>();
var jObj = (JObject) deserialized.MyObject;

((JValue)jObj["FormattedMessage"])!.Value.Should().Be("We are apple 20 points above value 10.01 ms");
((JValue)jObj["Message"])!.Value.Should().Be("We are {0} {1} points above value {2} ms");
var arr = ((JArray)jObj["Parameters"]);
((JValue)arr![0]).Value.Should().Be("apple");
((JValue)arr[1]).Value.Should().BeOfType<int>();
((JValue)arr[1]).Value.Should().Be(20);
((JValue)arr[2]).Value.Should().BeOfType<float>();
((JValue)arr[2]).Value.Should().Be(10.01F);
((JValue)arr[3]).Value.Should().BeOfType<long>();
((JValue)arr[3]).Value.Should().Be(50L);
((JValue)arr[4]).Value.Should().BeOfType<decimal>();
((JValue)arr[4]).Value.Should().Be((decimal)9.9);
((JValue)jObj["MessageType"])!.Value.Should().Be(200);
}

[Fact(DisplayName = "Should be able to serialize object property with anonymous type value")]
public void ObjectPropertyObjectTest()
{
var serializer = (NewtonSoftJsonSerializer) Sys.Serialization.FindSerializerForType(typeof(object));
var obj = new
{
FormattedMessage = "We are apple 20 points above value 10.01 ms",
Message = "We are {0} {1} points above value {2} ms",
Parameters = new List<object> { "apple", 20, 10.01F, 50L, (decimal) 9.9 },
MessageType = 200
};
var instance = new ObjectTestClass { MyObject = obj};

var serialized = serializer.ToBinary(instance);

var deserialized = serializer.FromBinary<ObjectTestClass>(serialized);
deserialized.MyObject.Should().BeEquivalentTo(obj);
}

public SerializationSpec():base(GetConfig())
{
}
Expand Down Expand Up @@ -708,6 +764,11 @@ public sealed class ChildClass
public string Value { get; set; }
}
}

public sealed class ObjectTestClass
{
public object MyObject { get; set; }
}
}
}

40 changes: 36 additions & 4 deletions src/core/Akka/Serialization/NewtonSoftJsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ public override object FromBinary(byte[] bytes, Type type)

private static object TranslateSurrogate(object deserializedValue, NewtonSoftJsonSerializer parent, Type type)
{
var j = deserializedValue as JObject;
if (j != null)
if (deserializedValue is JObject j)
{
//The JObject represents a special akka.net wrapper for primitives (int,float,decimal) to preserve correct type when deserializing
if (j["$"] != null)
Expand All @@ -341,19 +340,52 @@ private static object TranslateSurrogate(object deserializedValue, NewtonSoftJso
return GetValue(value);
}

// Bug: #6502 Newtonsoft could not deserialize pure JObject inside an object payload.
// If type is `object`, deep-convert object and return as is.
if (type == typeof(object))
{
return RestoreJToken(j);
}

//The JObject is not of our concern, let Json.NET deserialize it.
return j.ToObject(type, parent._serializer);
}
var surrogate = deserializedValue as ISurrogate;

//The deserialized object is a surrogate, unwrap it
if (surrogate != null)
if (deserializedValue is ISurrogate surrogate)
{
return surrogate.FromSurrogate(parent.system);
}
return deserializedValue;
}

private static JToken RestoreJToken(JToken value)
{
switch (value)
{
case JObject obj:
if (obj["$"] != null)
{
var v = obj["$"].Value<string>();
return new JValue(GetValue(v));
}
var dict = (IDictionary<string, JToken>)obj;
foreach (var kvp in dict)
{
dict[kvp.Key] = RestoreJToken(kvp.Value);
}
return obj;
case JArray arr:
for (var i = 0; i < arr.Count; i++)
{
arr[i] = RestoreJToken(arr[i]);
}
return arr;
default:
return value;
}
}

private static object GetValue(string V)
{
var t = V.Substring(0, 1);
Expand Down

0 comments on commit 64c6eff

Please sign in to comment.