Libguestfs - Oct 2015 - Re: [PATCH 2/2] actions: turn some params into RelativePathnameList (RHBZ#1174551).

On Tue, Oct 20, 2015 at 03:50:31PM +0200, Pino Toscano
wrote:
> On Tuesday 20 October 2015 14:43:53 Richard W.M. Jones wrote:
> > On Tue, Oct 20, 2015 at 01:59:10PM +0200, Pino Toscano wrote:
> > > Use RelativePathnameList as type for lists of relative paths, as
used in
> > > some listing-alike APIs.  This way we can ensure absolute paths
in those
> > > lists are rejects outright.
> > > 
> > > As a consequence, test-big-dirs.pl does not need to prepend the
> > > directory name anymore before calling listing-alike APIs:
previously
> > > they didn't fail, but the returned lists contained only
invalid
> > > elements (and only their size was checked).
> > 
> > Are these all relative pathnames, or are they in fact just filenames
> > without any path at all.  That is to say: is "foo/bar"
permitted, or
> > just "bar"?
> 
> At least with *lstat*list and *readlinklist functions, the file names
> are considered as relative wrt the path specified, as they are resolved
> against the file descriptor of the directory.
> 
> In case of *lxattrlist, the absolute path+name for each is built and
> used as path within the guest.
> 
> So yes, "bar", "foo/bar", and "../bar" too,
should work.
I think I really meant -- is it a caller bug if the parameter contains
a slash in it?

All the *list functions were really intended as optimizations for
fuse/guestmount, and IIRC it was intended that only filenames (not
even relative paths) be used there.

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 Tuesday 20 October 2015 15:22:22 Richard W.M. Jones
wrote:
> On Tue, Oct 20, 2015 at 03:50:31PM +0200, Pino Toscano wrote:
> > On Tuesday 20 October 2015 14:43:53 Richard W.M. Jones wrote:
> > > On Tue, Oct 20, 2015 at 01:59:10PM +0200, Pino Toscano wrote:
> > > > Use RelativePathnameList as type for lists of relative
paths, as used in
> > > > some listing-alike APIs.  This way we can ensure absolute
paths in those
> > > > lists are rejects outright.
> > > > 
> > > > As a consequence, test-big-dirs.pl does not need to prepend
the
> > > > directory name anymore before calling listing-alike APIs:
previously
> > > > they didn't fail, but the returned lists contained only
invalid
> > > > elements (and only their size was checked).
> > > 
> > > Are these all relative pathnames, or are they in fact just
filenames
> > > without any path at all.  That is to say: is "foo/bar"
permitted, or
> > > just "bar"?
> > 
> > At least with *lstat*list and *readlinklist functions, the file names
> > are considered as relative wrt the path specified, as they are
resolved
> > against the file descriptor of the directory.
> > 
> > In case of *lxattrlist, the absolute path+name for each is built and
> > used as path within the guest.
> > 
> > So yes, "bar", "foo/bar", and "../bar"
too, should work.
> 
> I think I really meant -- is it a caller bug if the parameter contains
> a slash in it?
I don't know that :) I mean, considering even our code (see my change
in test-big-dirs.pl) was passing more than just a filename in the
specified path, then I thought relative filenames were somehow intended,
and thus I just rejected absolute ones.

I'm open to both positions though, either what I proposed in the patches
or just reject any file name not in the current directory.
> All the *list functions were really intended as optimizations for
> fuse/guestmount, and IIRC it was intended that only filenames (not
> even relative paths) be used there.
They are used also in diff & ls nowadays (still with filenames only
though).

-- 
Pino Toscano

On Tue, Oct 20, 2015 at 06:17:59PM +0200, Pino Toscano
wrote:
> I don't know that :) I mean, considering even our code (see my change
> in test-big-dirs.pl) was passing more than just a filename in the
> specified path, then I thought relative filenames were somehow intended,
> and thus I just rejected absolute ones.
> 
> I'm open to both positions though, either what I proposed in the
patches
> or just reject any file name not in the current directory.
I think reject any filename which contains a slash char.

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/