Add support for parse stack limiting

[xyzzyz]

Currently. jsonhpp doesn't support stack limiting for parsing. If you try to parse a string that starts with 100 000 '[' characters, it will most likely overflow stack and crash the whole thread. That makes it unsuitable for parsing untrusted json inputs without separating parser to separate binary, potentially with sandboxing.

Jsonhpp should keep track of the parse stack level, allow for configurable maximum level, and return runtime parsing error whenever stack limit io reached.

[t-b]

@xyzzyz I'm also interested in that feature.

Any ideas how to implement that?

The output routine has the same issue, for that I would just add a maximum_depth parameter to serializer::dump.

[nlohmann]

There are quite some options for dump already. It seems that it is time to create some struct serializer_options and put all the options in there. Does anyone has an idea how to achieve this without breaking code in newer versions if an option is added?

Say I have this struct:

struct serializer_options
{
    bool pretty_print {false};
    std::size_t indentation_step {4};
    char indentation_char {' '};
    bool ensure_ascii {false};
    error_handler_t error_handler {error_handler_t::strict};
};

for the current options and their default values and client code with

serializer_options options;
options.pretty_print = true;
std::cout << j.dump(options) << std::endl;

What would I need to do to avoid breaking this code if I wanted to add

std::size stack_limit {0};

to serializer_options?

[nlohmann]

(The described approach #1788 (comment) could also fix the issues with floating-point precision #1519, #1421, #1384, #1191 #677.)

[xyzzyz]

Shouldn't it rather be, de-serializer options? This is an issue on parsing side, not on printing side.

[nlohmann]

Ooops. Yes. Sorry for the confusion. In any case, the same question also applies for serialization.

[t-b]

Does anyone has an idea how to achieve this without breaking code in newer versions if an option is added?

This is not possible. The value of sizeof $somestruct is part of interface and therefore any change of it, will break the compatiblity. Back in C-only days you would add some reserved space at the end of the struct with e.g. char reserved[26]; and then continously eat from that for new options while keeping the struct size constant. But I would need to do some researching to find out if that is still the best option.

[nlohmann]

Yes, I saw libuv doing this. Then maybe one could use json as type of the options to allow to add more options down the road.

[nlohmann]

Related #1599

[nickaein]

With the merge of #1436 and thanks to the non-recursive parser, the code will not crash for deeply-nested JSON values anymore. Now the parsing is only bounded by the available memory.

Even so, a max-depth parameter for parsing will probably be handy for the user to easily reject a subset of invalid JSONs.

[nickaein]

This is not possible. The value of sizeof $somestruct is part of interface and therefore any change of it, will break the compatiblity.

That's technically true. Though I wonder how users will be affected. For the most part, I think we can manage to add new members to serializer_options without breaking the user code. A disclaimer about the size of this struct can also be helpful. Therefore as long as they don't directly depend on the size of this struct, which is an implementation detail in a sense, we promise the future changes doesn't change the behavior.

We can also implement from_json/to_json for the users who want to store/load serializer_options.

One last solution would be to use pimpl trick to keep the struct size constant from the user point of view.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[t-b]

@nlohmann

Then maybe one could use json as type of the options to allow to add more options down the road.

So we would pass a json document with the options to dump? That sounds actually like a nice eat-your-own-dog-food approach.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.