From 33d60b8d57b0dc9e1a7b700ab5c9060d170247f7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 4 Apr 2019 16:40:02 +0200 Subject: [PATCH 1/2] json: simplify JSON_VARIANT_OBJECT_FOREACH() macro a bit There's no point in returning the "key" within each loop iteration as JsonVariant object. Let's simplify things and return it as string. That simplifies usage (since the caller doesn't have to convert the object to the string anymore) and is safe since we already validate that keys are strings when an object JsonVariant is allocated. --- src/nspawn/nspawn-oci.c | 24 +++++++++++------------- src/shared/json.h | 2 +- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/nspawn/nspawn-oci.c b/src/nspawn/nspawn-oci.c index db8e4d3b78..6040243899 100644 --- a/src/nspawn/nspawn-oci.c +++ b/src/nspawn/nspawn-oci.c @@ -1621,26 +1621,26 @@ static bool sysctl_key_valid(const char *s) { static int oci_sysctl(const char *name, JsonVariant *v, JsonDispatchFlags flags, void *userdata) { Settings *s = userdata; - JsonVariant *k, *w; + JsonVariant *w; + const char *k; int r; assert(s); JSON_VARIANT_OBJECT_FOREACH(k, w, v) { - const char *n, *m; + const char *m; if (!json_variant_is_string(w)) return json_log(v, flags, SYNTHETIC_ERRNO(EINVAL), "sysctl parameter is not a string, refusing."); - assert_se(n = json_variant_string(k)); assert_se(m = json_variant_string(w)); - if (sysctl_key_valid(n)) + if (sysctl_key_valid(k)) return json_log(v, flags, SYNTHETIC_ERRNO(EINVAL), - "sysctl key invalid, refusing: %s", n); + "sysctl key invalid, refusing: %s", k); - r = strv_extend_strv(&s->sysctl, STRV_MAKE(n, m), false); + r = strv_extend_strv(&s->sysctl, STRV_MAKE(k, m), false); if (r < 0) return log_oom(); } @@ -2171,22 +2171,20 @@ static int oci_hooks(const char *name, JsonVariant *v, JsonDispatchFlags flags, } static int oci_annotations(const char *name, JsonVariant *v, JsonDispatchFlags flags, void *userdata) { - JsonVariant *k, *w; + JsonVariant *w; + const char *k; JSON_VARIANT_OBJECT_FOREACH(k, w, v) { - const char *n; - assert_se(n = json_variant_string(k)); - - if (isempty(n)) - return json_log(k, flags, SYNTHETIC_ERRNO(EINVAL), + if (isempty(k)) + return json_log(v, flags, SYNTHETIC_ERRNO(EINVAL), "Annotation with empty key, refusing."); if (!json_variant_is_string(w)) return json_log(w, flags, SYNTHETIC_ERRNO(EINVAL), "Annotation has non-string value, refusing."); - json_log(k, flags|JSON_DEBUG, 0, "Ignoring annotation '%s' with value '%s'.", n, json_variant_string(w)); + json_log(w, flags|JSON_DEBUG, 0, "Ignoring annotation '%s' with value '%s'.", k, json_variant_string(w)); } return 0; diff --git a/src/shared/json.h b/src/shared/json.h index e5532c506e..675ce2091e 100644 --- a/src/shared/json.h +++ b/src/shared/json.h @@ -143,7 +143,7 @@ struct json_variant_foreach_state { #define JSON_VARIANT_OBJECT_FOREACH(k, e, v) \ for (struct json_variant_foreach_state _state = { (v), 0 }; \ _state.idx < json_variant_elements(_state.variant) && \ - ({ k = json_variant_by_index(_state.variant, _state.idx); \ + ({ k = json_variant_string(json_variant_by_index(_state.variant, _state.idx)); \ e = json_variant_by_index(_state.variant, _state.idx + 1); \ true; }); \ _state.idx += 2) From 1b266e3c6f462dd835d3890ab4b1cb316b6fc205 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 12 Apr 2019 12:59:05 +0200 Subject: [PATCH 2/2] json: be more careful when iterating through a JSON object/array Let's exit the loop early in case the variant is not actually an object or array. This is safer since otherwise we might end up iterating through these variants and access fields that aren't of the type we expect them to be and then bad things happen. Of course, this doesn't absolve uses of these macros to check the type of the variant explicitly beforehand, but it makes it less bad if they forget to do so. --- src/shared/json.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shared/json.h b/src/shared/json.h index 675ce2091e..70dfe70dfd 100644 --- a/src/shared/json.h +++ b/src/shared/json.h @@ -135,14 +135,16 @@ struct json_variant_foreach_state { #define JSON_VARIANT_ARRAY_FOREACH(i, v) \ for (struct json_variant_foreach_state _state = { (v), 0 }; \ - _state.idx < json_variant_elements(_state.variant) && \ + json_variant_is_array(_state.variant) && \ + _state.idx < json_variant_elements(_state.variant) && \ ({ i = json_variant_by_index(_state.variant, _state.idx); \ true; }); \ _state.idx++) #define JSON_VARIANT_OBJECT_FOREACH(k, e, v) \ for (struct json_variant_foreach_state _state = { (v), 0 }; \ - _state.idx < json_variant_elements(_state.variant) && \ + json_variant_is_object(_state.variant) && \ + _state.idx < json_variant_elements(_state.variant) && \ ({ k = json_variant_string(json_variant_by_index(_state.variant, _state.idx)); \ e = json_variant_by_index(_state.variant, _state.idx + 1); \ true; }); \