qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: richard.henderson@linaro.org, Eric Blake <eblake@redhat.com>
Subject: [PULL 08/17] qapi: Fix to reject 'data': 'mumble' in struct
Date: Wed, 26 Apr 2023 07:57:35 +0200	[thread overview]
Message-ID: <20230426055744.1041930-9-armbru@redhat.com> (raw)
In-Reply-To: <20230426055744.1041930-1-armbru@redhat.com>

A struct's 'data' must be a JSON object defining the struct's members.
The QAPI code generator incorrectly accepts a JSON string instead, and
then crashes in QAPISchema._make_members() called from
._def_struct_type().

Fix to reject it: factor check_type_implicit() out of
check_type_name_or_implicit(), and switch check_struct() to use it
instead.  Also add a test case.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20230316071325.492471-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[More detailed commit message]
---
 scripts/qapi/expr.py                        | 24 +++++++++++++--------
 tests/qapi-schema/meson.build               |  1 +
 tests/qapi-schema/struct-data-typename.err  |  2 ++
 tests/qapi-schema/struct-data-typename.json |  2 ++
 tests/qapi-schema/struct-data-typename.out  |  0
 5 files changed, 20 insertions(+), 9 deletions(-)
 create mode 100644 tests/qapi-schema/struct-data-typename.err
 create mode 100644 tests/qapi-schema/struct-data-typename.json
 create mode 100644 tests/qapi-schema/struct-data-typename.out

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 9bae500a7d..cae0a08359 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -354,14 +354,14 @@ def check_type_name_or_array(value: Optional[object],
                            source)
 
 
-def check_type_name_or_implicit(value: Optional[object],
-                                info: QAPISourceInfo, source: str,
-                                parent_name: Optional[str]) -> None:
+def check_type_implicit(value: Optional[object],
+                        info: QAPISourceInfo, source: str,
+                        parent_name: Optional[str]) -> None:
     """
     Normalize and validate an optional implicit struct type.
 
-    Accept ``None``, ``str``, or a ``dict`` defining an implicit
-    struct type.  The latter is normalized in place.
+    Accept ``None`` or a ``dict`` defining an implicit struct type.
+    The latter is normalized in place.
 
     :param value: The value to check.
     :param info: QAPI schema source file information.
@@ -377,9 +377,6 @@ def check_type_name_or_implicit(value: Optional[object],
     if value is None:
         return
 
-    if isinstance(value, str):
-        return
-
     if not isinstance(value, dict):
         raise QAPISemError(info,
                            "%s should be an object or type name" % source)
@@ -401,6 +398,15 @@ def check_type_name_or_implicit(value: Optional[object],
         check_type_name_or_array(arg['type'], info, key_source)
 
 
+def check_type_name_or_implicit(value: Optional[object],
+                                info: QAPISourceInfo, source: str,
+                                parent_name: Optional[str]) -> None:
+    if value is None or isinstance(value, str):
+        return
+
+    check_type_implicit(value, info, source, parent_name)
+
+
 def check_features(features: Optional[object],
                    info: QAPISourceInfo) -> None:
     """
@@ -486,7 +492,7 @@ def check_struct(expr: QAPIExpression) -> None:
     name = cast(str, expr['struct'])  # Checked in check_exprs
     members = expr['data']
 
-    check_type_name_or_implicit(members, expr.info, "'data'", name)
+    check_type_implicit(members, expr.info, "'data'", name)
     check_type_name(expr.get('base'), expr.info, "'base'")
 
 
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index d85b14f28c..f88110bddf 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -164,6 +164,7 @@ schemas = [
   'struct-base-clash-deep.json',
   'struct-base-clash.json',
   'struct-data-invalid.json',
+  'struct-data-typename.json',
   'struct-member-if-invalid.json',
   'struct-member-invalid-dict.json',
   'struct-member-invalid.json',
diff --git a/tests/qapi-schema/struct-data-typename.err b/tests/qapi-schema/struct-data-typename.err
new file mode 100644
index 0000000000..8fbfe99a42
--- /dev/null
+++ b/tests/qapi-schema/struct-data-typename.err
@@ -0,0 +1,2 @@
+struct-data-typename.json: In struct 'Stru2':
+struct-data-typename.json:2: 'data' should be an object or type name
diff --git a/tests/qapi-schema/struct-data-typename.json b/tests/qapi-schema/struct-data-typename.json
new file mode 100644
index 0000000000..70fbad0ee4
--- /dev/null
+++ b/tests/qapi-schema/struct-data-typename.json
@@ -0,0 +1,2 @@
+{ 'struct': 'Stru1', 'data': {} }
+{ 'struct': 'Stru2', 'data': 'Stru1' }
diff --git a/tests/qapi-schema/struct-data-typename.out b/tests/qapi-schema/struct-data-typename.out
new file mode 100644
index 0000000000..e69de29bb2
-- 
2.39.2



  parent reply	other threads:[~2023-04-26  5:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-26  5:57 [PULL 00/17] QAPI patches patches for 2023-04-26 Markus Armbruster
2023-04-26  5:57 ` [PULL 01/17] qapi: Fix error message format regression Markus Armbruster
2023-04-26  5:57 ` [PULL 02/17] qapi/schema: Use super() Markus Armbruster
2023-04-26  5:57 ` [PULL 03/17] qapi: Clean up after removal of simple unions Markus Armbruster
2023-04-26  5:57 ` [PULL 04/17] qapi: Split up check_type() Markus Armbruster
2023-04-26  5:57 ` [PULL 05/17] qapi: Improve error message for unexpected array types Markus Armbruster
2023-04-26  5:57 ` [PULL 06/17] qapi: Simplify code a bit after previous commits Markus Armbruster
2023-04-26  5:57 ` [PULL 07/17] qapi: Fix error message when type name or array is expected Markus Armbruster
2023-04-26  5:57 ` Markus Armbruster [this message]
2023-04-26  5:57 ` [PULL 09/17] tests/qapi-schema: Improve union discriminator coverage Markus Armbruster
2023-04-26  5:57 ` [PULL 10/17] tests/qapi-schema: Rename a few conditionals Markus Armbruster
2023-04-26  5:57 ` [PULL 11/17] tests/qapi-schema: Clean up positive test for conditionals Markus Armbruster
2023-04-26  5:57 ` [PULL 12/17] tests/qapi-schema: Cover optional conditional struct member Markus Armbruster
2023-04-26  5:57 ` [PULL 13/17] qapi: Fix code generated for " Markus Armbruster
2023-04-26  5:57 ` [PULL 14/17] qapi: Require boxed for conditional command and event arguments Markus Armbruster
2023-04-26  5:57 ` [PULL 15/17] qapi: support updating expected test output via make Markus Armbruster
2023-04-26  5:57 ` [PULL 16/17] qapi: Improve specificity of type/member descriptions Markus Armbruster
2023-04-26  5:57 ` [PULL 17/17] qapi: allow unions to contain further unions Markus Armbruster
2023-04-26 11:07 ` [PULL 00/17] QAPI patches patches for 2023-04-26 Richard Henderson

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=20230426055744.1041930-9-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).