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>

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 2402d053e4c6ae17159c68b9863a495ebd53ae65..e391b5a649a670fb67d9520d22f6d64db16c2197 100644 (file)
--- 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 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a83c3c655de2d0b8d6255f0f0d78ec647cafb5d5 100644 (file)
--- 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 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
--- 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 bb70bee085b81938374de6ddcddbdfdc6f7a709b..e8dec342492d01efb0eebc31b84a60454cb6f30b 100644 (file)
--- 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 e678f2c18e116d13fc4c98349b91768e449944d7..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
--- 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 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..20cef0a8a7fd5477578564a8a0dc1d032275a3ee 100644 (file)
--- 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 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
--- 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 af87c191adf88b58d69f1eb4a23ebd28ef6644db..4feae3719c07f5f4b4f1a45be3f6fe0c0cb5f80b 100644 (file)
--- 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 8b2a9ac94a48f45ef424b7ea80d492429607bc45..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
--- 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', '**')])]
-[]
-[]