Re: [Qemu-devel] [PATCH for-2.6 12/14] qemu-img: In 'map', use QDict to generate JSON output

[Eric Blake]

On 11/23/2015 10:22 PM, Fam Zheng wrote:
> Switch json output generation from hand-written to QDict encoder, so
> that the coming str field will be properly escaped.
> 
> Iotest 122 output is updated accordingly.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  qemu-img.c                 | 30 +++++++++++----
>  tests/qemu-iotests/122.out | 96 
> ++++++++++++++++++++++++++--------------------
>  2 files changed, 76 insertions(+), 50 deletions(-)
> 

>      case OFORMAT_JSON:
> -        printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": 
> %d,"
> -               " \"zero\": %s, \"data\": %s",
> -               (e->start == 0 ? "[" : ",\n"),
> -               e->start, e->length, e->depth,
> -               (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false",
> -               (e->flags & BDRV_BLOCK_DATA) ? "true" : "false");
> +        if (e->start == 0) {
> +            printf("[");
> +        } else {
> +            printf(",");
> +        }
> +
> +        dict = qdict_new();
> +        qdict_put(dict, "start", qint_from_int(e->start));
> +        qdict_put(dict, "length", qint_from_int(e->length));
> +        qdict_put(dict, "depth", qint_from_int(e->depth));
> +        qdict_put(dict, "zero", qbool_from_bool(e->flags & BDRV_BLOCK_ZERO));
> +        qdict_put(dict, "data", qbool_from_bool(e->flags & BDRV_BLOCK_DATA));
>          if (e->flags & BDRV_BLOCK_OFFSET_VALID) {
> -            printf(", \"offset\": %"PRId64"", e->offset);
> +            qdict_put(dict, "offset", qint_from_int(e->offset));
>          }
> -        putchar('}');
> +        str = qobject_to_json(QOBJECT(dict));
> +        printf("%s\n", qstring_get_str(str));

The conversion is sane...

> +++ b/tests/qemu-iotests/122.out
> @@ -49,12 +49,13 @@ read 65536/65536 bytes at offset 4194304
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  read 65536/65536 bytes at offset 8388608
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": true},
> -{ "start": 65536, "length": 4128768, "depth": 0, "zero": true, "data": 
> false},
> -{ "start": 4194304, "length": 65536, "depth": 0, "zero": false, "data": 
> true},
> -{ "start": 4259840, "length": 4128768, "depth": 0, "zero": true, "data": 
> false},
> -{ "start": 8388608, "length": 65536, "depth": 0, "zero": false, "data": 
> true},
> -{ "start": 8454144, "length": 4128768, "depth": 0, "zero": true, "data": 
> false}]
> +[{"length": 65536, "start": 0, "zero": false, "depth": 0, "data": true}

...but the output shows that QDict output is tied to internal hashing
and therefore different than first-in-first-out creation order.
JSON-wise, it's still valid.  But are we guaranteed that our hashing is
stable?  If so, is that something an attacker could attempt to exploit
as a Denial of Service, by intentionally creating predictable
collisions?  [I'm guessing not, as the denial of service is mainly an
issue if a user can degrade performance of typical lookups from nominal
O(1) to O(n) by supplying LOTS of user-provided input, but the keys we
output in JSON are generally under our control via .json files and not
something where the user is dynamically creating lots of keys - but
still worth asking.]  And if it is not stable, then will our test break
when someone else's system hashes differently than yours?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature