-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Feature request: non-recursive GenericValue
destructor
#2217
Comments
|
Hi @aikawayataro! Yes, sorry, you're right of course, this is all about Indeed, this is not an issue with the This is easily reproducible with the following code:
Now if i feed into
, which could be generated by the following code, for example:
, the |
GenericValue
destructor
Okay, I understand your point. The problem is that in order to implement a new flag for a value, the internal API have to be changed, including the I see the solution to your problem is to use void IterativeClean(Value &value);
int main() {
std::string_view json = "{}";
struct MyHandler : public BaseReaderHandler<UTF8<>, MyHandler> {
MyHandler(Document &doc) : doc_(doc) {
stack.push(&doc_);
}
bool Default() {
return true;
}
// Implement callbacks from BaseReaderHandler here.
std::stack<Value *> stack;
Document &doc_;
};
MemoryStream json_stream = MemoryStream(json.data(), json.size());
Reader reader;
Document doc;
MyHandler handler(doc);
auto res = reader.Parse<kParseIterativeFlag>(json_stream, handler);
if (res.IsError()) { // Parsing failed
IterativeClean(doc);
}
// ...
IterativeClean(doc);
return 0;
} |
A simpler and faster code can also be achieved by using |
Yes, the problem with At userver we try really hard to wrap any native
We consider this a security threat (carefully crafted data sent by an attacker could potentially lead to a DoS) for us, and are willing to provide a patch (we vendor rapidjson and we'll likely patch it anyway).
We understand that iterative destruction would be measurably slower, but we are ok with that. What do you think? |
I think you should submit a pull request then. |
Hi!
There is a
kParseIterativeFlag
, which prevents stack exhaustion when parsing deeply nested json data, however destruction of the parsed value is still recursive and might blow the stack up in case of extremely deeply nested objects or small stacks.We at https://github.com/userver-framework/userver use stackfull coroutines with the stacks way less than 8Mb, and we have a workaround for destroying
GenericDocument
iteratively, which is a bit ugly, but at least it works.Here comes the problem:
Given following code
and the json data in form
the "o" object would be destroyed in the
Parse
function itself, before we are able to wrap it into non-recursive destruction routine,and with depth big enough it would blow the stack up, which effectively defies the purpose of
kParseIterativeFlag
at all.One workaround for that could be a depth limit for
Parse
, however it would still leave some room for constructing jsons nested deeper than the aforementioned limit, so what i propose is a compile-time flag to destroyGenericDocument
iteratively, to avoid potentially dangerous recursion once and for all.The text was updated successfully, but these errors were encountered: