scripts/qmp/qmp-shell | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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 <kchamart@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
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)
# Transactional CLI entry/exit:
if cmdargs[0] == 'transaction(':
--
2.20.1.98.gecbdaf0899
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 <kchamart@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 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(':
>
On Tue, Feb 05, 2019 at 08:44:12PM -0500, John Snow wrote:
> On 2/5/19 8:49 AM, Marc-André Lureau wrote:
[...]
> > 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 <kchamart@redhat.com>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > 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.
Yeah, a little comment explaining it would be nice.
And thanks for the fix, indeed. FWIW:
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
> Though I suppose we are approaching the limits of what this hacky little
> debug script can do for us...
It is good enough for 80% of the cases. :-)
--
/kashyap
On Wed, Feb 06, 2019 at 11:48:51AM +0100, Kashyap Chamarthy wrote:
> On Tue, Feb 05, 2019 at 08:44:12PM -0500, John Snow wrote:
> > On 2/5/19 8:49 AM, Marc-André Lureau wrote:
[...]
> > > < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
> > > """
> > > - cmdargs = shlex.split(cmdline)
> > > + cmdargs = re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmdline)
Dan Berrangé explained on IRC this way:
In plain english what that is saying is: give me all blocks of text
which are:
(a) set of chars not including " or '
(b) a set of chars surrounded by ".."
(c) a set of chars surrounded by '...
> > It might really be nice to have a comment briefly explaining the regex.
> > This is pretty close to symbol soup.
>
> Yeah, a little comment explaining it would be nice.
For my own education today I learned that ?: is called a "non-capturing
match". For reference, quoting from the `perldoc perlretut`:
[quote]
Non-capturing groupings
A group that is required to bundle a set of alternatives may or may not
be useful as a capturing group. If it isn't, it just creates a
superfluous addition to the set of available capture group values,
inside as well as outside the regexp. Non-capturing groupings, denoted
by "(?:regexp)", still allow the regexp to be treated as a single unit,
but don't establish a capturing group at the same time. Both capturing
and non-capturing groupings are allowed to co-exist in the same regexp.
Because there is no extraction, non-capturing groupings are faster than
capturing groupings. Non-capturing groupings are also handy for choosing
exactly which parts of a regexp are to be extracted to matching
variables:
# match a number, $1-$4 are set, but we only want $1
/([+-]?\ *(\d+(\.\d*)?|\.\d+)([eE][+-]?\d+)?)/;
# match a number faster , only $1 is set
/([+-]?\ *(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?)/;
# match a number, get $1 = whole number, $2 = exponent
/([+-]?\ *(?:\d+(?:\.\d*)?|\.\d+)(?:[eE]([+-]?\d+))?)/;
Non-capturing groupings are also useful for removing nuisance elements
gathered from a split operation where parentheses are required for some
reason:
$x = '12aba34ba5';
@num = split /(a|b)+/, $x; # @num = ('12','a','34','a','5')
@num = split /(?:a|b)+/, $x; # @num = ('12','34','5')
[/quote]
[...]
--
/kashyap
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 <kchamart@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi, did this get misplaced? Would be nice to have back for 4.0 release.
John Snow <jsnow@redhat.com> writes:
> 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 <kchamart@redhat.com>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi, did this get misplaced? Would be nice to have back for 4.0 release.
Eduardo, Cleber, I think this one's for you.
On Sat, Mar 09, 2019 at 10:01:58AM +0100, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > 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 <kchamart@redhat.com>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi, did this get misplaced? Would be nice to have back for 4.0 release.
>
> Eduardo, Cleber, I think this one's for you.
Queued, thanks.
I'm really bothered by the lack of tests for qmp-shell
command-line parsing. Does anybody volunteer to write a small
set of doctest examples for __build_cmd()?
--
Eduardo
© 2016 - 2024 Red Hat, Inc.