On Thu, Oct 27, 2022 at 09:28:19AM +0100, Richard W.M. Jones wrote:
>
> On Wed, Oct 26, 2022 at 04:15:59PM -0500, Eric Blake wrote:
> > Slightly rearrange the code so that we can reuse the -c framework for
> > executing the code snippets for -u, --opt-mode, and --base-allocation.
> > Slightly changes the error message when a URI is invalid (which
> > requires revamping test-error.sh a bit to match), but otherwise no
> > semantic change intended.
> > ---
> > python/nbdsh.py | 67 ++++++++++++++++++++++--------------------------
> > sh/test-error.sh | 37 ++++++++++++++------------
> > 2 files changed, 52 insertions(+), 52 deletions(-)
> >
> > @@ -110,42 +107,40 @@ def shell():
> > if not args.n:
> > h = nbd.NBD()
> > h.set_handle_name("nbdsh")
> > + cmds = args.command
> >
> > # Set other attributes in the handle.
> > + if args.uri is not None:
> > + cmds.insert(0, 'h.connect_uri("%s")' % args.uri)
>
> This allows commands to be injected if, for example, the nbdsh -u
> parameter came from an untrusted source.
Good observation. The old way...
>
> After going through probably hundreds of stackoverflow answer this
> morning I cannot find if Python has an official way to escape this.
> Both string.encode() and %r seem like potential candidates:
>
> >>> s = "\""
> >>> 'h.connect_uri(%s)' % s.encode('unicode-escape')
> 'h.connect_uri(b\'"\')'
> >>> 'h.connect_uri(%r)' % s
> 'h.connect_uri(\'"\')'
>
> (Note the quoting displayed there is a little confusing because the
> output has been quoted.)
>
> >
> > - # Parse the URI.
> > - if args.uri is not None:
> > - try:
> > - h.connect_uri(args.uri)
...directly passed the caller's string as a single unit, instead of...
> > - except nbd.Error as ex:
> > - print("nbdsh: unable to connect to uri '%s': %s" %
> > - (args.uri, ex.string), file=sys.stderr)
> > - sys.exit(1)
> > -
> > - # If there are no -c or --command parameters, go interactive,
> > - # otherwise we run the commands and exit.
> > - if not args.command:
> > - code.interact(banner=banner, local=locals(), exitmsg='')
> > - else:
> > - # https://stackoverflow.com/a/11754346
> > - d = dict(locals(), **globals())
> > - try:
> > - for c in args.command:
> > - if c != '-':
> > - exec(c, d, d)
...possibly splitting it through exec().
> > +
> > + # If there are no explicit -c or --command parameters, go interactive.
> > + if len(args.command) - shortcuts == 0:
> > + sys.ps1 = "nbd> "
> > + code.interact(banner=make_banner(args), local=locals(), exitmsg='')
>
> Previously we were unconditionally setting sys.ps1. Don't know if the
> new behaviour is correct - possibly it is because we will only use
> sys.ps1 when we call code.interact.
Yes, this part of the code motion is because sys.ps1 only matters
prior to code.interact().
Your reply to 3/3 has merit; I'll add more there.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
