Re: mkdir builtin and $'\0'

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Fri, 21 Aug 2015 08:20:55 +0200
Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
> % mkdir /tmp/ホ
> % cd /tmp/ホ
> cd: no such file or directory: /tmp/ホ
> % cd /tmp
> % cd ホ
> % pwd
> /tmp/ホ
> % echo ホ|xxd
> 00000000: e383 9b0a
> 
> Do we have inconsistent metafication in this string somehow?

Yes --- it appears by tracing system calls everything is unmetafied but
null terminated up to lchdir().  The culprit for the case Stephane saw
was the second of a pair of lchdir() calls, where there's a string that
needs unmetafying, while you're hitting the first, which is raw.  I
think this fixes both cases.

I think we weren't seeing this before because Stephane's case is
actually quite unusual --- we failed to changed directory normally
because it doesn't exist, and only then hit the second lchdir().

There are a couple of remaining lchdir() calls in glob.c.  I haven't
determined whether the arguments are already unmetafied or not.

(Er, this is if people think we really need to fix up little-used
builtins of this sort... mmm....)

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index 572a0dd..3d34aa7 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1163,7 +1163,7 @@ cd_try_chdir(char *pfix, char *dest, int hard)
      * or a parent directory is renamed in the interim.
      */
     if (lchdir(buf, NULL, hard) &&
-	(pfix || *dest == '/' || lchdir(dest, NULL, hard))) {
+	(pfix || *dest == '/' || lchdir(unmeta(dest), NULL, hard))) {
 	free(buf);
 	return NULL;
     }
diff --git a/Src/compat.c b/Src/compat.c
index a0ce182..db46852 100644
--- a/Src/compat.c
+++ b/Src/compat.c
@@ -454,8 +454,13 @@ zgetcwd(void)
     return ret;
 }
 
-/* chdir with arbitrary long pathname.  Returns 0 on success, -1 on normal *
- * failure and -2 when chdir failed and the current directory is lost.  */
+/*
+ * chdir with arbitrary long pathname.  Returns 0 on success, -1 on normal *
+ * failure and -2 when chdir failed and the current directory is lost.
+ *
+ * This is to be treated as if at system level, so dir is unmetafied but
+ * terminated by a NULL.
+ */
 
 /**/
 mod_export int
@@ -465,7 +470,7 @@ zchdir(char *dir)
     int currdir = -2;
 
     for (;;) {
-	if (!*dir || chdir(unmeta(dir)) == 0) {
+	if (!*dir || chdir(dir) == 0) {
 #ifdef HAVE_FCHDIR
            if (currdir >= 0)
                close(currdir);
diff --git a/Src/utils.c b/Src/utils.c
index 20e01a2..4c4dc55 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -6440,10 +6440,15 @@ init_dirsav(Dirsav d)
     d->dirfd = d->level = -1;
 }
 
-/* Change directory, without following symlinks.  Returns 0 on success, -1 *
- * on failure.  Sets errno to ENOTDIR if any symlinks are encountered.  If *
- * fchdir() fails, or the current directory is unreadable, we might end up *
- * in an unwanted directory in case of failure.                            */
+/*
+ * Change directory, without following symlinks.  Returns 0 on success, -1
+ * on failure.  Sets errno to ENOTDIR if any symlinks are encountered.  If
+ * fchdir() fails, or the current directory is unreadable, we might end up
+ * in an unwanted directory in case of failure.
+ *
+ * path is an unmetafied but null-terminated string, as needed by system
+ * calls.
+ */
 
 /**/
 mod_export int
diff --git a/Test/D07multibyte.ztst b/Test/D07multibyte.ztst
index 644d280..0e3e98d 100644
--- a/Test/D07multibyte.ztst
+++ b/Test/D07multibyte.ztst
@@ -496,3 +496,15 @@
 >OK
 >OK
 >OK
+
+  () {
+     emulate -L zsh
+     setopt errreturn
+     local cdpath=(.)
+     mkdir ホ
+     cd ホ
+     cd ..
+     cd ./ホ
+     cd ..
+  }
+0:cd with special characters