[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor
From: |
Laurent Desnogues |
Subject: |
Re: [Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor |
Date: |
Mon, 2 Apr 2012 12:34:41 +0200 |
Hello,
On Thu, Mar 22, 2012 at 12:51 PM, Paolo Bonzini <address@hidden> wrote:
> 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 <address@hidden>
> ---
> 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 97a0186..eb09874 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;
GHashTableIter is alas not available in the glib (2.12) that
the distros we use at work run. Is there a workaround for
this issue?
Thanks,
Laurent
> + 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--;
> }
> @@ -257,3 +289,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 3f798f0..e0a48a5 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 0000000..f6df8cb
> --- /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 <address@hidden>
> + * Paolo Bonzini <address@hidden>
> + *
> + * 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 <glib.h>
> +#include <stdarg.h>
> +
> +#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 c78ade1..31349f7 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -15,7 +15,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-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
>
> @@ -36,6 +36,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
>
> --
> 1.7.9.1
>
>
>
- Re: [Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor,
Laurent Desnogues <=