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

add more strict checking #119

Closed
wants to merge 3 commits into from
Closed

add more strict checking #119

wants to merge 3 commits into from

Conversation

lunatikub
Copy link

add more strict checking

jsmn.c Outdated
r = jsmn_parse_string(parser, js, len, tokens, num_tokens);
if (r < 0) return r;
count++;
if (parser->toksuper != -1 && tokens != NULL)
tokens[parser->toksuper].size++;
#ifdef JSMN_STRICT
if (tokens != NULL) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parser->toksuper might be -1

@lunatikub
Copy link
Author

@chlunde Fixed !

@lunatikub
Copy link
Author

Any news ?

@edsiper
Copy link

edsiper commented Aug 6, 2018

@lunatikub

I am using your patch in our project here:

https://github.com/fluent/fluent-bit/tree/master/lib/jsmn

I've found that when running in strict mode and tokens is NULL (because I want only to validate the string and not to populate results), the validation fails, a simple JSON map input like this helps to reproduce the problem:

{"key": "some value"}

e.g:

    ret = jsmn_parse(&parser, json, len, NULL, 0);

any hints are appreciated.

@lunatikub
Copy link
Author

Hi @edsiper,

Thanks for highlighting this important point in my patch:

The strict mode needs the array of tokens to validate a JSON: in the case of a string token, its needs to know the type of the parent to deduce the type of the next token by the function 'jsmn_string_next_tok'.

For instance, if a string is being parsed and the parent is an 'array', the next type after a comma must be a 'value'.

With this point in mind, I will modify the patch so that passing a NULL 'tokens' array will not be accepted by the function 'jsmn_parse'.

It will look like:
#ifdef JSMN_STRICT assert(tokens != NULL); #endif

Warm regards,
Thomas,

@edsiper
Copy link

edsiper commented Aug 6, 2018

@lunatikub thanks for the detailed info, that makes a lot of sense, I will go ahead and deprecate the usage of jsmn_parse() in that mode.

btw, have you consider to maintain a fork of this project since it low activity from a maintainer perspective ?

edsiper added a commit to fluent/fluent-bit that referenced this pull request Aug 6, 2018
JSMN in strict mode cannot be used to validate a JSON string without
filing the tokens, it behavior is unexpected, more details about this
in the following jsmn pull request:

    zserge/jsmn#119 (comment)

this patch remove the flb_pack_json_valid() function.

Signed-off-by: Eduardo Silva <[email protected]>
edsiper added a commit to fluent/fluent-bit that referenced this pull request Aug 6, 2018
JSMN in strict mode cannot be used to validate a JSON string without
filing the tokens, it behavior is unexpected, more details about this
in the following jsmn pull request:

    zserge/jsmn#119 (comment)

this patch remove the flb_pack_json_valid() function.

Signed-off-by: Eduardo Silva <[email protected]>
@pt300
Copy link
Collaborator

pt300 commented May 12, 2019

More changes ahead.

@pt300 pt300 closed this May 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants