mirror of https://github.com/xemu-project/xemu.git
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>
This commit is contained in:
parent
10d4d997f8
commit
2cbf09925a
|
@ -324,14 +324,15 @@ def check_name(expr_info, source, name, allow_optional = False,
|
||||||
"%s uses invalid name '%s'" % (source, name))
|
"%s uses invalid name '%s'" % (source, name))
|
||||||
|
|
||||||
def check_type(expr_info, source, value, allow_array = False,
|
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
|
global all_names
|
||||||
orig_value = value
|
orig_value = value
|
||||||
|
|
||||||
if value is None:
|
if value is None:
|
||||||
return
|
return
|
||||||
|
|
||||||
if value == '**':
|
if allow_star and value == '**':
|
||||||
return
|
return
|
||||||
|
|
||||||
# Check if array type for value is okay
|
# 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
|
# Check if type name for value is okay
|
||||||
if isinstance(value, str):
|
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:
|
if not value in all_names:
|
||||||
raise QAPIExprError(expr_info,
|
raise QAPIExprError(expr_info,
|
||||||
"%s uses unknown type '%s'"
|
"%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,
|
check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
|
||||||
allow_array=True, allow_dict=True, allow_optional=True,
|
allow_array=True, allow_dict=True, allow_optional=True,
|
||||||
allow_metas=['built-in', 'union', 'alternate', 'struct',
|
allow_metas=['built-in', 'union', 'alternate', 'struct',
|
||||||
'enum'])
|
'enum'], allow_star=allow_star)
|
||||||
|
|
||||||
def check_command(expr, expr_info):
|
def check_command(expr, expr_info):
|
||||||
name = expr['command']
|
name = expr['command']
|
||||||
|
allow_star = expr.has_key('gen')
|
||||||
|
|
||||||
check_type(expr_info, "'data' for command '%s'" % name,
|
check_type(expr_info, "'data' for command '%s'" % name,
|
||||||
expr.get('data'), allow_dict=True, allow_optional=True,
|
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']
|
returns_meta = ['union', 'struct']
|
||||||
if name in returns_whitelist:
|
if name in returns_whitelist:
|
||||||
returns_meta += ['built-in', 'alternate', 'enum']
|
returns_meta += ['built-in', 'alternate', 'enum']
|
||||||
check_type(expr_info, "'returns' for command '%s'" % name,
|
check_type(expr_info, "'returns' for command '%s'" % name,
|
||||||
expr.get('returns'), allow_array=True, allow_dict=True,
|
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):
|
def check_event(expr, expr_info):
|
||||||
global events
|
global events
|
||||||
|
@ -579,6 +587,10 @@ def check_keys(expr_elem, meta, required, optional=[]):
|
||||||
raise QAPIExprError(info,
|
raise QAPIExprError(info,
|
||||||
"Unknown key '%s' in %s '%s'"
|
"Unknown key '%s' in %s '%s'"
|
||||||
% (key, meta, name))
|
% (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:
|
for key in required:
|
||||||
if not expr.has_key(key):
|
if not expr.has_key(key):
|
||||||
raise QAPIExprError(info,
|
raise QAPIExprError(info,
|
||||||
|
|
|
@ -0,0 +1 @@
|
||||||
|
tests/qapi-schema/type-bypass-bad-gen.json:2: 'gen' of command 'foo' should only use false value
|
|
@ -1 +1 @@
|
||||||
0
|
1
|
||||||
|
|
|
@ -1,2 +1,2 @@
|
||||||
# FIXME: 'gen' should only appear with value false
|
# 'gen' should only appear with value false
|
||||||
{ 'command': 'foo', 'gen': 'whatever' }
|
{ 'command': 'foo', 'gen': 'whatever' }
|
||||||
|
|
|
@ -1,3 +0,0 @@
|
||||||
[OrderedDict([('command', 'foo'), ('gen', 'whatever')])]
|
|
||||||
[]
|
|
||||||
[]
|
|
|
@ -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
|
|
@ -1 +1 @@
|
||||||
0
|
1
|
||||||
|
|
|
@ -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': '**' }
|
{ 'command': 'unsafe', 'data': { 'arg': '**' }, 'returns': '**' }
|
||||||
|
|
|
@ -1,3 +0,0 @@
|
||||||
[OrderedDict([('command', 'unsafe'), ('data', OrderedDict([('arg', '**')])), ('returns', '**')])]
|
|
||||||
[]
|
|
||||||
[]
|
|
Loading…
Reference in New Issue