[Libguestfs] [libnbd PATCH 1/3] nbdsh: Refactor handling of -u and friends
Richard W.M. Jones
rjones at redhat.com
Thu Oct 27 08:28:19 UTC 2022
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(-)
>
> diff --git a/python/nbdsh.py b/python/nbdsh.py
> index a98a9113..84ac84e2 100644
> --- a/python/nbdsh.py
> +++ b/python/nbdsh.py
> @@ -1,5 +1,5 @@
> # NBD client library in userspace
> -# Copyright (C) 2013-2021 Red Hat Inc.
> +# Copyright (C) 2013-2022 Red Hat Inc.
> #
> # This library is free software; you can redistribute it and/or
> # modify it under the terms of the GNU Lesser General Public
> @@ -35,12 +35,13 @@ def shell():
> epilog=epilog)
> short_options = []
> long_options = []
> + shortcuts = 0
>
> parser.add_argument('--base-allocation', action='store_true',
> help='request the "base:allocation" meta context')
> long_options.append("--base-allocation")
>
> - parser.add_argument('-c', '--command', action='append',
> + parser.add_argument('-c', '--command', action='append', default=[],
> help="run a Python statement "
> "(may be used multiple times)")
> short_options.append("-c")
> @@ -98,10 +99,6 @@ def shell():
> print("\n".join(long_options))
> exit(0)
>
> - # Create the banner and prompt.
> - banner = make_banner(args)
> - sys.ps1 = "nbd> "
> -
> # If verbose, set LIBNBD_DEBUG=1
> if args.verbose:
> os.environ["LIBNBD_DEBUG"] = "1"
> @@ -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.)
> + shortcuts += 1
> if args.base_allocation:
> - h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
> + cmds.insert(0, 'h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)')
> + shortcuts += 1
> if args.opt_mode:
> - h.set_opt_mode(True)
> + cmds.insert(0, 'h.set_opt_mode(True)')
> + shortcuts += 1
>
> - # 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)
> - else:
> - exec(sys.stdin.read(), d, d)
> - except nbd.Error as ex:
> - if nbd.NBD().get_debug():
> - traceback.print_exc()
> + # Run all -c snippets, including any shortcuts we just added
> + # https://stackoverflow.com/a/11754346
> + d = dict(locals(), **globals())
> + try:
> + for c in args.command:
> + if c != '-':
> + exec(c, d, d)
> else:
> - print("nbdsh: command line script failed: %s" % ex.string,
> - file=sys.stderr)
> - sys.exit(1)
> + exec(sys.stdin.read(), d, d)
> + except nbd.Error as ex:
> + if nbd.NBD().get_debug():
> + traceback.print_exc()
> + else:
> + print("nbdsh: command line script failed: %s" % ex.string,
> + file=sys.stderr)
> + sys.exit(1)
> +
> + # 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.
Rich.
>
> def make_banner(args):
> diff --git a/sh/test-error.sh b/sh/test-error.sh
> index a33ce475..7dbd7ae4 100755
> --- a/sh/test-error.sh
> +++ b/sh/test-error.sh
> @@ -35,32 +35,37 @@ cat $err
> grep Traceback $err && fail=1
> grep '^nbdsh: .*unrecognized.*no-such' $err
>
> -# Test behavior when -u fails. No python trace should be present.
> -nbdsh -u 'nbd+unix:///?socket=/nosuchsock' >$out 2>$err && fail=1
> -test ! -s $out
> -cat $err
> -grep Traceback $err && fail=1
> -grep '^nbdsh: unable to connect to uri.*nosuchsock' $err
> -
> # Triggering nbd.Error non-interactively (via -c) prints the error. The
> -# output includes the python trace when debugging is enabled (which is
> -# the default for our testsuite, when using ./run).
> -nbdsh -c 'h.is_read_only()' >$out 2>$err && fail=1
> +# output includes the python trace when debugging is enabled (our default
> +# when run under 'make check', but set explicitly here to make sure).
> +LIBNBD_DEBUG=1 nbdsh -c 'h.is_read_only()' >$out 2>$err && fail=1
> test ! -s $out
> cat $err
> grep Traceback $err
> grep 'in is_read_only' $err
> grep '^nbd\.Error: nbd_is_read_only: ' $err
>
> -# Override ./run's default to show that without debug, the error is succinct.
> -nbdsh -c '
> -import os
> -os.environ["LIBNBD_DEBUG"] = "0"
> -h.is_read_only()
> -' >$out 2>$err && fail=1
> +# Without debugging, the error is succinct.
> +LIBNBD_DEBUG=0 nbdsh -c 'h.is_read_only()' >$out 2>$err && fail=1
> test ! -s $out
> cat $err
> grep Traceback $err && fail=1
> grep '^nbdsh: command line script failed: nbd_is_read_only: ' $err
>
> +# --verbose overrides environment to request debugging.
> +LIBNBD_DEBUG=0 nbdsh --verbose -c 'h.is_read_only()' >$out 2>$err && fail=1
> +test ! -s $out
> +cat $err
> +grep Traceback $err
> +grep 'in is_read_only' $err
> +grep '^nbd\.Error: nbd_is_read_only: ' $err
> +
> +# Test behavior when -u fails; since it triggers nbd.Error, it should
> +# be succinct without debug.
> +LIBNBD_DEBUG=0 nbdsh -u 'nbd+unix:///?socket=/nosuchsock' >$out 2>$err && fail=1
> +test ! -s $out
> +cat $err
> +grep Traceback $err && fail=1
> +grep '^nbdsh: .*nbd_connect_uri: ' $err
> +
> exit $fail
> --
> 2.37.3
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list