From 7a0b9af6626e0b11231b8fb068a49dab2d5d2bd7 Mon Sep 17 00:00:00 2001 From: Vincent Bernat Date: Sat, 15 Feb 2014 17:23:53 +0100 Subject: [PATCH 1/2] Allow to mix JSON_STRICT with optional keys On unpack, one may want to mix `JSON_STRICT` and optional keys by using a format like `{s:i,s?o!}`. Unfortunately, this fails the stric test with `-1 object item(s) left unpacked` error when the second key is not specified. To fix that, we iter on each key and we check if we have successfully unpacked them. This is less efficient than the previous method but it brings correctness. --- src/pack_unpack.c | 18 ++++++++++++++---- test/suites/api/test_unpack.c | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/pack_unpack.c b/src/pack_unpack.c index 76d946b..abed654 100644 --- a/src/pack_unpack.c +++ b/src/pack_unpack.c @@ -432,10 +432,20 @@ static int unpack_object(scanner_t *s, json_t *root, va_list *ap) if(strict == 0 && (s->flags & JSON_STRICT)) strict = 1; - if(root && strict == 1 && key_set.size != json_object_size(root)) { - long diff = (long)json_object_size(root) - (long)key_set.size; - set_error(s, "", "%li object item(s) left unpacked", diff); - goto out; + if(root && strict == 1) { + /* We need to check that all non optional items have been parsed */ + const char *key; + json_t *value; + long unpacked = 0; + json_object_foreach(root, key, value) { + if(!hashtable_get(&key_set, key)) { + unpacked++; + } + } + if (unpacked) { + set_error(s, "", "%li object item(s) left unpacked", unpacked); + goto out; + } } ret = 0; diff --git a/test/suites/api/test_unpack.c b/test/suites/api/test_unpack.c index fac1be3..6b76106 100644 --- a/test/suites/api/test_unpack.c +++ b/test/suites/api/test_unpack.c @@ -378,4 +378,23 @@ static void run_tests() if(i1 != 42) fail("json_unpack failed to unpack"); json_decref(j); + + /* Combine ? and ! */ + j = json_pack("{si}", "foo", 42); + i1 = i2 = 0; + if(json_unpack(j, "{sis?i!}", "foo", &i1, "bar", &i2)) + fail("json_unpack failed for optional values with strict mode"); + if(i1 != 42) + fail("json_unpack failed to unpack"); + if(i2 != 0) + fail("json_unpack failed to unpack"); + json_decref(j); + + /* But don't compensate a missing key with an optional one. */ + j = json_pack("{sisi}", "foo", 42, "baz", 43); + i1 = i2 = i3 = 0; + if(!json_unpack_ex(j, &error, 0, "{sis?i!}", "foo", &i1, "bar", &i2)) + fail("json_unpack failed for optional values with strict mode and compensation"); + check_error("1 object item(s) left unpacked", "", 1, 8, 8); + json_decref(j); } From 56a50e147dabdb57459a19c6c00f194b2ac49d24 Mon Sep 17 00:00:00 2001 From: Vincent Bernat Date: Sat, 15 Feb 2014 17:44:02 +0100 Subject: [PATCH 2/2] Micro-optimization for JSON_STRICT when no optional key is used The previous commit introduced a loop on all input keys to check the strict mode. We can avoid this if we don't expect an optional key. In this case, we fallback to the previous method to compare the length of the set of expected keys and the length of the parsed keys. --- src/pack_unpack.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/pack_unpack.c b/src/pack_unpack.c index abed654..af381a5 100644 --- a/src/pack_unpack.c +++ b/src/pack_unpack.c @@ -350,6 +350,7 @@ static int unpack_object(scanner_t *s, json_t *root, va_list *ap) { int ret = -1; int strict = 0; + int gotopt = 0; /* Use a set (emulated by a hashtable) to check that all object keys are accessed. Checking that the correct number of keys @@ -406,7 +407,7 @@ static int unpack_object(scanner_t *s, json_t *root, va_list *ap) next_token(s); if(token(s) == '?') { - opt = 1; + opt = gotopt = 1; next_token(s); } @@ -437,10 +438,16 @@ static int unpack_object(scanner_t *s, json_t *root, va_list *ap) const char *key; json_t *value; long unpacked = 0; - json_object_foreach(root, key, value) { - if(!hashtable_get(&key_set, key)) { - unpacked++; + if (gotopt) { + /* We have optional keys, we need to iter on each key */ + json_object_foreach(root, key, value) { + if(!hashtable_get(&key_set, key)) { + unpacked++; + } } + } else { + /* No optional keys, we can just compare the number of items */ + unpacked = (long)json_object_size(root) - (long)key_set.size; } if (unpacked) { set_error(s, "", "%li object item(s) left unpacked", unpacked);