Re: [Qemu-devel] [PATCH] qmp-shell: fix nested json regression

[John Snow]

On 2/5/19 8:49 AM, Marc-André Lureau wrote:
> Commit fcfab7541 ("qmp-shell: learn to send commands with quoted
> arguments") introduces the usage of Python 'shlex' to handle quoted
> arguments, but it accidentally broke generation of nested JSON
> structs.
> 
> shlex drops quotes, which breaks parsing of the nested struct.
> 
> cmd='blockdev-create job-id="job0 foo" 
> options={"driver":"qcow2","size":16384,"file":{"driver":"file","filename":"foo.qcow2"}}'
> 
> shlex.split(cmd)
> ['blockdev-create',
>  'job-id=job0 foo',
>  'options={driver:qcow2,size:16384,file:{driver:file,filename:foo.qcow2}}']
> 
> Replace with a regexp to split while respecting quoted strings and preserving 
> quotes:
> 
> re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmd)
> ['blockdev-create',
>  'job-id="job0 foo"',
>  
> 'options={"driver":"qcow2","size":16384,"file":{"driver":"file","filename":"foo.qcow2"}}']
> 
> Fixes: fcfab7541 ("qmp-shell: learn to send commands with quoted arguments")
> Reported-by: Kashyap Chamarthy <address@hidden>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/qmp/qmp-shell | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 770140772d..813dd68232 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -74,7 +74,7 @@ import sys
>  import os
>  import errno
>  import atexit
> -import shlex
> +import re
>  
>  class QMPCompleter(list):
>      def complete(self, text, state):
> @@ -220,7 +220,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>  
>              < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
>          """
> -        cmdargs = shlex.split(cmdline)
> +        cmdargs = 
> re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmdline)

It might really be nice to have a comment briefly explaining the regex.
This is pretty close to symbol soup.

Though I suppose we are approaching the limits of what this hacky little
debug script can do for us...

thank you for fixing it.

>  
>          # Transactional CLI entry/exit:
>          if cmdargs[0] == 'transaction(':
>