Libguestfs - Sep 2018 - Re: [PATCH nbdkit 3/6] file: Make the file= parameter into a magic config key.

On 09/07/2018 06:51 AM, Richard W.M. Jones wrote:
> +++ b/plugins/file/nbdkit-file-plugin.pod
> @@ -4,7 +4,7 @@ nbdkit-file-plugin - nbdkit file plugin
>   
>   =head1 SYNOPSIS
>   
> - nbdkit file file=FILENAME
> + nbdkit file FILENAME
>   
>   =head1 DESCRIPTION
>   
> @@ -26,6 +26,9 @@ be used here.
>   
>   This parameter is required.
>   
> +In nbdkit E<ge> 1.7, C<file=> may be omitted if the filename
does not
> +start with a C<-> or contain an C<=> character.
This is accurate, but annoying for 'nbdkit file "$name"' when
$name is
not under your control.

With just a bit more code in patch 2/6, you could:
  s|contain an C<=> character|& with no earlier C</> character|

That is, if we assume/enforce that all valid key values that any plugin 
should ever want to support are also valid shell variable names, then we 
can tell that 'a=b' is a key/value, while './a=b' is an attempt
to use
the magic-key functionality (equivalent to 'file=./a=b').  Then, it 
becomes easy to write:

case $name in
   /*) nbdkit file "$name" ;;
   *) nbdkit file "./$name" ;;
esac

without worrying about leading - or contained = in $name.

Of course, you can still always write:
   nbdkit file file="$name"

when untrusted names are a concern, so I don't know if the extra magic 
is worth it.

Otherwise, I'm liking this series.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Fri, Sep 07, 2018 at 08:34:34AM -0500, Eric Blake
wrote:
> On 09/07/2018 06:51 AM, Richard W.M. Jones wrote:
> 
> >+++ b/plugins/file/nbdkit-file-plugin.pod
> >@@ -4,7 +4,7 @@ nbdkit-file-plugin - nbdkit file plugin
> >  =head1 SYNOPSIS
> >- nbdkit file file=FILENAME
> >+ nbdkit file FILENAME
> >  =head1 DESCRIPTION
> >@@ -26,6 +26,9 @@ be used here.
> >  This parameter is required.
> >+In nbdkit E<ge> 1.7, C<file=> may be omitted if the
filename does not
> >+start with a C<-> or contain an C<=> character.
> 
> This is accurate, but annoying for 'nbdkit file "$name"'
when $name
> is not under your control.
> 
> With just a bit more code in patch 2/6, you could:
>  s|contain an C<=> character|& with no earlier C</>
character|
> 
> That is, if we assume/enforce that all valid key values that any
> plugin should ever want to support are also valid shell variable
> names, then we can tell that 'a=b' is a key/value, while
'./a=b' is
> an attempt to use the magic-key functionality (equivalent to
> 'file=./a=b').  Then, it becomes easy to write:
> 
> case $name in
>   /*) nbdkit file "$name" ;;
>   *) nbdkit file "./$name" ;;
> esac
> 
> without worrying about leading - or contained = in $name.
> 
> Of course, you can still always write:
>   nbdkit file file="$name"
> 
> when untrusted names are a concern, so I don't know if the extra
> magic is worth it.
> 
> Otherwise, I'm liking this series.
They could of course continue to use file=foo=bar which will work to
describe a file called "foo=bar".  But I agree we ought to limit the
choice of characters in the key.  There are some filters (eg.  error
filter) already using '-' in keys, and I guess it's also valid to
use
'_' as well as the usual alphanumeric.

I'll send a follow up patch to limit this and make the
documentation clearer.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Fri, Sep 07, 2018 at 03:58:44PM +0100, Richard W.M. Jones
wrote:
> They could of course continue to use file=foo=bar which will work to
> describe a file called "foo=bar".
Ooops, as indeed you said in the bit past what I read.  I'll
send a follow up anyhow.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/