From 2cbf09925ad45401673a79ab77f67de2f04a826c Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 4 May 2015 09:05:24 -0600
Subject: [PATCH] qapi: More rigorous checking for type safety bypass
Now that we have a way to validate every type, we can also be
stricter about enforcing that callers that want to bypass
type safety in generated code. Prior to this patch, it didn't
matter what value was associated with the key 'gen', but it
looked odd that 'gen':'yes' could result in bypassing the
generated code. These changes also enforce the changes made
earlier in the series for documentation and consolidation of
using '**' as the wildcard type, as well as 'gen':false as the
canonical spelling for requesting type bypass.
Note that 'gen':false is a one-way switch away from the default;
we do not support 'gen':true (similar for 'success-response').
In practice, this doesn't matter.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 22 +++++++++++++++++-----
tests/qapi-schema/type-bypass-bad-gen.err | 1 +
tests/qapi-schema/type-bypass-bad-gen.exit | 2 +-
tests/qapi-schema/type-bypass-bad-gen.json | 2 +-
tests/qapi-schema/type-bypass-bad-gen.out | 3 ---
tests/qapi-schema/type-bypass-no-gen.err | 1 +
tests/qapi-schema/type-bypass-no-gen.exit | 2 +-
tests/qapi-schema/type-bypass-no-gen.json | 2 +-
tests/qapi-schema/type-bypass-no-gen.out | 3 ---
9 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 2402d053e4..e391b5a649 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -324,14 +324,15 @@ def check_name(expr_info, source, name, allow_optional = False,
"%s uses invalid name '%s'" % (source, name))
def check_type(expr_info, source, value, allow_array = False,
- allow_dict = False, allow_optional = False, allow_metas = []):
+ allow_dict = False, allow_optional = False,
+ allow_star = False, allow_metas = []):
global all_names
orig_value = value
if value is None:
return
- if value == '**':
+ if allow_star and value == '**':
return
# Check if array type for value is okay
@@ -348,6 +349,10 @@ def check_type(expr_info, source, value, allow_array = False,
# Check if type name for value is okay
if isinstance(value, str):
+ if value == '**':
+ raise QAPIExprError(expr_info,
+ "%s uses '**' but did not request 'gen':false"
+ % source)
if not value in all_names:
raise QAPIExprError(expr_info,
"%s uses unknown type '%s'"
@@ -371,19 +376,22 @@ def check_type(expr_info, source, value, allow_array = False,
check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
allow_array=True, allow_dict=True, allow_optional=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
- 'enum'])
+ 'enum'], allow_star=allow_star)
def check_command(expr, expr_info):
name = expr['command']
+ allow_star = expr.has_key('gen')
+
check_type(expr_info, "'data' for command '%s'" % name,
expr.get('data'), allow_dict=True, allow_optional=True,
- allow_metas=['union', 'struct'])
+ allow_metas=['union', 'struct'], allow_star=allow_star)
returns_meta = ['union', 'struct']
if name in returns_whitelist:
returns_meta += ['built-in', 'alternate', 'enum']
check_type(expr_info, "'returns' for command '%s'" % name,
expr.get('returns'), allow_array=True, allow_dict=True,
- allow_optional=True, allow_metas=returns_meta)
+ allow_optional=True, allow_metas=returns_meta,
+ allow_star=allow_star)
def check_event(expr, expr_info):
global events
@@ -579,6 +587,10 @@ def check_keys(expr_elem, meta, required, optional=[]):
raise QAPIExprError(info,
"Unknown key '%s' in %s '%s'"
% (key, meta, name))
+ if (key == 'gen' or key == 'success-response') and value != False:
+ raise QAPIExprError(info,
+ "'%s' of %s '%s' should only use false value"
+ % (key, meta, name))
for key in required:
if not expr.has_key(key):
raise QAPIExprError(info,
diff --git a/tests/qapi-schema/type-bypass-bad-gen.err b/tests/qapi-schema/type-bypass-bad-gen.err
index e69de29bb2..a83c3c655d 100644
--- a/tests/qapi-schema/type-bypass-bad-gen.err
+++ b/tests/qapi-schema/type-bypass-bad-gen.err
@@ -0,0 +1 @@
+tests/qapi-schema/type-bypass-bad-gen.json:2: 'gen' of command 'foo' should only use false value
diff --git a/tests/qapi-schema/type-bypass-bad-gen.exit b/tests/qapi-schema/type-bypass-bad-gen.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/type-bypass-bad-gen.exit
+++ b/tests/qapi-schema/type-bypass-bad-gen.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/type-bypass-bad-gen.json b/tests/qapi-schema/type-bypass-bad-gen.json
index bb70bee085..e8dec34249 100644
--- a/tests/qapi-schema/type-bypass-bad-gen.json
+++ b/tests/qapi-schema/type-bypass-bad-gen.json
@@ -1,2 +1,2 @@
-# FIXME: 'gen' should only appear with value false
+# 'gen' should only appear with value false
{ 'command': 'foo', 'gen': 'whatever' }
diff --git a/tests/qapi-schema/type-bypass-bad-gen.out b/tests/qapi-schema/type-bypass-bad-gen.out
index e678f2c18e..e69de29bb2 100644
--- a/tests/qapi-schema/type-bypass-bad-gen.out
+++ b/tests/qapi-schema/type-bypass-bad-gen.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'foo'), ('gen', 'whatever')])]
-[]
-[]
diff --git a/tests/qapi-schema/type-bypass-no-gen.err b/tests/qapi-schema/type-bypass-no-gen.err
index e69de29bb2..20cef0a8a7 100644
--- a/tests/qapi-schema/type-bypass-no-gen.err
+++ b/tests/qapi-schema/type-bypass-no-gen.err
@@ -0,0 +1 @@
+tests/qapi-schema/type-bypass-no-gen.json:2: Member 'arg' of 'data' for command 'unsafe' uses '**' but did not request 'gen':false
diff --git a/tests/qapi-schema/type-bypass-no-gen.exit b/tests/qapi-schema/type-bypass-no-gen.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/type-bypass-no-gen.exit
+++ b/tests/qapi-schema/type-bypass-no-gen.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/type-bypass-no-gen.json b/tests/qapi-schema/type-bypass-no-gen.json
index af87c191ad..4feae3719c 100644
--- a/tests/qapi-schema/type-bypass-no-gen.json
+++ b/tests/qapi-schema/type-bypass-no-gen.json
@@ -1,2 +1,2 @@
-# FIXME: type bypass should only work with 'gen':false
+# type bypass only works with 'gen':false
{ 'command': 'unsafe', 'data': { 'arg': '**' }, 'returns': '**' }
diff --git a/tests/qapi-schema/type-bypass-no-gen.out b/tests/qapi-schema/type-bypass-no-gen.out
index 8b2a9ac94a..e69de29bb2 100644
--- a/tests/qapi-schema/type-bypass-no-gen.out
+++ b/tests/qapi-schema/type-bypass-no-gen.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'unsafe'), ('data', OrderedDict([('arg', '**')])), ('returns', '**')])]
-[]
-[]