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

Stackoverflow when deserializing more than one time with ValidatingJsonConverter #550

Closed
1 task done
EmmyDream opened this issue Nov 3, 2023 · 5 comments · Fixed by #554
Closed
1 task done
Labels
bug Something isn't working

Comments

@EmmyDream
Copy link

Nuget Package

JsonSchema.Net

Package Version

5.3.0

Operating System

Windows

.Net Target (if relevant, please specify the version in the description)

.Net (5 or after)

A clear and concise description of the bug.

Usage of ValidatingJsonConverter throws stackoverflow after second usage. From my understanding _optionsFactory is not working correctly (ValidatingJsonConverter is not removed from options), when there is a usage of cache - converter has been already defined in cache.
image

What did you expect?

Not having stackoverflow.

Please add test code or steps to reproduce the behavior.

I have followed instruction here https://docs.json-everything.net/schema/serialization/, how to use ValidatingJsonConverter with attribute on deserializing type.

Is there any other information you'd like to share regarding this bug?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@EmmyDream EmmyDream added the bug Something isn't working label Nov 3, 2023
@gregsdennis
Copy link
Owner

Thanks for reporting. I'll have a look.

@gregsdennis
Copy link
Owner

I've added this test which runs deserialization 10 times repeatedly. It passes.

[Test]
public void WithSchema_MultipleDeserializations()
{
    try
    {
        var jsonText = @"{
  ""Bar"": ""bartholomew"",
  ""Value"": 42
}";

        for (int i = 0; i < 10; i++)
        {
            var model = JsonSerializer.Deserialize<FooWithSchema>(jsonText, _options);

            Console.WriteLine(JsonSerializer.Serialize(model, _options));
        }
    }
    catch (Exception e)
    {
        HandleException(e);
        throw;
    }
}

Can you provide a replicating test, please?

@EmmyDream
Copy link
Author

EmmyDream commented Nov 3, 2023

Hello, thank you for your test. Please update it as follow:

    [Test]
    public void WithSchema_MultipleDeserializations()
    {
        try
        {
            var jsonText = @"{
  ""Bar"": ""bartholomew"",
  ""Value"": 42
}";
            
            for (int i = 0; i < 10; i++)
            {
                var options = new JsonSerializerOptions
                {
                    WriteIndented = true,
                    Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
                    Converters = { new ValidatingJsonConverter { OutputFormat = OutputFormat.List } }
                };
                
                var model = JsonSerializer.Deserialize<FooWithSchema>(jsonText, options);

                Console.WriteLine(JsonSerializer.Serialize(model, options));
            }
        }
        catch (Exception e)
        {
            HandleException(e);
            throw;
        }
    }

@gregsdennis
Copy link
Owner

Interesting. You're generating new options objects for each iteration? Why not create a static one and re-use that?

(I'll fix this, obviously, but it's still worth asking the question.)

@EmmyDream
Copy link
Author

Interesting. You're generating new options objects for each iteration? Why not create a static one and re-use that?

(I'll fix this, obviously, but it's still worth asking the question.)

For example, lets consider some scope service like this:

private class FooService
 {
     private readonly JsonSerializerOptions _fooOptions = new JsonSerializerOptions
     {
         WriteIndented = true,
         Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
         Converters = { new ValidatingJsonConverter { OutputFormat = OutputFormat.List } }
     };
     
     public FooWithSchema DeserializeFoo(string jsonText)
     {
         return JsonSerializer.Deserialize<FooWithSchema>(jsonText, _fooOptions);

     }
 }

And test method:

    [Test]
    public void WithSchema_MultipleDeserializations()
    {
        try
        {
            var jsonText = @"{
  ""Bar"": ""bartholomew"",
  ""Value"": 42
}";
            
            for (int i = 0; i < 10; i++)
            {
                var model = new FooService().DeserializeFoo(jsonText);
                Console.WriteLine(JsonSerializer.Serialize(model, _options));
            }
        }
        catch (Exception e)
        {
            HandleException(e);
            throw;
        }
    }

In my case I have common implementation of deserialization service, which allows to provide different JsonSerializerOptions, so that is why these options are coming from outside implementation and can be created each time when I call common service.

Anyway thank you for a quick fix! I will test it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants