All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: thuth@redhat.com, qemu-devel@nongnu.org, f4bug@amsat.org
Subject: Re: [Qemu-devel] [PATCH v2 13/23] tests: Clean up string interpolation around qtest_qmp_device_add()
Date: Tue, 31 Jul 2018 08:16:19 +0200	[thread overview]
Message-ID: <87bmaoszf0.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <2d7397fd-87e3-32a3-e5e2-38557ae65e2e@redhat.com> (Eric Blake's message of "Mon, 30 Jul 2018 10:25:26 -0500")

Eric Blake <eblake@redhat.com> writes:

> On 07/30/2018 03:34 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> 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.

  reply	other threads:[~2018-07-31  6:16 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-27 15:13 [Qemu-devel] [PATCH v2 00/23] tests: Compile-time format string checking for libqtest.h Markus Armbruster
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 01/23] libqtest: Document calling conventions Markus Armbruster
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 02/23] libqtest: Rename functions to send QMP messages Markus Armbruster
2018-07-27 15:24   ` Eric Blake
2018-07-30  5:41     ` Markus Armbruster
2018-07-27 16:35   ` Thomas Huth
2018-07-27 17:06     ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 03/23] libqtest: Clean up how we read device_del messages Markus Armbruster
2018-07-27 15:24   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 04/23] libqtest: Clean up how we read the QMP greeting Markus Armbruster
2018-07-27 15:25   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 05/23] qobject: Replace qobject_from_jsonf() by qobject_from_jsonf_nofail() Markus Armbruster
2018-07-27 15:28   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 06/23] qobject: New qobject_from_vjsonf_nofail(), qdict_from_vjsonf_nofail() Markus Armbruster
2018-07-27 15:30   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 07/23] libqtest: Simplify qmp_fd_vsend() a bit Markus Armbruster
2018-07-27 15:31   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 08/23] test-qobject-input-visitor: Avoid format string ambiguity Markus Armbruster
2018-07-27 15:33   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 09/23] qobject: qobject_from_jsonv() is dangerous, hide it away Markus Armbruster
2018-07-27 15:34   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 10/23] tests: Pass literal format strings directly to qmp_FOO() Markus Armbruster
2018-07-27 15:35   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 11/23] tests: Clean up string interpolation into QMP input (simple cases) Markus Armbruster
2018-07-27 15:39   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 12/23] cpu-plug-test: Don't pass integers as strings to device_add Markus Armbruster
2018-07-27 15:42   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 13/23] tests: Clean up string interpolation around qtest_qmp_device_add() Markus Armbruster
2018-07-27 15:48   ` Eric Blake
2018-07-30  6:04     ` Markus Armbruster
2018-07-30  8:34     ` Markus Armbruster
2018-07-30 15:25       ` Eric Blake
2018-07-31  6:16         ` Markus Armbruster [this message]
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 14/23] migration-test: Make wait_command() return the "return" member Markus Armbruster
2018-07-27 15:50   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 15/23] tests: New helper qtest_qmp_receive_success() Markus Armbruster
2018-07-27 16:00   ` Eric Blake
2018-07-30  6:10     ` Markus Armbruster
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 16/23] migration-test: Make wait_command() cope with '%' Markus Armbruster
2018-07-27 16:02   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 17/23] migration-test: Clean up string interpolation into QMP, part 1 Markus Armbruster
2018-07-27 16:04   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 18/23] migration-test: Clean up string interpolation into QMP, part 2 Markus Armbruster
2018-07-27 16:05   ` Eric Blake
2018-07-30  6:19     ` Markus Armbruster
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 19/23] migration-test: Clean up string interpolation into QMP, part 3 Markus Armbruster
2018-07-27 16:11   ` Eric Blake
2018-07-30  6:25     ` Markus Armbruster
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 20/23] libqtest: Enable compile-time format string checking Markus Armbruster
2018-07-27 16:18   ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 21/23] libqtest: Remove qtest_qmp_discard_response() & friends Markus Armbruster
2018-07-27 16:46   ` Thomas Huth
2018-07-27 17:03     ` Eric Blake
2018-07-30  6:32       ` Markus Armbruster
2018-08-01  6:46         ` Thomas Huth
2018-08-02  4:53           ` Markus Armbruster
2018-08-02  5:30             ` Thomas Huth
2018-08-02 18:31               ` Markus Armbruster
2018-07-27 17:05   ` Eric Blake
2018-07-30  6:28     ` Markus Armbruster
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 22/23] libqtest: Replace qtest_startf() by qtest_initf() Markus Armbruster
2018-07-27 17:08   ` Eric Blake
2018-07-30  6:32     ` Markus Armbruster
2018-07-27 17:18   ` Thomas Huth
2018-07-27 18:52     ` Eric Blake
2018-07-27 15:13 ` [Qemu-devel] [PATCH v2 23/23] libqtest: Rename qtest_FOOv() to qtest_vFOO() for consistency Markus Armbruster
2018-07-27 17:10   ` Eric Blake
2018-07-27 17:19   ` Thomas Huth
2018-07-30  6:47     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bmaoszf0.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.