JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.82k stars 3.26k forks source link

Deserializing with MaxDepth throws StackOverflowException with relatively shallow depth on .NET Core #2436

Open madelson opened 3 years ago

madelson commented 3 years ago

Thanks for maintaining this great library! I'm running into an interesting issue porting my Json.NET code from .NET Framework to .NET Core.

Steps to reproduce

        [Test]
        public void TestMaxDepth()
        {
                        // With a value of 150, this stackoverflows when run in .NET Core and passes when run in .NET Framework.
                        // Even much higher values (e. g. 500) pass on .NET Framework without incident.
            const int MaxDepth = 150;

            var deeplyNested = new Nested();
            var inner = deeplyNested;
            for (var i = 0; i < MaxDepth + 1; ++i)
            {
                inner.Inner = new Nested();
                inner = inner.Inner;
            }

            var serialized = JsonConvert.SerializeObject(deeplyNested);

            Assert.Throws<JsonReaderException>(() => JsonConvert.DeserializeObject<Nested>(serialized, new JsonSerializerSettings { MaxDepth = MaxDepth }));
        }

Expected behavior

Setting a max depth setting that is much less than the max stack should prevent stackoverflow when deserializing. Behavior should be comparable between .NET Framework and .NET Core.

Actual behavior

.NET Framework behaves as expected but when running against .NET Core (target framework netcoreapp3.1) stack overflows very quickly.

When examining in the debugger, it seems like the stack overflow occurs AFTER the max depth trigger is hit and not during the recursion down to that depth (perhaps when constructing the exception?).

This is concerning for us because we use MaxDepth to protect from application crashes due to stack overflow. While 150 may seem like plenty of depth, I've seen it fail with even lower depth values in more complex scenarios with things like custom JsonConverters adding to the stack (not to mention whatever initial depth the serialization occurs at).

JamesNK commented 3 years ago

It works fine on my desktop, but fails on CI build. I'm guessing something about .NET Core in low resource environments decreases the available stack size, and makes a stack overflow easier.

boydc2014 commented 2 years ago

We met the same issue, is there any thing we can do to avoid this exception?