Skip to content
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.

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

NewtonSoftJsonSerializer throws a StackOverflow exception when trying to deserialize an object field #6502

Closed
Arkatufus opened this issue Mar 9, 2023 · 10 comments · Fixed by #6503

Comments

@Arkatufus
Copy link
Contributor

Arkatufus commented Mar 9, 2023

Version Information
Version of Akka.NET? 1.4.49/1.5.0
Which Akka.NET Modules? Akka

Describe the bug
NewtonSoftJsonSerializer throws a StackOverflow exception when trying to deserialize a class with an object field/property when assingned a JObject instance.

To Reproduce
Steps to reproduce the behavior:
#6503

Expected behavior
Should deserialize successfully

Actual behavior
Throws StackOverflow

Environment
Any

@Aaronontheweb
Copy link
Member

We'll release a fix for this in 1.4.50 and 1.5.1

Arkatufus added a commit to Arkatufus/akka.net that referenced this issue Mar 9, 2023
@Aaronontheweb
Copy link
Member

Is there any relation to these types of issues? #4463

i.e. is this all Newtonsoft.Json type-conversion issues?

@Arkatufus
Copy link
Contributor Author

That issue is actually by design, we are replacing int, decimal, and float with a JObject so that we can preserve the value type inside an anonymous object across the wire.

@Arkatufus
Copy link
Contributor Author

Note that we're not replacing long and double as those are actually the default Newtonsoft Json behavior

@Aaronontheweb
Copy link
Member

That issue is actually by design, we are replacing int, decimal, and float with a JObject so that we can preserve the value type inside an anonymous object across the wire.

Which issue?

@Arkatufus
Copy link
Contributor Author

I was replying to your comment on issue #4463

@Aaronontheweb
Copy link
Member

Ah ok, so close that then?

@Arkatufus
Copy link
Contributor Author

Either close that issue, or reconsider our JSON wire format to not replace value types

@Aaronontheweb
Copy link
Member

Done

@Arkatufus
Copy link
Contributor Author

These are the corresponding lines:

if (value is int || value is decimal || value is float)
{
writer.WriteStartObject();
writer.WritePropertyName("$");
writer.WriteValue(GetString(value));
writer.WriteEndObject();
}

private object GetString(object value)
{
if (value is int)
return "I" + ((int)value).ToString(NumberFormatInfo.InvariantInfo);
if (value is float)
return "F" + ((float)value).ToString(NumberFormatInfo.InvariantInfo);
if (value is decimal)
return "M" + ((decimal)value).ToString(NumberFormatInfo.InvariantInfo);
throw new NotSupportedException();
}

Aaronontheweb added a commit that referenced this issue Mar 15, 2023
…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>
Arkatufus added a commit to Arkatufus/akka.net that referenced this issue Mar 15, 2023
…o deserialize a `JObject` inside an `object` field (akkadotnet#6503)

* Reproduction for  akkadotnet#6502

* Fix JObject inside object property/field overflow

---------

Co-authored-by: Aaron Stannard <aaron@petabridge.com>
(cherry picked from commit 64c6eff)
Aaronontheweb pushed a commit that referenced this issue Mar 15, 2023
…rializer` tries to deserialize a `JObject` inside an `object` field (#6522)

* Fix `StackOverflow` exception when `NewtonsoftJsonSerializer` tries to 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>
(cherry picked from commit 64c6eff)

* Remove nullable support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants