bugs in cd

bugs in cd

[Eric Blake]

For the cd command, POSIX 2008 requires that after all pathnames in CDPATH have 
been tested and failed in step 5, then step 6 interprets the directory argument 
relative to PWD.  In other words, this demonstrates a bug:

$ dash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
cd: 1: can't cd to foo
2
/tmp

while bash gets it correct:

$ bash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
0
/tmp/foo

Furthermore, POSIX requires that if the element in CDPATH ends in slash, that 
no additional slashes are added while forming the candidate curpath.  In light 
of the fact that //home need not be the same directory as /home (and indeed, on 
cygwin, they are distinct entities), this is also a bug:

$ dash -c 'CDPATH=/; cd home'
//home
$ bash -c 'CDPATH=/; cd home'
/home

-- 
Eric Blake

Re: bugs in cd

[Eric Blake]

According to Eric Blake on 7/14/2009 3:39 PM:
> For the cd command, POSIX 2008 requires that after all pathnames in CDPATH have 
> been tested and failed in step 5, then step 6 interprets the directory argument 
> relative to PWD.  In other words, this demonstrates a bug:
> 
> $ dash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
> cd: 1: can't cd to foo
> 2
> /tmp
> 
> while bash gets it correct:
> 
> $ bash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
> 0
> /tmp/foo
> 
> Furthermore, POSIX requires that if the element in CDPATH ends in slash, that 
> no additional slashes are added while forming the candidate curpath.  In light 
> of the fact that //home need not be the same directory as /home (and indeed, on 
> cygwin, they are distinct entities), this is also a bug:
> 
> $ dash -c 'CDPATH=/; cd home'
> //home
> $ bash -c 'CDPATH=/; cd home'
> /home

Ping.

-- 
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net

Re: bugs in cd

[Herbert Xu]

On Tue, Jul 14, 2009 at 09:39:03PM +0000, Eric Blake wrote:
> For the cd command, POSIX 2008 requires that after all pathnames in CDPATH have 
> been tested and failed in step 5, then step 6 interprets the directory argument 
> relative to PWD.  In other words, this demonstrates a bug:
> 
> $ dash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
> cd: 1: can't cd to foo
> 2
> /tmp
> 
> while bash gets it correct:
> 
> $ bash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
> 0
> /tmp/foo

This patch fixes the first part.

commit 7877713690c498bb74823542231a81362e8eedb3
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Mon Aug 31 22:06:41 2009 +1000

    [CD] Lookup PWD after going through CDPATH
    
    On Tue, Jul 14, 2009 at 09:39:03PM +0000, Eric Blake wrote:
    > For the cd command, POSIX 2008 requires that after all pathnames in CDPATH
    > have been tested and failed in step 5, then step 6 interprets the directory
    > argument relative to PWD.  In other words, this demonstrates a bug:
    >
    > $ dash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
    > cd: 1: can't cd to foo
    > 2
    > /tmp
    >
    > while bash gets it correct:
    >
    > $ bash -c 'cd /tmp; mkdir -p foo; CDPATH=oops; cd foo; echo $?; pwd'
    > 0
    > /tmp/foo
    
    This patch fixes the problem.
    
    Reported-by: Eric Blake <ebb9@byu.net>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index cb08ae5..d9dcb0c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,7 @@
 2009-08-31  Herbert Xu <herbert@gondor.apana.org.au>
 
 	* Fix NUL termination in readcmd.
+	* Lookup PWD after going through CDPATH.
 
 2009-08-11  Herbert Xu <herbert@gondor.apana.org.au>
 
diff --git a/src/cd.c b/src/cd.c
index 3770664..9a69b69 100644
--- a/src/cd.c
+++ b/src/cd.c
@@ -106,7 +106,7 @@ cdcmd(int argc, char **argv)
 	if (!dest)
 		dest = nullstr;
 	if (*dest == '/')
-		goto step7;
+		goto step6;
 	if (*dest == '.') {
 		c = dest[1];
 dotdot:
@@ -122,13 +122,8 @@ dotdot:
 	}
 	if (!*dest)
 		dest = ".";
-	if (!(path = bltinlookup("CDPATH"))) {
-step6:
-step7:
-		p = dest;
-		goto docd;
-	}
-	do {
+	path = bltinlookup("CDPATH");
+	while (path) {
 		c = *path;
 		p = padvance(&path, dest);
 		if (stat(p, &statb) >= 0 && S_ISDIR(statb.st_mode)) {
@@ -137,9 +132,15 @@ step7:
 docd:
 			if (!docd(p, flags))
 				goto out;
-			break;
+			goto err;
 		}
-	} while (path);
+	}
+
+step6:
+	p = dest;
+	goto docd;
+
+err:
 	sh_error("can't cd to %s", dest);
 	/* NOTREACHED */
 out:

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: bugs in cd

[Herbert Xu]

On Tue, Jul 14, 2009 at 09:39:03PM +0000, Eric Blake wrote:
> 
> Furthermore, POSIX requires that if the element in CDPATH ends in slash, that 
> no additional slashes are added while forming the candidate curpath.  In light 
> of the fact that //home need not be the same directory as /home (and indeed, on 
> cygwin, they are distinct entities), this is also a bug:

