On Mon, Jun 15, 2026 at 12:58 PM Markus Armbruster <[email protected]> wrote:
> > + parser->token_size += input->len;
> > + parser->token_count++;
>
> Note for later: @token_size and @token_count are incremented early
> rather than late.
Yes.
> > - /*
> > - * Security consideration, we limit total memory allocated per object
> > - * and the maximum recursion depth that a message can force.
> > - */
> > - if (parser->token_size + input->len + 1 > MAX_TOKEN_SIZE) {
>
> Left operand of > is unincremented token_size plus increment plus 1.
>
> > - if (g_queue_get_length(&parser->tokens) + 1 > MAX_TOKEN_COUNT) {
> > - error_setg(&err, "JSON token count limit exceeded");
> > - goto out_emit;
> > - }
> > - if (parser->bracket_count + parser->brace_count > MAX_NESTING) {
> > - error_setg(&err, "JSON nesting depth limit exceeded");
> > - goto out_emit;
> > - }
...
> > + * Safety consideration, we limit total memory allocated per object
> > + * and the maximum nesting depth that a message can force.
> > + */
> > + if (parser->token_size > MAX_TOKEN_SIZE) {
>
> Left operand of > is incremented token size.
>
> Isn't this one less than before the patch?
Yes.
> > + error_setg(&err, "JSON token size limit exceeded");
> > + } else if (parser->token_count > MAX_TOKEN_COUNT) {
>
> Likewise.
Not here, g_queue_get_length() was incremented before g_queue_push_tail().
The MAX_NESTING comparison is pre-incremented both before and after my patch.
In the end the only difference between before and after is for
MAX_TOKEN_SIZE. I honestly didn't notice, but it seems to me that the
difference is irrelevant and the new code is more consistent.
I'm not sure if the code before was written to mimic the two
pre-incremented conditionals, but either way the first "+ 1" does not
make much sense logically:
if (parser->token_size + input->len + 1 > MAX_TOKEN_SIZE) {
if (g_queue_get_length(&parser->tokens) + 1 > MAX_TOKEN_COUNT) {
and (unlike the second) would be better written as >=.
Paolo
> > -out_emit:
> > - parser->brace_count = 0;
> > - parser->bracket_count = 0;
> > - json_message_free_tokens(parser);
> > - parser->token_size = 0;
> > - parser->emit(parser->opaque, json, err);
> > - json_parser_reset(&parser->parser);
> > + if ((parser->brace_count == 0 && parser->bracket_count == 0)
> > + || type == JSON_END_OF_INPUT) {
>
> We need to check for JSON_END_OF_INPUT, because this is the one error
> that cannot go through normal error recovery "eat tokens until curlies
> and squares balance or we ate JSON_END_OF_INPUT". Took me a few minutes
> to figure out.
>
> > + parser->error = false;
> > + parser->brace_count = 0;
> > + parser->bracket_count = 0;
> > + parser->token_count = 0;
> > + parser->token_size = 0;
> > + json_parser_reset(&parser->parser);
> > + }
> > }
> >
> > void json_message_parser_init(JSONMessageParser *parser,
> > @@ -128,9 +109,10 @@ void json_message_parser_init(JSONMessageParser
> > *parser,
> > {
> > parser->emit = emit;
> > parser->opaque = opaque;
> > + parser->error = false;
> > parser->brace_count = 0;
> > parser->bracket_count = 0;
> > - g_queue_init(&parser->tokens);
> > + parser->token_count = 0;
> > parser->token_size = 0;
> >
> > json_parser_init(&parser->parser, ap);
> > @@ -146,12 +128,10 @@ void json_message_parser_feed(JSONMessageParser
> > *parser,
> > void json_message_parser_flush(JSONMessageParser *parser)
> > {
> > json_lexer_flush(&parser->lexer);
> > - assert(g_queue_is_empty(&parser->tokens));
> > }
> >
> > void json_message_parser_destroy(JSONMessageParser *parser)
> > {
> > json_lexer_destroy(&parser->lexer);
> > - json_message_free_tokens(parser);
> > json_parser_destroy(&parser->parser);
> > }
>
