From a0067da1577e3eb0c60758384282568f4b2328fe Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Mon, 12 Sep 2016 13:19:08 +0400 Subject: qapi: check invalid arguments on no-args commands 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. Currently, qmp_check_client_args() checks for invalid arguments and correctly catches this case. When switching to qmp_dispatch() we want to keep that behaviour. The commands using 'O' may have arbitrary arguments, and must have 'gen': false in the qapi schema to skip the generated checks. Old/new diff: void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp) { Error *err = NULL; + Visitor *v = 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); + } } The new code closely resembles code for a command with arguments. Differences: - the visit of the argument and its cleanup struct don't visit any members (because there are none). - the visit of the argument struct and its cleanup are conditional. Signed-off-by: Marc-André Lureau Message-Id: <20160912091913.15831-14-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- tests/test-qmp-commands.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'tests') diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 261fd9e313..81cbe545c4 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"))); @@ -116,6 +117,20 @@ static void test_dispatch_cmd_failure(void) 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); } static QObject *test_qmp_dispatch(QDict *req) -- cgit v1.2.3-55-g7522 From 4bdadd86718c901fc1a512a7e3e0cbe7518cf277 Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Mon, 12 Sep 2016 13:19:09 +0400 Subject: tests: add a test to check invalid args Check that invalid args on commands without arguments returns an error. Signed-off-by: Marc-André Lureau Message-Id: <20160912091913.15831-15-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- tests/test-qga.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'tests') diff --git a/tests/test-qga.c b/tests/test-qga.c index 21f44f8915..40af64987a 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -198,6 +198,26 @@ static void test_qga_ping(gconstpointer fix) QDECREF(ret); } +static void test_qga_invalid_args(gconstpointer fix) +{ + const TestFixture *fixture = fix; + QDict *ret, *error; + const gchar *class, *desc; + + ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping', " + "'arguments': {'foo': 42 }}"); + g_assert_nonnull(ret); + + error = qdict_get_qdict(ret, "error"); + class = qdict_get_try_str(error, "class"); + desc = qdict_get_try_str(error, "desc"); + + g_assert_cmpstr(class, ==, "GenericError"); + g_assert_cmpstr(desc, ==, "QMP input object member 'foo' is unexpected"); + + QDECREF(ret); +} + static void test_qga_invalid_cmd(gconstpointer fix) { const TestFixture *fixture = fix; @@ -911,6 +931,7 @@ int main(int argc, char **argv) g_test_add_data_func("/qga/file-write-read", &fix, test_qga_file_write_read); g_test_add_data_func("/qga/get-time", &fix, test_qga_get_time); g_test_add_data_func("/qga/invalid-cmd", &fix, test_qga_invalid_cmd); + g_test_add_data_func("/qga/invalid-args", &fix, test_qga_invalid_args); g_test_add_data_func("/qga/fsfreeze-status", &fix, test_qga_fsfreeze_status); -- cgit v1.2.3-55-g7522