Can you quote where POSIX says this? I can't find anything that
stipulates this in my copy.  In fact, I can't even find anything
that allows it (only 3 or more leading slashes may be removed).

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: bugs in cd

[Eric Blake]

According to Herbert Xu on 8/31/2009 6:18 AM:
> On Tue, Jul 14, 2009 at 09:39:03PM +0000, Eric Blake wrote:
>> Furthermore, POSIX requires that if the element in CDPATH ends in slash, that 
>> no additional slashes are added while forming the candidate curpath.  In light 
>> of the fact that //home need not be the same directory as /home (and indeed, on 
>> cygwin, they are distinct entities), this is also a bug:
> 
> Can you quote where POSIX says this?

Which version of POSIX are you looking at?  POSIX 2008 added quite a few
clarifications about the handling of // that were not listed in POSIX 2001.

http://www.opengroup.org/onlinepubs/9699919799/utilities/cd.html

"5. Starting with the first pathname in the <colon>-separated pathnames of
CDPATH (see the ENVIRONMENT VARIABLES section) if the pathname is
non-null, test if the concatenation of that pathname, a <slash> character
if that pathname did not end with a <slash> character, and the directory
operand names a directory."...

In other words, if CDPATH is "/", then you should not append any
additional <slash> characters, such that you end up checking for the
existence of "/foo", not "//foo".

-- 
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net

Re: bugs in cd

[Herbert Xu]

On Mon, Aug 31, 2009 at 06:23:27AM -0600, Eric Blake wrote:
>
> Which version of POSIX are you looking at?  POSIX 2008 added quite a few
> clarifications about the handling of // that were not listed in POSIX 2001.
> 
> http://www.opengroup.org/onlinepubs/9699919799/utilities/cd.html
> 
> "5. Starting with the first pathname in the <colon>-separated pathnames of
> CDPATH (see the ENVIRONMENT VARIABLES section) if the pathname is
> non-null, test if the concatenation of that pathname, a <slash> character
> if that pathname did not end with a <slash> character, and the directory
> operand names a directory."...
> 
> In other words, if CDPATH is "/", then you should not append any
> additional <slash> characters, such that you end up checking for the
> existence of "/foo", not "//foo".

Fair enough.  However, I noticed that they haven't fixed up PATH
to do the same thing.

Since we use the same function for PATH and CDPATH, I'm not going
to make any changes until POSIX becomes consistent in this respect.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: bugs in cd

[Eric Blake]

Herbert Xu <herbert <at> gondor.apana.org.au> writes:

> > In other words, if CDPATH is "/", then you should not append any
> > additional <slash> characters, such that you end up checking for the
> > existence of "/foo", not "//foo".
> 
> Fair enough.  However, I noticed that they haven't fixed up PATH
> to do the same thing.

Most likely an unintentional oversight.  But to make sure, I've created a 
defect report:

http://austingroupbugs.net/view.php?id=139

https://www.opengroup.org/sophocles/show_mail.tpl?
CALLER=index.tpl&source=L&listname=austin-group-l&id=12720

[the former link may require free registration with the Austin group Mantis 
tracker; the latter is the public email summary generated from my edits]

> Since we use the same function for PATH and CDPATH, I'm not going
> to make any changes until POSIX becomes consistent in this respect.

Fair enough; I'll keep you posted when the Austin group rules on my report.

-- 
Eric Blake

Re: bugs in cd

[Eric Blake]

According to Eric Blake on 8/31/2009 8:08 AM:
> Herbert Xu <herbert <at> gondor.apana.org.au> writes:
> 
>>> In other words, if CDPATH is "/", then you should not append any
>>> additional <slash> characters, such that you end up checking for the
>>> existence of "/foo", not "//foo".
>> Fair enough.  However, I noticed that they haven't fixed up PATH
>> to do the same thing.

>> Since we use the same function for PATH and CDPATH, I'm not going
>> to make any changes until POSIX becomes consistent in this respect.

> Fair enough; I'll keep you posted when the Austin group rules on my report.
> http://austingroupbugs.net/view.php?id=139

The Austin Group has now made their ruling; there is a 30 day clock before
the interpretation becomes official, but consensus will probably be
granted to the proposed wording:

> Interpretation response
> ------------------------
> The standard states the requirements for PATH
> and conforming implementations must conform to this. However, concerns
> have been raised about this which are being referred to the sponsor."
> 
> Rationale:
> -------------
> This is for consistency with the previous change to CDPATH made in interpretation 1003.1-2001 #199
> https://www.opengroup.org/austin/interps/uploads/40/15188/AI-199.txt [^]
> 
> 
> Notes to the Editor (not part of this interpretation):
> -------------------------------------------------------

> At line 5674, replace the sentence:
> 
> When a nonzero-length prefix is applied to this filename, a <slash> shall be inserted between the prefix and the filename.
> 
> with:
> 
> When a nonzero-length prefix is applied to this filename, a <slash> shall be inserted between the prefix and the filename if the prefix did not end in <slash>.


-- 
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net