Re: [Qemu-devel] [PATCH v2 13/23] tests: Clean up string interpolation around qtest_qmp_device_add()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 07/30/2018 03:34 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>> [...]
>>> (We really want to assert that any % interpolations in our JSON parser
>>> are NOT embedded in '').
>>
>> I'll look into that, but it'll be in a separate series.
>
> Agreed.  In fact, my more ambitious series had reached that point by
> implementing %% interpolation inside strings, combined with asserting
> that %% cannot occur except within strings during the JSON parse, then
> during the JSON interpolation that the only use of % within strings
> was the %% escape (so that we no longer risk consuming a va-arg during
> string interpolation, while still benefiting from gcc's -Wformat
> checking).  So probably one of the easier things to revive, once this
> series lands.

The problem: the compiler recognizes conversion specifications the JSON
parser doesn't recognize.  Can lead to crashes or silent misbehavior.

The JSON lexer recognizes conversion specification tokens, and the JSON
parser accepts them as JSON value.  The lexer rejects conversion
specification tokens we don't support.  Conversion specifications within
tokens are not recognized.  Since only string tokens can contain '%',
conversion specifications can hide from the JSON parser only there.

I can see three ways to fix that.  All three make the JSON parser
recognize all conversion specifications.  They differ in which ones
fail.

1. Obey:

   The ones inside strings work as in sprintf().

   Example:
   "{ 'str': %s, 'act': '%.0f%%', 'max': '100%%' }", str, p * 100.0

   Encourages interpolation into strings, which is problematic, as
   explained in PATCH 11 "tests: Clean up string interpolation into QMP
   input (simple cases)".  Moreover, supporting some conversion
   specifications only in strings (e.g. %g, %.0f, %%) could be
   confusing.

2. Obey "%%" in strings, reject the rest

   You can only interpolate strings, not into strings.  To put a '%'
   intro a string, you have to double it.

   Example:
   "{ 'str': %s, 'pct': %s, 'max': '100%%' }", str, pct
   where pct = g_strdup_printf("%0.f%%", p * 100.0)

   Strings are interpreted differently when the JSON parser has
   interpolation enabled.  That's the price we have to pay for not
   having to interpolate strings containing '%'.

3. Reject:

   You can't have '%' in strings.  To get a string containing '%', you
   have to interpolate it.

   Example:
   "{ 'str': %s, 'pct': %s, 'max': %s }", str, pct, "100%"
   where pct = g_strdup_printf("%0.f%%", p * 100.0)

   This is the simplest solution.