Re: cd -s symlink hangs (sometimes?)

[Peter Stephenson <pws@csr.com>]

On Tue, 24 Mar 2009 08:15:38 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mar 24, 12:46pm, Peter Stephenson wrote:
> }
> } For the diagnostics and pwd, I've just borrowed what the recursive
> } handling in zsh/files does, which is cd to /, set pwd consistently,
> } and report the error.
> 
> This is calling zerr(), which sets errflag, which means a script that
> encounters this problem would abort at that point, correct?  (That
> seems to be the behavior in a few simple tests I did.)

Yes, this seems serious enough.

> I'd be worried that if the only error is that chdir returns nonzero,
> changing into the root directory creates a security problem (what if
> the user is privileged and the next command is "rm -rf *" etc.).
> 
> Maybe it would be better to first attempt chdir($HOME) and only fall
> back on "/" if that also fails.

That makes sense, that is what "cd" on its own does, after all.

> Also, should $OLDPWD be getting updated?
> I'm undecided on that.  We did change directories, even though not to the
> desired destination.

Codewise, we're a little bit too low down to deal with this easily and
correctly.  I think the only consistent way of doing this would be to call
cd_new_pd(), which we currently only do on a successful cd or pushd.
Anything else looks to me like an inconsistent half measure.  We would then
be executing chpwd (etc.); this might be correct (fixing up a terminal
header, for example), or it might do stuff the user didn't want.  It's hard
to decide.

Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.219
diff -u -r1.219 utils.c
--- Src/utils.c	24 Mar 2009 12:52:07 -0000	1.219
+++ Src/utils.c	24 Mar 2009 15:58:30 -0000
@@ -5493,16 +5493,30 @@
     }
     if (restoredir(d)) {
 	int restoreerr = errno;
+	int i;
 	/*
 	 * Failed to restore the directory.
 	 * Just be definite, cd to root and report the result.
 	 */
-	zsfree(pwd);
-	pwd = ztrdup("/");
-	if (chdir(pwd) < 0)
+	for (i = 0; i < 2; i++) {
+	    const char *cdest;
+	    if (i)
+		cdest = "/";
+	    else {
+		if (!home)
+		    continue;
+		cdest = home;
+	    }
+	    zsfree(pwd);
+	    pwd = ztrdup(cdest);
+	    if (chdir(pwd) == 0)
+		break;
+	}
+	if (i == 2)
 	    zerr("lost current directory, failed to cd to /: %e", errno);
 	else
-	    zerr("lost current directory: %e: changed to /", restoreerr);
+	    zerr("lost current directory: %e: changed to `%s'", restoreerr,
+		pwd);
 	if (d == &ds)
 	    zsfree(ds.dirname);
 #ifdef HAVE_FCHDIR


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070