[libvirt] [PATCH 0/2] Test JSON parsing with invalid input
Ján Tomko (2): Test if JSON parser fails on invalid input Error out on unterminated arrays and objects in JSON parser src/util/virjson.c | 9 ++++++++- tests/jsontest.c | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) -- 1.8.1.5
--- tests/jsontest.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/jsontest.c b/tests/jsontest.c index 6add816..808a2ea 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -145,6 +145,10 @@ mymain(void) #define DO_TEST_PARSE(name, doc) \ DO_TEST_FULL(name, FromString, doc, NULL, true) +#define DO_TEST_PARSE_FAIL(name, doc) \ + DO_TEST_FULL(name, FromString, doc, NULL, false) + + DO_TEST_PARSE("Simple", "{\"return\": {}, \"id\": \"libvirt-1\"}"); DO_TEST_PARSE("NotSoSimple", "{\"QMP\": {\"version\": {\"qemu\":" "{\"micro\": 91, \"minor\": 13, \"major\": 0}," @@ -188,6 +192,15 @@ mymain(void) DO_TEST_FULL("add and remove", AddRemove, "[ 1 ]", NULL, false); + + DO_TEST_PARSE_FAIL("nothing", ""); + DO_TEST_PARSE_FAIL("number with garbage", "2345b45"); + DO_TEST_PARSE_FAIL("float with garbage", "0.0314159ee+100"); + DO_TEST_PARSE_FAIL("unterminated string", "\"The meaning of lif"); + DO_TEST_PARSE_FAIL("object with numeric keys", "{ 1:1, 2:1, 3:2 }"); + DO_TEST_PARSE_FAIL("array of an object with an array as a key", + "[ {[\"key1\", \"key2\"]: \"value\"} ]"); + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.1.5
On Tue, Nov 05, 2013 at 03:59:55PM +0100, Ján Tomko wrote:
--- tests/jsontest.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/jsontest.c b/tests/jsontest.c index 6add816..808a2ea 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -145,6 +145,10 @@ mymain(void) #define DO_TEST_PARSE(name, doc) \ DO_TEST_FULL(name, FromString, doc, NULL, true)
+#define DO_TEST_PARSE_FAIL(name, doc) \ + DO_TEST_FULL(name, FromString, doc, NULL, false) + +
You're testing virJSONValueFromString() here ...
DO_TEST_PARSE("Simple", "{\"return\": {}, \"id\": \"libvirt-1\"}"); DO_TEST_PARSE("NotSoSimple", "{\"QMP\": {\"version\": {\"qemu\":" "{\"micro\": 91, \"minor\": 13, \"major\": 0}," @@ -188,6 +192,15 @@ mymain(void) DO_TEST_FULL("add and remove", AddRemove, "[ 1 ]", NULL, false);
+ + DO_TEST_PARSE_FAIL("nothing", ""); + DO_TEST_PARSE_FAIL("number with garbage", "2345b45"); + DO_TEST_PARSE_FAIL("float with garbage", "0.0314159ee+100"); + DO_TEST_PARSE_FAIL("unterminated string", "\"The meaning of lif");
... that not only cannot parse these, but it can't even parse proper numbers (e.g. "12345", "1.2e+34"), because it expects object (array or map). You can do two things with this. Either create new testJSON...() which will test numbers only (requires v2) or squash this in (which makes it work properly and shows that also): diff --git a/tests/jsontest.c b/tests/jsontest.c index e806b6f..a7ee2ef 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -193,10 +193,18 @@ mymain(void) "[ 1 ]", NULL, false); + DO_TEST_PARSE("almost nothing", "[]"); DO_TEST_PARSE_FAIL("nothing", ""); - DO_TEST_PARSE_FAIL("number with garbage", "2345b45"); - DO_TEST_PARSE_FAIL("float with garbage", "0.0314159ee+100"); - DO_TEST_PARSE_FAIL("unterminated string", "\"The meaning of lif"); + + DO_TEST_PARSE("number without garbage", "[ 234545 ]"); + DO_TEST_PARSE_FAIL("number with garbage", "[ 2345b45 ]"); + + DO_TEST_PARSE("float without garbage", "[ 0.0314159e+100 ]"); + DO_TEST_PARSE_FAIL("float with garbage", "[ 0.0314159ee+100 ]"); + + DO_TEST_PARSE("string", "[ \"The meaning of life\" ]"); + DO_TEST_PARSE_FAIL("unterminated string", "[ \"The meaning of lif ]"); + DO_TEST_PARSE_FAIL("unterminated array", "[ 1, 2, 3"); DO_TEST_PARSE_FAIL("object with numeric keys", "{ 1:1, 2:1, 3:2 }"); DO_TEST_PARSE_FAIL("unterminated object", "{ \"1\":1, \"2\":1, \"3\":2"); -- ACK if you want to use the second option. Martin
On 11/15/2013 08:47 AM, Martin Kletzander wrote:
On Tue, Nov 05, 2013 at 03:59:55PM +0100, Ján Tomko wrote:
--- tests/jsontest.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
ACK if you want to use the second option.
I've changed it as you suggested and pushed it. Thanks! Jan
--- src/util/virjson.c | 9 ++++++++- tests/jsontest.c | 5 +++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 2bb7324..f0a06ab 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1004,7 +1004,14 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring) goto cleanup; } - ret = parser.head; + if (parser.nstate != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse json %s: unterminated string/map/array"), + jsonstring); + virJSONValueFree(parser.head); + } else { + ret = parser.head; + } cleanup: yajl_free(hand); diff --git a/tests/jsontest.c b/tests/jsontest.c index 808a2ea..e806b6f 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -197,9 +197,14 @@ mymain(void) DO_TEST_PARSE_FAIL("number with garbage", "2345b45"); DO_TEST_PARSE_FAIL("float with garbage", "0.0314159ee+100"); DO_TEST_PARSE_FAIL("unterminated string", "\"The meaning of lif"); + DO_TEST_PARSE_FAIL("unterminated array", "[ 1, 2, 3"); DO_TEST_PARSE_FAIL("object with numeric keys", "{ 1:1, 2:1, 3:2 }"); + DO_TEST_PARSE_FAIL("unterminated object", "{ \"1\":1, \"2\":1, \"3\":2"); + DO_TEST_PARSE_FAIL("unterminated array of objects", + "[ {\"name\": \"John\"}, {\"name\": \"Paul\"}, "); DO_TEST_PARSE_FAIL("array of an object with an array as a key", "[ {[\"key1\", \"key2\"]: \"value\"} ]"); + DO_TEST_PARSE_FAIL("object with unterminated key", "{ \"key:7 }"); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.1.5
On Tue, Nov 05, 2013 at 03:59:56PM +0100, Ján Tomko wrote:
--- src/util/virjson.c | 9 ++++++++- tests/jsontest.c | 5 +++++ 2 files changed, 13 insertions(+), 1 deletion(-)
ACK, Martin
participants (2)
-
Ján Tomko -
Martin Kletzander