bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Builtins should canonicalize path arguments


From: Bob Proulx
Subject: Re: Builtins should canonicalize path arguments
Date: Thu, 9 Jan 2014 14:20:25 -0700
User-agent: Mutt/1.5.21 (2010-09-15)

Chet Ramey wrote:
> Ondrej Oprala wrote:
> > Hi, I investigated this bug report:
> > https://bugzilla.redhat.com/show_bug.cgi?id=987975
> > and found out that some of bash's builtins (source at the very
> > least) do not canonicalize pathnames given as arguments (builtin
> > "open" is instead fed with the path - failing in the BZ case).

The bug report referenced was a missing a simple reproducer.  It is
also missing the information entirely on the location of the
"rpmbuild" directory.

The condensed bug report:

  $ set +o physical | grep physical
  physical        off
  address@hidden rpmbuild]$ ls -ld ~/repo
  lrwxrwxrwx. 1 aledvink aledvink 8 Jan 26 12:08 /home/aledvink/repo -> /al/repo
  address@hidden rpmbuild]$ cd ~/repo/ipmitool.f
  address@hidden ipmitool.f]$ . ../../rpmbuild/runme
  bash: ../../rpmbuild/runme: No such file or directory
  address@hidden ipmitool.f]$ cd ../../rpmbuild
  address@hidden rpmbuild]$

Where is rpmbuild located?  Because of the confusion I can guess that
it must only be located where there will be maximum confusion but a
bug report really should say these things explicitly.

Hmm...  Perhaps rpmbuild is in /home/aledvink/rpmbuild maybe?

  /
  /al
  /al/repo
  /al/repo/ipmitool.f
  /home
  /home/aledvink
  /home/aledvink/repo -> /al/repo
  /home/aledvink/rpmbuild
  /home/aledvink/rpmbuild/runme

I reproduced the confusion with this sequence.

  address@hidden:~$ set +o physical
  address@hidden:~$ rm -rf /tmp/testdir
  address@hidden:~$ mkdir /tmp/testdir
  address@hidden:~$ mkdir /tmp/testdir/al
  address@hidden:~$ mkdir /tmp/testdir/al/repo
  address@hidden:~$ mkdir /tmp/testdir/home
  address@hidden:~$ mkdir /tmp/testdir/home/aledvink
  address@hidden:~$ mkdir /tmp/testdir/home/aledvink/rpmbuild
  address@hidden:~$ ln -s /tmp/testdir/al/repo /tmp/testdir/home/aledvink/
  address@hidden:~$ mkdir /tmp/testdir/al/repo/ipmitool.f
  address@hidden:~$ echo echo inside runme file > 
/tmp/testdir/home/aledvink/rpmbuild/runme
  address@hidden:~$ cd /tmp/testdir/home/aledvink/repo/ipmitool.f
  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ /bin/pwd
  /tmp/testdir/al/repo/ipmitool.f
  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ echo $PWD
  /tmp/testdir/home/aledvink/repo/ipmitool.f

That sets up the test case.

  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ . 
../../rpmbuild/runme
  bash: ../../rpmbuild/runme: No such file or directory

Can't source the script using a logical path because it isn't
physically there.  The canonical path for the file is elsewhere.  And
because of this no utility can access it by that path.

  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ ls -ld 
../../rpmbuild/runme
  ls: cannot access ../../rpmbuild/runme: No such file or directory
  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ stat 
../../rpmbuild/runme
  stat: cannot stat ‘../../rpmbuild/runme’: No such file or directory
  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ cat 
../../rpmbuild/runme
  cat: ../../rpmbuild/runme: No such file or directory

That is not a correct path to access the runme file.

Only the shell maintaining $PWD and a fake model of how the user
arrived there is able to do this.

  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ cd ../../rpmbuild
  address@hidden:/tmp/testdir/home/aledvink/rpmbuild$ 

Why isn't the file there for anything other than the shell?  Because
"." and ".." are actual directory entries.  Let's track them down by
inode number.

  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ ls -ldogi . .. 
../.. ../../..
  23842833 drwxrwxr-x 2 40 Jan  9 13:46 .
  23840406 drwxrwxr-x 3 60 Jan  9 13:46 ..
  23840405 drwxrwxr-x 3 60 Jan  9 13:46 ../..
  23842828 drwxrwxr-x 4 80 Jan  9 13:46 ../../..

  address@hidden:/tmp/testdir/home/aledvink/repo/ipmitool.f$ find /tmp/testdir 
-type d -exec ls -ldogi {} +
  23842828 drwxrwxr-x 4 80 Jan  9 13:46 /tmp/testdir
  23840405 drwxrwxr-x 3 60 Jan  9 13:46 /tmp/testdir/al
  23840406 drwxrwxr-x 3 60 Jan  9 13:46 /tmp/testdir/al/repo
  23842833 drwxrwxr-x 2 40 Jan  9 13:46 /tmp/testdir/al/repo/ipmitool.f
  23840407 drwxrwxr-x 3 60 Jan  9 13:46 /tmp/testdir/home
  23842831 drwxrwxr-x 3 80 Jan  9 13:46 /tmp/testdir/home/aledvink
  23840410 drwxrwxr-x 2 60 Jan  9 13:46 /tmp/testdir/home/aledvink/rpmbuild

When you try to access "../" the ".." directory is
/tmp/testdir/al/repo.  When accessing "../../" that directory is
/tmp/testdir/al.

> > The builtin "cd" seems to handle relative paths correctly.

Actually "cd" in logical mode works in conjuction with a logical path
as displayed in $PWD to handle a fake model of the file system in the
presence of symbolic links.  Whether this fake model is correct or not
is a matter of perspective.  From my perspective it is most definitely
NOT correct.

I acknowledge that it is a convenient lie that many like.  Which is
exactly why there is the "physical" bash shell option to set how to
handle it.  Some people like it one way and others like it the other
way.  I do not like it and so I always set the physical view.
Otherwise the logical view inevitably leads to confusion such as the
above report.

This problem was introduced with the introduction of symlinks.  The
introduction of symlinks is a powerful feature.  But it did not
include a change in semantics for every other part of the system that
deals with file names.  It really can't because that code is
distributed here and there across the system.  We can only live with
the knowledge that symlinks create symbolic links across the file
system and try not to abuse them.  I wish the shell had never added
the logical view feature to try to cover up the short cut feature
introduced by symlinks.

> > I think it would be reasonable to take part of cd's
> > canonicalization code and use it in other builtins as well. I'd
> > gladly take care of the patch.

What would you do about 'ls' and the unbounded list of other utilities
that operate on files?  Would you modify it and every other program
that accesses files?  How would this be done as a practical matter?

The "." and "source" builtins should continue to behave the same as
other commands such as 'ls', 'cat', 'less' and others when accessing
files.  Consistent file handling is best.

> > Would upstream consider this a good approach?
> 
> I have reservations.  If the user in question wants consistent behavior,
> I suggest he use `set -o physical' for a while and see if it does what
> he wants.  The solution might be that simple.

Agreed.  Use 'set -o physical'.

Bob



reply via email to

[Prev in Thread] Current Thread [Next in Thread]