Re: [Qemu-devel] [PATCH v2 06/16] qapi: Restrict strings to printable ASCII

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 9/10/19 1:37 AM, Markus Armbruster wrote:
>> RFC 8259 on string contents:
>> 
>>    All Unicode characters may be placed within the quotation marks,
>>    except for the characters that MUST be escaped: quotation mark,
>>    reverse solidus, and the control characters (U+0000 through
>>    U+001F).
>> 
>> The QAPI schema parser accepts both less and more than JSON: it
>> accepts only ASCII with \u (less), and accepts control characters
>> other than LF (new line) unescaped.  How it treats unescaped non-ASCII
>> input differs between Python 2 and Python 3.
>> 
>> Make it accept strictly less: require printable ASCII.  Drop support
>> for \b, \f, \n, \r, \t.
>
> Fair enough.  It doesn't prevent QMP clients from sending strings with
> non-ASCII characters, merely that those strings will never match the
> schema because we have guaranteed our schema is limited to ASCII.  (This

Note that this only affects QMP commands, events, parameter and enum
value names, *not* non-enum string arguments.

> change means we are promising to never allow { "execute": "a\tb" } as a
> valid QMP command, for instance.)

We're not actually promising anything.  We're merely making it slightly
harder to do: need to revert this patch first.  Feels quite unlikely,
though.

>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
>> @@ -523,17 +523,7 @@ class QAPISchemaParser(object):
>>                      if ch == '\n':
>>                          raise QAPIParseError(self, 'Missing terminating 
>> "\'"')
>>                      if esc:
>> -                        if ch == 'b':
>> -                            string += '\b'
>> -                        elif ch == 'f':
>> -                            string += '\f'
>> -                        elif ch == 'n':
>> -                            string += '\n'
>
> Is it worth a comment in the code that we are specifically not parsing
> all possible JSON escapes, because of the later requirement that QAPI
> strings be limited to the subset of printable ASCII?

Can't hurt.

> Reviewed-by: Eric Blake <address@hidden>

Thanks!