[Top][All Lists]

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

Re: [Qemu-devel] [PATCH 47/56] qjson: Have qobject_from_json() & friends

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 47/56] qjson: Have qobject_from_json() & friends reject empty and blank
Date: Thu, 16 Aug 2018 17:40:47 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> The last case where qobject_from_json() & friends return null without
>> setting an error is empty or blank input.  Callers:
>> * block.c's parse_json_protocol() reports "Could not parse the JSON
>>    options".  It's marked as a work-around, because it also covered
>>    actual bugs, but they got fixed in the previous few commits.
> How would you trigger this?

$ qemu-system-x86_64 json:{}
qemu-system-x86_64: Must specify either driver or file
$ qemu-system-x86_64 json:
qemu-system-x86_64: Could not parse the JSON options

>                             I guess that would be by using the
> pseud-json block specification spelled "json:" rather than the usual
> "json:{...}".
>> * qobject_input_visitor_new_str() reports "JSON parse error".  Also
>>    marked as work-around.  The recent fixes have made this unreachable,
>>    because it currently gets called only for input starting with '{'.
> Indeed, no triggers to this.
>> * check-qjson.c's empty_input() and blank_input() demonstrate the
>>    behavior.
>> * The other callers are not affected since they only pass input with
>>    exactly one JSON value or, in the case of negative tests, one error.
> As long as sending back-to-back newlines to QMP does not treat the
> empty line as an error, you should be okay. (If sending two newlines
> in a row now results in a {"error":...} response from the server for
> the blank line, then you've regressed).

QMP doesn't parse with qobject_from_json(), so it isn't affected.

Permit me a digression on newlines.

Newlines are like any other whitespace.  Whitespace can be necessary to
make the lexer emit a token.  For instance, sending "123" without a
newline to QMP does not produce a reply.  The lexer is in state
IN_DIGITS then.  You can make it go to JSON_INTEGER and emit the token
by sending a newline.  This produces a reply.

This doesn't match a (naive?) interactive users mental model of newline.
When such a user hits newline, he expects a reply.  If he doesn't get
one, say because he miscounted his curlies, confusion ensues.

A better designed protocol would avoid that trap.

>> Fail with "Expecting a JSON value" instead of returning null, and
>> simplify callers.
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
> Yay for getting rid of the inconsistent error reporting!
> Reviewed-by: Eric Blake <address@hidden>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]