Re: A brief look at deprecating our JSON extensions over RFC 8259

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Daniel P. Berrangé <berrange@xxxxxxxxxx> writes:

> On Mon, Feb 22, 2021 at 06:47:30PM +0100, Paolo Bonzini wrote:
>> On 22/02/21 16:24, Daniel P. Berrangé wrote:
>> > This problem isn't unique to QEMU. Any app using JSON from the
>> > shell will have the tedium of quote escaping. JSON is incredibly
>> > widespread and no other apps felt it neccessary to introduce single
>> > quoting support, because the benefit doesn't outweigh the interop
>> > problem it introduces.
>> 
>> The quotes were introduced for C code (and especially qtest), not for the
>> shell.  We have something like
>> 
>>     response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
>>                    "'property': 'temperature' } }", id);
>> 
>> These are sent to QEMU as double-quoted strings (the single-quoted JSON is
>> parsed to get interpolation and printed back; commit 563890c7c7, "libqtest:
>> escape strings in QMP commands, fix leak", 2014-07-01). However, doing the
>> interpolation requires a parser that recognizes the single-quoted strings.
>
> IMHO this is the wrong solution to the problem.  Consider the equivalent
> libvirt code that uses a standard JSON library underneath and has a high
> level API to serialize args into the command
>
>       qemuMonitorJSONMakeCommand("qom-get",
>                                  "s:path", id,
> 				 "s:property", "temperature");
>
> Of course this example is reasonably easy since it is a flat set of
> arguments. Nested args get slightly more complicated, but still always
> preferrable to doing string interpolation IMHO.

Misunderstanding: our JSON interpolation feature is *not* string
interpolation!  It interpolates *objects* into the QObject built by the
parser.

Best explained with an example.  The qmp() call above internally builds
the following QObject:

    QDict mapping
        key "execute" to a QString for "qom-get"
        key "arguments" to a QDict mapping
            key "path" to a QString for @id (interpolation!)
            key "property" to a QString for "temperature"

Unlike string interpolation, this is safe for any valid C string @id.

If you think like a hacker, you might try shenanigans like

    "{'execute': '%s'}"

You will then find that somebody has thought like a hacker before
you[*].

The %-sequences are cleverly chosen to get some protection against
common mistakes from the compiler.

This interpolation has worked quite well for us, and I have no plans to
replace it.  Doesn't mean I'm against replacing it, only that I want to
see benefits exceeding the costs.

A bigger stumbling block for replacement is our need for a streaming
interface: we feed the parser characters, and expect to be called back
when an expression is complete.


[*] Commit 16a4859921 "json: Improve safety of
qobject_from_jsonf_nofail() & friends", fixed up in commit bbc0586ced
"json: Fix % handling when not interpolating".




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux