Re: [Libguestfs] [PATCH nbdkit] server: Add -D nbdkit.environ=1 to dump the environment
On Mon, May 08, 2023 at 08:35:53AM -0500, Eric Blake wrote:
On Sun, May 07, 2023 at 11:43:17AM +0100, Richard W.M. Jones wrote:
> This is not secure so should not be used routinely. Also we do not
> attempt to filter environment variables, so even ones containing
> multiple lines or special characters are all sent to nbdkit_debug.
>
> The reason for adding this is to allow for debugging the new
> nbd_set_socket_activation_name(3) API added in libnbd 1.16.
> ---
> +static void
> +dump_environment (void)
> +{
> + size_t i;
> +
> + for (i = 0; environ[i]; ++i)
> + nbdkit_debug ("%s", environ[i]);
We have shell_quote in common/utils/quote.c; shouldn't we use it here
to avoid ambiguities when an env-var contains newline?
I was wondering more generally about this. I actually think that
nbdkit_debug ought to be more tolerant of unusual inputs -- not least
because there are potential security concerns. All kinds of random
user-controlled strings are sent to nbdkit_debug and it just dumps it
to stderr entirely unfiltered. So rather than fixing this one
instance, we should fix nbdkit_debug. I'll look at a follow-up patch.
> +}
> +
> +#endif /* !WIN32 */
> +
> int
> main (int argc, char *argv[])
> {
> @@ -662,6 +682,14 @@ main (int argc, char *argv[])
> /* Check all debug flags were used, and free them. */
> free_debug_flags ();
>
> +#ifndef WIN32
> + /* Dump the environment if asked. This is the earliest we can do it
> + * because it uses a debug flag.
> + */
> + if (nbdkit_debug_environ && verbose)
> + dump_environment ();
Why does this not work on WIN32?
I just assumed it wouldn't, but you're right that it would work.
I'll respin this patch once I've tested what it would do on Windows.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit