From e38ac9621c8ab51880b9e6e833a125342d6b46b2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 22 Mar 2012 12:51:10 +0100 Subject: qapi: add strict mode to input visitor While QMP in general is designed so that it is possible to ignore unknown arguments, in the case of the QMP server it is better to reject them to detect bad clients. In fact, we're already doing this at the top level in the argument checker. To extend this to complex structures, add a mode to the input visitor where it checks for unvisited keys and raises an error if it finds one. Signed-off-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- qapi/qmp-input-visitor.c | 48 +++++++++- qapi/qmp-input-visitor.h | 2 + test-qmp-input-strict.c | 234 +++++++++++++++++++++++++++++++++++++++++++++++ tests/Makefile | 5 +- 4 files changed, 285 insertions(+), 4 deletions(-) create mode 100644 test-qmp-input-strict.c diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 5765e76e43..74386b9b1b 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -24,6 +24,7 @@ typedef struct StackObject { QObject *obj; const QListEntry *entry; + GHashTable *h; } StackObject; struct QmpInputVisitor @@ -31,6 +32,7 @@ struct QmpInputVisitor Visitor visitor; StackObject stack[QIV_STACK_SIZE]; int nb_stack; + bool strict; }; static QmpInputVisitor *to_qiv(Visitor *v) @@ -45,6 +47,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, if (qobj) { if (name && qobject_type(qobj) == QTYPE_QDICT) { + if (qiv->stack[qiv->nb_stack - 1].h) { + g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name); + } return qdict_get(qobject_to_qdict(qobj), name); } else if (qiv->stack[qiv->nb_stack - 1].entry) { return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry); @@ -54,20 +59,47 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, return qobj; } +static void qdict_add_key(const char *key, QObject *obj, void *opaque) +{ + GHashTable *h = opaque; + g_hash_table_insert(h, (gpointer) key, NULL); +} + static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp) { - qiv->stack[qiv->nb_stack].obj = obj; - qiv->stack[qiv->nb_stack].entry = NULL; - qiv->nb_stack++; + GHashTable *h; if (qiv->nb_stack >= QIV_STACK_SIZE) { error_set(errp, QERR_BUFFER_OVERRUN); return; } + + qiv->stack[qiv->nb_stack].obj = obj; + qiv->stack[qiv->nb_stack].entry = NULL; + qiv->stack[qiv->nb_stack].h = NULL; + + if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) { + h = g_hash_table_new(g_str_hash, g_str_equal); + qdict_iter(qobject_to_qdict(obj), qdict_add_key, h); + qiv->stack[qiv->nb_stack].h = h; + } + + qiv->nb_stack++; } static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp) { + GHashTableIter iter; + gpointer key; + + if (qiv->strict && qiv->stack[qiv->nb_stack - 1].h) { + g_hash_table_iter_init(&iter, qiv->stack[qiv->nb_stack - 1].h); + if (g_hash_table_iter_next(&iter, &key, NULL)) { + error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key); + } + g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h); + } + assert(qiv->nb_stack > 0); qiv->nb_stack--; } @@ -262,3 +294,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) return v; } + +QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj) +{ + QmpInputVisitor *v; + + v = qmp_input_visitor_new(obj); + v->strict = true; + + return v; +} diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h index 3f798f0335..e0a48a5f3b 100644 --- a/qapi/qmp-input-visitor.h +++ b/qapi/qmp-input-visitor.h @@ -20,6 +20,8 @@ typedef struct QmpInputVisitor QmpInputVisitor; QmpInputVisitor *qmp_input_visitor_new(QObject *obj); +QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj); + void qmp_input_visitor_cleanup(QmpInputVisitor *v); Visitor *qmp_input_get_visitor(QmpInputVisitor *v); diff --git a/test-qmp-input-strict.c b/test-qmp-input-strict.c new file mode 100644 index 0000000000..f6df8cbe1e --- /dev/null +++ b/test-qmp-input-strict.c @@ -0,0 +1,234 @@ +/* + * QMP Input Visitor unit-tests (strict mode). + * + * Copyright (C) 2011-2012 Red Hat Inc. + * + * Authors: + * Luiz Capitulino + * Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include + +#include "qapi/qmp-input-visitor.h" +#include "test-qapi-types.h" +#include "test-qapi-visit.h" +#include "qemu-objects.h" + +typedef struct TestInputVisitorData { + QObject *obj; + QmpInputVisitor *qiv; +} TestInputVisitorData; + +static void validate_teardown(TestInputVisitorData *data, + const void *unused) +{ + qobject_decref(data->obj); + data->obj = NULL; + + if (data->qiv) { + qmp_input_visitor_cleanup(data->qiv); + data->qiv = NULL; + } +} + +/* This is provided instead of a test setup function so that the JSON + string used by the tests are kept in the test functions (and not + int main()) */ +static GCC_FMT_ATTR(2, 3) +Visitor *validate_test_init(TestInputVisitorData *data, + const char *json_string, ...) +{ + Visitor *v; + va_list ap; + + va_start(ap, json_string); + data->obj = qobject_from_jsonv(json_string, &ap); + va_end(ap); + + g_assert(data->obj != NULL); + + data->qiv = qmp_input_visitor_new_strict(data->obj); + g_assert(data->qiv != NULL); + + v = qmp_input_get_visitor(data->qiv); + g_assert(v != NULL); + + return v; +} + +typedef struct TestStruct +{ + int64_t integer; + bool boolean; + char *string; +} TestStruct; + +static void visit_type_TestStruct(Visitor *v, TestStruct **obj, + const char *name, Error **errp) +{ + visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct), + errp); + + visit_type_int(v, &(*obj)->integer, "integer", errp); + visit_type_bool(v, &(*obj)->boolean, "boolean", errp); + visit_type_str(v, &(*obj)->string, "string", errp); + + visit_end_struct(v, errp); +} + +static void test_validate_struct(TestInputVisitorData *data, + const void *unused) +{ + TestStruct *p = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }"); + + visit_type_TestStruct(v, &p, NULL, &errp); + g_assert(!error_is_set(&errp)); + g_free(p->string); + g_free(p); +} + +static void test_validate_struct_nested(TestInputVisitorData *data, + const void *unused) +{ + UserDefNested *udp = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string' }, 'string2': 'string2'}}}"); + + visit_type_UserDefNested(v, &udp, NULL, &errp); + g_assert(!error_is_set(&errp)); + qapi_free_UserDefNested(udp); +} + +static void test_validate_list(TestInputVisitorData *data, + const void *unused) +{ + UserDefOneList *head = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]"); + + visit_type_UserDefOneList(v, &head, NULL, &errp); + g_assert(!error_is_set(&errp)); + qapi_free_UserDefOneList(head); +} + +static void test_validate_union(TestInputVisitorData *data, + const void *unused) +{ + UserDefUnion *tmp = NULL; + Visitor *v; + Error *errp = NULL; + + v = validate_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 } }"); + + visit_type_UserDefUnion(v, &tmp, NULL, &errp); + g_assert(!error_is_set(&errp)); + qapi_free_UserDefUnion(tmp); +} + +static void test_validate_fail_struct(TestInputVisitorData *data, + const void *unused) +{ + TestStruct *p = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }"); + + visit_type_TestStruct(v, &p, NULL, &errp); + g_assert(error_is_set(&errp)); + if (p) { + g_free(p->string); + } + g_free(p); +} + +static void test_validate_fail_struct_nested(TestInputVisitorData *data, + const void *unused) +{ + UserDefNested *udp = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}"); + + visit_type_UserDefNested(v, &udp, NULL, &errp); + g_assert(error_is_set(&errp)); + qapi_free_UserDefNested(udp); +} + +static void test_validate_fail_list(TestInputVisitorData *data, + const void *unused) +{ + UserDefOneList *head = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]"); + + visit_type_UserDefOneList(v, &head, NULL, &errp); + g_assert(error_is_set(&errp)); + qapi_free_UserDefOneList(head); +} + +static void test_validate_fail_union(TestInputVisitorData *data, + const void *unused) +{ + UserDefUnion *tmp = NULL; + Error *errp = NULL; + Visitor *v; + + v = validate_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 }, 'extra': 'yyy' }"); + + visit_type_UserDefUnion(v, &tmp, NULL, &errp); + g_assert(error_is_set(&errp)); + qapi_free_UserDefUnion(tmp); +} + +static void validate_test_add(const char *testpath, + TestInputVisitorData *data, + void (*test_func)(TestInputVisitorData *data, const void *user_data)) +{ + g_test_add(testpath, TestInputVisitorData, data, NULL, test_func, + validate_teardown); +} + +int main(int argc, char **argv) +{ + TestInputVisitorData testdata; + + g_test_init(&argc, &argv, NULL); + + validate_test_add("/visitor/input-strict/pass/struct", + &testdata, test_validate_struct); + validate_test_add("/visitor/input-strict/pass/struct-nested", + &testdata, test_validate_struct_nested); + validate_test_add("/visitor/input-strict/pass/list", + &testdata, test_validate_list); + validate_test_add("/visitor/input-strict/pass/union", + &testdata, test_validate_union); + validate_test_add("/visitor/input-strict/fail/struct", + &testdata, test_validate_fail_struct); + validate_test_add("/visitor/input-strict/fail/struct-nested", + &testdata, test_validate_fail_struct_nested); + validate_test_add("/visitor/input-strict/fail/list", + &testdata, test_validate_fail_list); + validate_test_add("/visitor/input-strict/fail/union", + &testdata, test_validate_fail_union); + + g_test_run(); + + return 0; +} diff --git a/tests/Makefile b/tests/Makefile index 94ea3421ad..2a2fff7c6d 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -16,7 +16,7 @@ check-qfloat: check-qfloat.o qfloat.o $(tools-obj-y) check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y) test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(tools-obj-y) -test-qmp-input-visitor.o test-qmp-output-visitor.o \ +test-qmp-input-visitor.o test-qmp-output-visitor.o test-qmp-input-strict.o \ test-string-input-visitor.o test-string-output-visitor.o \ test-qmp-commands.o: QEMU_CFLAGS += -I $(qapi-dir) @@ -37,6 +37,9 @@ test-string-output-visitor: test-string-output-visitor.o $(qobject-obj-y) $(qapi test-string-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) test-string-input-visitor: test-string-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o +test-qmp-input-strict.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) +test-qmp-input-strict: test-qmp-input-strict.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o + test-qmp-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) test-qmp-output-visitor: test-qmp-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o -- cgit v1.2.1