From 6ce80fd80355d29b3ed8c2fa14251a9b8276a86a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 6 Aug 2018 08:53:27 +0200 Subject: qobject: Replace qobject_from_jsonf() by qobject_from_jsonf_nofail() Commit ab45015a968 "qobject: Let qobject_from_jsonf() fail instead of abort" fails to accomplish its stated aim: the function can still abort due to its use of &error_abort. Its rationale for letting it fail is that all remaining users cope fine with failure. Well, they're just fine with aborting, too; it's what they do on failure. Simply reverting the broken commit would bring back the unfortunate asymmetry between qobject_from_jsonf() and qobject_from_jsonv(): one aborts, the other returns null. So also rename it to qobject_from_jsonf_nofail(). Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth Reviewed-by: Eric Blake Message-Id: <20180806065344.7103-7-armbru@redhat.com> --- qobject/qjson.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'qobject') diff --git a/qobject/qjson.c b/qobject/qjson.c index 2f6a590e44..4a9dcff343 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -59,7 +59,12 @@ QObject *qobject_from_json(const char *string, Error **errp) return qobject_from_jsonv(string, NULL, errp); } -QObject *qobject_from_jsonf(const char *string, ...) +/* + * Parse @string as JSON value with %-escapes interpolated. + * Abort on error. Do not use with untrusted @string. + * Return the resulting QObject. It is never null. + */ +QObject *qobject_from_jsonf_nofail(const char *string, ...) { QObject *obj; va_list ap; @@ -68,6 +73,7 @@ QObject *qobject_from_jsonf(const char *string, ...) obj = qobject_from_jsonv(string, &ap, &error_abort); va_end(ap); + assert(obj); return obj; } -- cgit v1.2.3-55-g7522 From 4ff184689bf3d22b01c0d00c2bf6bf9595ff9b48 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 6 Aug 2018 08:53:28 +0200 Subject: qobject: New qobject_from_vjsonf_nofail(), qdict_from_vjsonf_nofail() Every printf()-like function sooner or later needs its vprintf()-like buddy. The next commit will need qobject_from_jsonf_nofail()'s buddy, and qdict_from_jsonf_nofail()'s buddy will be used later in this series. Add both. Signed-off-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Eric Blake Message-Id: <20180806065344.7103-8-armbru@redhat.com> --- include/qapi/qmp/qjson.h | 4 ++++ qobject/qjson.c | 44 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 7 deletions(-) (limited to 'qobject') diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h index dc509d51ae..dce78583dc 100644 --- a/include/qapi/qmp/qjson.h +++ b/include/qapi/qmp/qjson.h @@ -18,8 +18,12 @@ QObject *qobject_from_json(const char *string, Error **errp); QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp) GCC_FMT_ATTR(1, 0); +QObject *qobject_from_vjsonf_nofail(const char *string, va_list ap) + GCC_FMT_ATTR(1, 0); QObject *qobject_from_jsonf_nofail(const char *string, ...) GCC_FMT_ATTR(1, 2); +QDict *qdict_from_vjsonf_nofail(const char *string, va_list ap) + GCC_FMT_ATTR(1, 0); QDict *qdict_from_jsonf_nofail(const char *string, ...) GCC_FMT_ATTR(1, 2); diff --git a/qobject/qjson.c b/qobject/qjson.c index 4a9dcff343..2e450231ff 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -59,6 +59,25 @@ QObject *qobject_from_json(const char *string, Error **errp) return qobject_from_jsonv(string, NULL, errp); } +/* + * Parse @string as JSON value with %-escapes interpolated. + * Abort on error. Do not use with untrusted @string. + * Return the resulting QObject. It is never null. + */ +QObject *qobject_from_vjsonf_nofail(const char *string, va_list ap) +{ + va_list ap_copy; + QObject *obj; + + /* va_copy() is needed when va_list is an array type */ + va_copy(ap_copy, ap); + obj = qobject_from_jsonv(string, &ap_copy, &error_abort); + va_end(ap_copy); + + assert(obj); + return obj; +} + /* * Parse @string as JSON value with %-escapes interpolated. * Abort on error. Do not use with untrusted @string. @@ -70,13 +89,26 @@ QObject *qobject_from_jsonf_nofail(const char *string, ...) va_list ap; va_start(ap, string); - obj = qobject_from_jsonv(string, &ap, &error_abort); + obj = qobject_from_vjsonf_nofail(string, ap); va_end(ap); - assert(obj); return obj; } +/* + * Parse @string as JSON object with %-escapes interpolated. + * Abort on error. Do not use with untrusted @string. + * Return the resulting QDict. It is never null. + */ +QDict *qdict_from_vjsonf_nofail(const char *string, va_list ap) +{ + QDict *qdict; + + qdict = qobject_to(QDict, qobject_from_vjsonf_nofail(string, ap)); + assert(qdict); + return qdict; +} + /* * Parse @string as JSON object with %-escapes interpolated. * Abort on error. Do not use with untrusted @string. @@ -84,15 +116,13 @@ QObject *qobject_from_jsonf_nofail(const char *string, ...) */ QDict *qdict_from_jsonf_nofail(const char *string, ...) { - QDict *obj; + QDict *qdict; va_list ap; va_start(ap, string); - obj = qobject_to(QDict, qobject_from_jsonv(string, &ap, &error_abort)); + qdict = qdict_from_vjsonf_nofail(string, ap); va_end(ap); - - assert(obj); - return obj; + return qdict; } typedef struct ToJsonIterState -- cgit v1.2.3-55-g7522 From 2d36e843042d2ef47f3bfc47a1a83401fdb07b84 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 6 Aug 2018 08:53:31 +0200 Subject: qobject: qobject_from_jsonv() is dangerous, hide it away qobject_from_jsonv() takes ownership of %p arguments. On failure, we can't generally know whether we failed before or after %p, so ownership becomes indeterminate. To avoid leaks, callers passing %p must terminate on error, e.g. by passing &error_abort. Trap for the unwary; document and give the function internal linkage. Signed-off-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Eric Blake Message-Id: <20180806065344.7103-11-armbru@redhat.com> --- include/qapi/qmp/qjson.h | 2 -- qobject/qjson.c | 13 ++++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) (limited to 'qobject') diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h index dce78583dc..5ebbe5a118 100644 --- a/include/qapi/qmp/qjson.h +++ b/include/qapi/qmp/qjson.h @@ -15,8 +15,6 @@ #define QJSON_H QObject *qobject_from_json(const char *string, Error **errp); -QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp) - GCC_FMT_ATTR(1, 0); QObject *qobject_from_vjsonf_nofail(const char *string, va_list ap) GCC_FMT_ATTR(1, 0); diff --git a/qobject/qjson.c b/qobject/qjson.c index 2e450231ff..ab4040f235 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -39,7 +39,18 @@ static void parse_json(JSONMessageParser *parser, GQueue *tokens) s->result = json_parser_parse_err(tokens, s->ap, &s->err); } -QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp) +/* + * Parse @string as JSON value. + * If @ap is non-null, interpolate %-escapes. + * Takes ownership of %p arguments. + * On success, return the JSON value. + * On failure, store an error through @errp and return NULL. + * Ownership of %p arguments becomes indeterminate then. To avoid + * leaks, callers passing %p must terminate on error, e.g. by passing + * &error_abort. + */ +static QObject *qobject_from_jsonv(const char *string, va_list *ap, + Error **errp) { JSONParsingState state = {}; -- cgit v1.2.3-55-g7522