From: Marc-André Lureau <marcandre.lur...@redhat.com> The generated marshal functions do not visit arguments from commands that take no arguments. Thus they fail to catch invalid members. Visit the arguments, if provided, to throw an error in case of invalid members.
static void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp) { + Visitor *v = NULL; Error *err = NULL; - - (void)args; + if (args) { + v = qmp_input_visitor_new(QOBJECT(args), true); + visit_start_struct(v, NULL, NULL, 0, &err); + if (err) { + goto out; + } + + if (!err) { + visit_check_struct(v, &err); + } + visit_end_struct(v, NULL); + if (err) { + goto out; + } + } qmp_stop(&err); + +out: error_propagate(errp, err); + visit_free(v); + if (args) { + v = qapi_dealloc_visitor_new(); + visit_start_struct(v, NULL, NULL, 0, NULL); + + visit_end_struct(v, NULL); + visit_free(v); + } } Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> --- tests/test-qmp-commands.c | 15 ++++++++++++++ scripts/qapi-commands.py | 51 ++++++++++++++++++++++++++++++----------------- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 261fd9e..81cbe54 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -106,6 +106,7 @@ static void test_dispatch_cmd(void) static void test_dispatch_cmd_failure(void) { QDict *req = qdict_new(); + QDict *args = qdict_new(); QObject *resp; qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2"))); @@ -114,6 +115,20 @@ static void test_dispatch_cmd_failure(void) assert(resp != NULL); assert(qdict_haskey(qobject_to_qdict(resp), "error")); + qobject_decref(resp); + QDECREF(req); + + /* check that with extra arguments it throws an error */ + req = qdict_new(); + qdict_put(args, "a", qint_from_int(66)); + qdict_put(req, "arguments", args); + + qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd"))); + + resp = qmp_dispatch(QOBJECT(req)); + assert(resp != NULL); + assert(qdict_haskey(qobject_to_qdict(resp), "error")); + qobject_decref(resp); QDECREF(req); } diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 11aa54b..735e463 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -97,11 +97,27 @@ def gen_marshal_decl(name, export): proto=gen_marshal_proto(name, export)) +def if_args(arg_type, block): + ret = '' + if not arg_type or arg_type.is_empty(): + ret += mcgen(''' + if (args) { +''') + push_indent() + ret += block() + if not arg_type or arg_type.is_empty(): + pop_indent() + ret += mcgen(''' + } +''') + return ret + def gen_marshal(name, arg_type, boxed, ret_type, export): ret = mcgen(''' %(proto)s { + Visitor *v = NULL; Error *err = NULL; ''', proto=gen_marshal_proto(name, export)) @@ -112,17 +128,23 @@ def gen_marshal(name, arg_type, boxed, ret_type, export): ''', c_type=ret_type.c_type()) + visit_members = '' if arg_type and not arg_type.is_empty(): + visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, &err);', + c_name=arg_type.c_name()) ret += mcgen(''' - Visitor *v; %(c_name)s arg = {0}; +''', + c_name=arg_type.c_name()) + + ret += if_args(arg_type, lambda: mcgen(''' v = qmp_input_visitor_new(QOBJECT(args), true); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; } - visit_type_%(c_name)s_members(v, &arg, &err); + %(visit_members)s if (!err) { visit_check_struct(v, &err); } @@ -131,35 +153,28 @@ def gen_marshal(name, arg_type, boxed, ret_type, export): goto out; } ''', - c_name=arg_type.c_name()) - - else: - ret += mcgen(''' - - (void)args; -''') + visit_members=visit_members)) ret += gen_call(name, arg_type, boxed, ret_type) # 'goto out' produced above for arg_type, and by gen_call() for ret_type - if (arg_type and not arg_type.is_empty()) or ret_type: - ret += mcgen(''' + if arg_type and not arg_type.is_empty(): + visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, NULL);', + c_name=arg_type.c_name()) + ret += mcgen(''' out: -''') - ret += mcgen(''' error_propagate(errp, err); -''') - if arg_type and not arg_type.is_empty(): - ret += mcgen(''' visit_free(v); +''') + ret += if_args(arg_type, lambda: mcgen(''' v = qapi_dealloc_visitor_new(); visit_start_struct(v, NULL, NULL, 0, NULL); - visit_type_%(c_name)s_members(v, &arg, NULL); + %(visit_members)s visit_end_struct(v, NULL); visit_free(v); ''', - c_name=arg_type.c_name()) + visit_members=visit_members)) ret += mcgen(''' } -- 2.9.0