Commit 2cbf0992 authored by Eric Blake's avatar Eric Blake Committed by Markus Armbruster
Browse files

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: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
parent 10d4d997

scripts/qapi.py

+17 −5
Original line number Diff line number Diff line
@@ -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,

tests/qapi-schema/type-bypass-bad-gen.err

+1 −0
Original line number Diff line number Diff line 
tests/qapi-schema/type-bypass-bad-gen.json:2: 'gen' of command 'foo' should only use false value

tests/qapi-schema/type-bypass-bad-gen.exit

+1 −1
Original line number Diff line number Diff line 
0
 
1

tests/qapi-schema/type-bypass-bad-gen.json

+1 −1
Original line number Diff line number Diff line 
# FIXME: 'gen' should only appear with value false

# 'gen' should only appear with value false

{ 'command': 'foo', 'gen': 'whatever' }

tests/qapi-schema/type-bypass-bad-gen.out

+0 −3
Original line number Diff line number Diff line 
[OrderedDict([('command', 'foo'), ('gen', 'whatever')])]

[]

[]