[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>
Thanks!
- [Qemu-devel] [PATCH 46/56] json: Assert json_parser_parse() consumes all tokens on success, (continued)
- [Qemu-devel] [PATCH 46/56] json: Assert json_parser_parse() consumes all tokens on success, Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 49/56] json: Streamline json_message_process_token(), Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 56/56] docs/interop/qmp-spec: How to force known good parser state, Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 55/56] json: Clean up headers, Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 47/56] qjson: Have qobject_from_json() & friends reject empty and blank, Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 33/56] json: Redesign the callback to consume JSON values, Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 34/56] json: Don't pass null @tokens to json_parser_parse(), Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 40/56] json: Replace %I64d, %I64u by %PRId64, %PRIu64, Markus Armbruster, 2018/08/08
- [Qemu-devel] [PATCH 29/56] check-qjson: Fix and enable utf8_string()'s disabled part, Markus Armbruster, 2018/08/08