Re: [Libguestfs] [libnbd PATCH 1/3] nbdsh: Refactor handling of -u and friends

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.

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)

> - 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)

> + > + # 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.