[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v2 59/60] json: Improve safety of qobject_from_j

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v2 59/60] json: Improve safety of qobject_from_jsonf_nofail() & friends
Date: Fri, 17 Aug 2018 13:14:28 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

On 08/17/2018 10:05 AM, Markus Armbruster wrote:
The JSON parser optionally supports interpolation.  This is used to
build QObjects by parsing string templates.  The templates are C
literals, so parse errors (such as invalid interpolation
specifications) are actually programming errors.  Consequently, the
functions providing parsing with interpolation
(qobject_from_jsonf_nofail(), qobject_from_vjsonf_nofail(),
qdict_from_jsonf_nofail(), qdict_from_vjsonf_nofail()) pass
&error_abort to the parser.

Avoid undefined behavior by catching the programming error at run
time: have the parser recognize and reject '%' in JSON strings.

Signed-off-by: Markus Armbruster <address@hidden>

@@ -205,7 +206,14 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
                  parse_error(ctxt, token, "invalid escape sequence in string");
                  goto out;
-        } else {
+            break;
+        case '%':
+            if (ctxt->ap) {
+                parse_error(ctxt, token, "can't interpolate into string");

Do we want to abort() here, to match the fact that all our other interpolation functions abort on programmer errors?

+++ b/tests/check-qjson.c
@@ -1037,16 +1037,13 @@ static void interpolation_unknown(void)
static void interpolation_string(void)
-    QLitObject decoded = QLIT_QLIST(((QLitObject[]){
-            QLIT_QSTR("%s"),
-            QLIT_QSTR("eins"),
-            {}}));
-    QObject *qobj;
-    /* Dangerous misfeature: % is silently ignored in strings */
-    qobj = qobject_from_jsonf_nofail("['%s', %s]", "eins", "zwei");
-    g_assert(qlit_equal_qobject(&decoded, qobj));
-    qobject_unref(qobj);
+    if (g_test_subprocess()) {
+        qobject_from_jsonf_nofail("['%s', %s]", "eins", "zwei");
+    }
+    g_test_trap_subprocess(NULL, 0, 0);
+    g_test_trap_assert_failed();
+    g_test_trap_assert_stderr("*Unexpected error*"
+                              "can't interpolate into string*");

Oh, we still abort, but later on in the caller function.

Okay, looks like reasonable handling.
Reviewed-by: Eric Blake <address@hidden>

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

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