From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3RBn-0003p4-BH for qemu-devel@nongnu.org; Thu, 19 May 2016 12:52:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b3RBi-0001RI-BZ for qemu-devel@nongnu.org; Thu, 19 May 2016 12:52:47 -0400 Received: from resqmta-po-06v.sys.comcast.net ([2001:558:fe16:19:96:114:154:165]:58385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3RBi-0001R1-3W for qemu-devel@nongnu.org; Thu, 19 May 2016 12:52:42 -0400 From: Eric Blake Date: Thu, 19 May 2016 10:52:23 -0600 Message-Id: <1463676743-22133-1-git-send-email-eblake@redhat.com> In-Reply-To: <1463632874-28559-1-git-send-email-eblake@redhat.com> References: <1463632874-28559-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v4 29/28] qapi: Add strict mode to JSON output visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth , Luiz Capitulino , Kevin Wolf , Max Reitz , "open list:Block layer core" Let the caller decide whether output must be strict JSON (and raise an error on an attempt to output an encoding error or non-finite number), vs. the status quo of relaxed (encoding errors are rewritten to use substitute U+fffd characters, and non-finite numbers are output). Adjust the testsuite to cover this: check-qobject-json checks relaxed mode (since qobject_to_json() is unchanged in behavior), test-qmp-output-visitor checks that QObject doesn't care about JSON restrictions, and test-json-output-visitor is now in strict mode and flags the errors. Signed-off-by: Eric Blake --- v4: split out as new patch [was parts of 04/18 and 07/18 in v3] --- include/qapi/json-output-visitor.h | 7 ++++- include/qapi/qmp/qobject-json.h | 3 +- qapi/json-output-visitor.c | 19 ++++++++---- qemu-img.c | 6 ++-- qobject/qobject-json.c | 61 ++++++++++++++++++++++++-------------- tests/check-qobject-json.c | 15 ++++++++-- tests/test-json-output-visitor.c | 17 +++++++++-- tests/test-qmp-output-visitor.c | 17 +++++++++++ 8 files changed, 107 insertions(+), 38 deletions(-) diff --git a/include/qapi/json-output-visitor.h b/include/qapi/json-output-visitor.h index 414f91b..2ce2758 100644 --- a/include/qapi/json-output-visitor.h +++ b/include/qapi/json-output-visitor.h @@ -23,7 +23,12 @@ typedef struct JsonOutputVisitor JsonOutputVisitor; * * If @pretty, make the output legible with newlines and indentation; * otherwise the output uses a single line. + * + * If @strict, attempts to output invalid JSON (strings not properly + * encoded in UTF-8, or Infinity or NaN as a number) will be treated + * as an error; otherwise, output is best-effort even if not pure + * JSON. */ -Visitor *json_output_visitor_new(bool pretty, char **result); +Visitor *json_output_visitor_new(bool pretty, bool strict, char **result); #endif diff --git a/include/qapi/qmp/qobject-json.h b/include/qapi/qmp/qobject-json.h index 28e44b2..30bc753 100644 --- a/include/qapi/qmp/qobject-json.h +++ b/include/qapi/qmp/qobject-json.h @@ -31,7 +31,8 @@ QString *qobject_to_json(const QObject *obj, bool pretty); * (ignored for a top-level visit, must be set if part of a * dictionary, should be NULL if part of a list). */ -void qobject_visit_output(Visitor *v, const char *name, const QObject *obj); +void qobject_visit_output(Visitor *v, const char *name, const QObject *obj, + Error **errp); int qstring_append_json_string(QString *qstring, const char *str); int qstring_append_json_number(QString *qstring, double number); diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c index ba647d3..28e95ee 100644 --- a/qapi/json-output-visitor.c +++ b/qapi/json-output-visitor.c @@ -13,11 +13,13 @@ #include "qapi/visitor-impl.h" #include "qapi/qmp/qstring.h" #include "qapi/qmp/qobject-json.h" +#include "qapi/error.h" struct JsonOutputVisitor { Visitor visitor; QString *str; bool pretty; + bool strict; bool comma; unsigned int depth; char **result; @@ -133,8 +135,10 @@ static void json_output_type_str(Visitor *v, const char *name, char **obj, assert(*obj); json_output_name(jov, name); - /* FIXME: report invalid UTF-8 encoding */ - qstring_append_json_string(jov->str, *obj); + if (qstring_append_json_string(jov->str, *obj) < 0 && jov->strict) { + error_setg(errp, "string value of '%s' is not valid UTF-8", + name ? name : "null"); + } } static void json_output_type_number(Visitor *v, const char *name, double *obj, @@ -143,14 +147,16 @@ static void json_output_type_number(Visitor *v, const char *name, double *obj, JsonOutputVisitor *jov = to_jov(v); json_output_name(jov, name); - /* FIXME: report Inf/NaN problems */ - qstring_append_json_number(jov->str, *obj); + if (qstring_append_json_number(jov->str, *obj) < 0 && jov->strict) { + error_setg(errp, "number value of '%s' is not finite", + name ? name : "null"); + } } static void json_output_type_any(Visitor *v, const char *name, QObject **obj, Error **errp) { - qobject_visit_output(v, name, *obj); + qobject_visit_output(v, name, *obj, errp); } static void json_output_type_null(Visitor *v, const char *name, Error **errp) @@ -181,12 +187,13 @@ static void json_output_free(Visitor *v) g_free(jov); } -Visitor *json_output_visitor_new(bool pretty, char **result) +Visitor *json_output_visitor_new(bool pretty, bool strict, char **result) { JsonOutputVisitor *v; v = g_malloc0(sizeof(*v)); v->pretty = pretty; + v->strict = strict; v->result = result; *result = NULL; v->str = qstring_new(); diff --git a/qemu-img.c b/qemu-img.c index d5dc19b..b0c6dd3 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -480,7 +480,7 @@ fail: static void dump_json_image_check(ImageCheck *check, bool quiet) { char *str; - Visitor *v = json_output_visitor_new(true, &str); + Visitor *v = json_output_visitor_new(true, true, &str); visit_type_ImageCheck(v, NULL, &check, &error_abort); visit_complete(v, &str); @@ -2166,7 +2166,7 @@ static void dump_snapshots(BlockDriverState *bs) static void dump_json_image_info_list(ImageInfoList *list) { char *str; - Visitor *v = json_output_visitor_new(true, &str); + Visitor *v = json_output_visitor_new(true, true, &str); visit_type_ImageInfoList(v, NULL, &list, &error_abort); visit_complete(v, &str); @@ -2178,7 +2178,7 @@ static void dump_json_image_info_list(ImageInfoList *list) static void dump_json_image_info(ImageInfo *info) { char *str; - Visitor *v = json_output_visitor_new(true, &str); + Visitor *v = json_output_visitor_new(true, true, &str); visit_type_ImageInfo(v, NULL, &info, &error_abort); visit_complete(v, &str); diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c index 05b020a..cf81ab5 100644 --- a/qobject/qobject-json.c +++ b/qobject/qobject-json.c @@ -72,62 +72,81 @@ QObject *qobject_from_jsonf(const char *string, ...) return obj; } +typedef struct ToJson { +{ + Visitor *v; + Error **errp; +} ToJson; + static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) { - Visitor *v = opaque; + ToJson *s = opaque; - qobject_visit_output(v, key, obj); + if (!*s->errp) { + qobject_visit_output(s->v, key, obj, s->errp); + } } static void to_json_list_iter(QObject *obj, void *opaque) { - Visitor *v = opaque; + ToJson *s = opaque; - qobject_visit_output(v, NULL, obj); + if (!*s->errp) { + qobject_visit_output(s->v, NULL, obj, s->errp); + } } -void qobject_visit_output(Visitor *v, const char *name, const QObject *obj) +void qobject_visit_output(Visitor *v, const char *name, const QObject *obj, + Error **errp) { + Error *err = NULL; + ToJson s = { .v = v, .errp = &err, }; + switch (qobject_type(obj)) { case QTYPE_QNULL: - visit_type_null(v, name, &error_abort); + visit_type_null(v, name, errp); break; case QTYPE_QINT: { int64_t val = qint_get_int(qobject_to_qint(obj)); - visit_type_int64(v, name, &val, &error_abort); + visit_type_int64(v, name, &val, errp); break; } case QTYPE_QSTRING: { const char *str = qstring_get_str(qobject_to_qstring(obj)); - /* FIXME: no way inform user if we modified the string to - * avoid encoding errors */ - visit_type_str(v, name, (char **)&str, &error_abort); + visit_type_str(v, name, (char **)&str, errp); break; } case QTYPE_QDICT: { QDict *val = qobject_to_qdict(obj); - visit_start_struct(v, name, NULL, 0, &error_abort); - qdict_iter(val, to_json_dict_iter, v); - visit_check_struct(v, &error_abort); + visit_start_struct(v, name, NULL, 0, &err); + if (!err) { + qdict_iter(val, to_json_dict_iter, &s); + } + if (!err) { + visit_check_struct(v, &err); + } + error_propagate(errp, err); visit_end_struct(v, NULL); break; } case QTYPE_QLIST: { QList *val = qobject_to_qlist(obj); - visit_start_list(v, name, NULL, 0, &error_abort); - qlist_iter(val, to_json_list_iter, v); + visit_start_list(v, name, NULL, 0, &err); + if (!err) { + qlist_iter(val, to_json_list_iter, &s); + } + error_propagate(errp, err); visit_end_list(v, NULL); break; } case QTYPE_QFLOAT: { double val = qfloat_get_double(qobject_to_qfloat(obj)); - /* FIXME: no way inform user if we generated invalid JSON */ - visit_type_number(v, name, &val, NULL); + visit_type_number(v, name, &val, errp); break; } case QTYPE_QBOOL: { bool val = qbool_get_bool(qobject_to_qbool(obj)); - visit_type_bool(v, name, &val, &error_abort); + visit_type_bool(v, name, &val, errp); break; } default: @@ -138,9 +157,9 @@ void qobject_visit_output(Visitor *v, const char *name, const QObject *obj) QString *qobject_to_json(const QObject *obj, bool pretty) { char *str; - Visitor *v = json_output_visitor_new(pretty, &str); + Visitor *v = json_output_visitor_new(pretty, false, &str); - qobject_visit_output(v, NULL, obj); + qobject_visit_output(v, NULL, obj, &error_abort); visit_complete(v, &str); visit_free(v); return qstring_wrap_str(str); @@ -222,8 +241,6 @@ int qstring_append_json_number(QString *qstring, double number) /* FIXME: snprintf() is locale dependent; but JSON requires * numbers to be formatted as if in the C locale. Dependence * on C locale is a pervasive issue in QEMU. */ - /* FIXME: This risks printing Inf or NaN, which are not valid - * JSON values. */ /* FIXME: the default precision of 6 for %f often causes * rounding errors; we should be using DBL_DECIMAL_DIG (17), * and only rounding to a shorter number if the result would diff --git a/tests/check-qobject-json.c b/tests/check-qobject-json.c index 267fc67..b4f3dc4 100644 --- a/tests/check-qobject-json.c +++ b/tests/check-qobject-json.c @@ -12,6 +12,7 @@ */ #include "qemu/osdep.h" #include +#include #include "qapi/qmp/types.h" #include "qapi/qmp/qobject-json.h" @@ -920,6 +921,8 @@ static void simple_number(void) static void float_number(void) { int i; + QFloat *qfloat; + QString *str; struct { const char *encoded; double decoded; @@ -934,7 +937,6 @@ static void float_number(void) for (i = 0; test_cases[i].encoded; i++) { QObject *obj; - QFloat *qfloat; obj = qobject_from_json(test_cases[i].encoded); g_assert(obj != NULL); @@ -944,8 +946,6 @@ static void float_number(void) g_assert(qfloat_get_double(qfloat) == test_cases[i].decoded); if (test_cases[i].skip == 0) { - QString *str; - str = qobject_to_json(obj, false); g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0); QDECREF(str); @@ -953,6 +953,15 @@ static void float_number(void) QDECREF(qfloat); } + + /* Although Infinity/NaN are not valid JSON, we can generate them + * on output as an extension. + * FIXME: The parser doesn't handle infinity/NaN on input. */ + qfloat = qfloat_from_double(INFINITY); + str = qobject_to_json(QOBJECT(qfloat), false); + g_assert_cmpstr(qstring_get_str(str), ==, "inf"); + QDECREF(str); + QDECREF(qfloat); } static void vararg_number(void) diff --git a/tests/test-json-output-visitor.c b/tests/test-json-output-visitor.c index fd14e26..e0478bc 100644 --- a/tests/test-json-output-visitor.c +++ b/tests/test-json-output-visitor.c @@ -14,6 +14,7 @@ #include "qemu/osdep.h" #include +#include #include "qemu-common.h" #include "qapi/json-output-visitor.h" @@ -32,7 +33,7 @@ static void visitor_output_setup(TestOutputVisitorData *data, const void *arg) { const bool *pretty = arg; - data->ov = json_output_visitor_new(*pretty, &data->str); + data->ov = json_output_visitor_new(*pretty, true, &data->str); g_assert(data->ov); data->pretty = *pretty; } @@ -89,13 +90,18 @@ static void test_visitor_out_number(TestOutputVisitorData *data, { double value = 3.14; const char *out; + Error *err = NULL; visit_type_number(data->ov, NULL, &value, &error_abort); out = visitor_get(data); g_assert_cmpstr(out, ==, "3.14"); - /* FIXME: JSON requires finite numbers */ + /* JSON requires finite numbers */ + visitor_reset(data); + value = INFINITY; + visit_type_number(data->ov, NULL, &value, &err); + error_free_or_abort(&err); } static void test_visitor_out_string(TestOutputVisitorData *data, @@ -103,11 +109,18 @@ static void test_visitor_out_string(TestOutputVisitorData *data, { char *string = (char *) "Q E M U"; const char *out; + Error *err = NULL; visit_type_str(data->ov, NULL, &string, &error_abort); out = visitor_get(data); g_assert_cmpstr(out, ==, "\"Q E M U\""); + + /* JSON requires valid UTF-8 encodings */ + visitor_reset(data); + string = (char *) "\xfe"; + visit_type_str(data->ov, NULL, &string, &err); + error_free_or_abort(&err); } static void test_visitor_out_enum(TestOutputVisitorData *data, diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index 0db0b19..68e074f 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -17,6 +17,7 @@ #include "qemu/osdep.h" #include +#include #include "qemu-common.h" #include "qapi/error.h" @@ -97,6 +98,14 @@ static void test_visitor_out_number(TestOutputVisitorData *data, obj = visitor_get(data); g_assert(qobject_type(obj) == QTYPE_QFLOAT); g_assert(qfloat_get_double(qobject_to_qfloat(obj)) == value); + + /* While JSON requires finite values, QObject does not. */ + visitor_reset(data); + value = INFINITY; + visit_type_number(data->ov, NULL, &value, &error_abort); + obj = visitor_get(data); + g_assert(qobject_type(obj) == QTYPE_QFLOAT); + g_assert(qfloat_get_double(qobject_to_qfloat(obj)) == INFINITY); } static void test_visitor_out_string(TestOutputVisitorData *data, @@ -110,6 +119,14 @@ static void test_visitor_out_string(TestOutputVisitorData *data, obj = visitor_get(data); g_assert(qobject_type(obj) == QTYPE_QSTRING); g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==, string); + + /* While JSON requires valid UTF-8 encoding, QObject does not. */ + visitor_reset(data); + string = (char *) "\xfe"; + visit_type_str(data->ov, NULL, &string, &error_abort); + obj = visitor_get(data); + g_assert(qobject_type(obj) == QTYPE_QSTRING); + g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==, string); } static void test_visitor_out_no_string(TestOutputVisitorData *data, -- 2.5.5