| Message ID | 20180808120334.10970-3-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | json: Fixes, error reporting improvements, cleanups | expand |
On 08/08/2018 07:02 AM, Markus Armbruster wrote: > qobject_from_json() can return null without setting an error on > lexical errors. I call that a bug. Add test coverage to demonstrate > it. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/check-qjson.c | 36 +++++++++++++++++++++++++++++++++--- > 1 file changed, 33 insertions(+), 3 deletions(-) > > +static void junk_input(void) > +{ > + /* Note: junk within strings is covered elsewhere */ > + Error *err = NULL; > + QObject *obj; > + > + obj = qobject_from_json("@", &err); Invalid token > + g_assert(!err); /* BUG */ > + g_assert(obj == NULL); > + > + obj = qobject_from_json("[0\xFF]", &err); \xff stream reset, followed by unbalanced ] > + error_free_or_abort(&err); > + g_assert(obj == NULL); > + > + obj = qobject_from_json("00", &err); Invalid as a JSON number > + g_assert(!err); /* BUG */ > + g_assert(obj == NULL); > + > + obj = qobject_from_json("[1e", &err); Incomplete as a JSON number > + g_assert(!err); /* BUG */ > g_assert(obj == NULL); > } Is it also worth testing: "t" (incomplete as a JSON literal) "a" (not a valid JSON literal, but alphabetic and thus different from the "@" test above) At any rate, with or without further tests this is good improved coverage dealt with in the rest of the series. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 08/08/2018 07:02 AM, Markus Armbruster wrote: >> qobject_from_json() can return null without setting an error on >> lexical errors. I call that a bug. Add test coverage to demonstrate >> it. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/check-qjson.c | 36 +++++++++++++++++++++++++++++++++--- >> 1 file changed, 33 insertions(+), 3 deletions(-) >> > >> +static void junk_input(void) >> +{ >> + /* Note: junk within strings is covered elsewhere */ >> + Error *err = NULL; >> + QObject *obj; >> + >> + obj = qobject_from_json("@", &err); > > Invalid token > >> + g_assert(!err); /* BUG */ >> + g_assert(obj == NULL); >> + >> + obj = qobject_from_json("[0\xFF]", &err); > > \xff stream reset, followed by unbalanced ] >> + error_free_or_abort(&err); >> + g_assert(obj == NULL); >> + >> + obj = qobject_from_json("00", &err); > > Invalid as a JSON number > >> + g_assert(!err); /* BUG */ >> + g_assert(obj == NULL); >> + >> + obj = qobject_from_json("[1e", &err); > > Incomplete as a JSON number > >> + g_assert(!err); /* BUG */ >> g_assert(obj == NULL); >> } > > Is it also worth testing: > > "t" (incomplete as a JSON literal) > "a" (not a valid JSON literal, but alphabetic and thus different from > the "@" test above) Yes, an invalid keyword is worth testing. The way the code works, testing one should suffice. > At any rate, with or without further tests this is good improved > coverage dealt with in the rest of the series. > > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
diff --git a/tests/check-qjson.c b/tests/check-qjson.c index cc952c56ea..81b92d6b0c 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -1307,8 +1307,36 @@ static void simple_varargs(void) static void empty_input(void) { - const char *empty = ""; - QObject *obj = qobject_from_json(empty, &error_abort); + QObject *obj = qobject_from_json("", &error_abort); + g_assert(obj == NULL); +} + +static void blank_input(void) +{ + QObject *obj = qobject_from_json("\n ", &error_abort); + g_assert(obj == NULL); +} + +static void junk_input(void) +{ + /* Note: junk within strings is covered elsewhere */ + Error *err = NULL; + QObject *obj; + + obj = qobject_from_json("@", &err); + g_assert(!err); /* BUG */ + g_assert(obj == NULL); + + obj = qobject_from_json("[0\xFF]", &err); + error_free_or_abort(&err); + g_assert(obj == NULL); + + obj = qobject_from_json("00", &err); + g_assert(!err); /* BUG */ + g_assert(obj == NULL); + + obj = qobject_from_json("[1e", &err); + g_assert(!err); /* BUG */ g_assert(obj == NULL); } @@ -1462,7 +1490,9 @@ int main(int argc, char **argv) g_test_add_func("/varargs/simple_varargs", simple_varargs); - g_test_add_func("/errors/empty_input", empty_input); + g_test_add_func("/errors/empty", empty_input); + g_test_add_func("/errors/blank", blank_input); + g_test_add_func("/errors/junk", junk_input); g_test_add_func("/errors/unterminated/string", unterminated_string); g_test_add_func("/errors/unterminated/escape", unterminated_escape); g_test_add_func("/errors/unterminated/sq_string", unterminated_sq_string);
qobject_from_json() can return null without setting an error on lexical errors. I call that a bug. Add test coverage to demonstrate it. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/check-qjson.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)