Running 'type' causes false positive hashed command completion

Running 'type' causes false positive hashed command completion

[Daniel Shahaf]

$ zsh -f
% cd $(mtemp -d)
% touch sudofoo; chmod +x $_
% ./sudo<TAB>
<becomes>
% ./sudofoo <^C>
% type -w ./sudo
./sudo: none
% ./sudo<TAB>
./sudo    sudofoo*

That's wrong because ./sudo does not exist.  However, it's hashed:

% print $commands[./sudo]
/usr/bin/./sudo

To confuse matters further, even though "./sudo" is hashed, a subsequent
'type -w ./sudo' will print "none", because the hash node lacks the
HASHED bit in its .flags and the PATH_DIRS option is unset by default.

Backtrace:

(gdb) 
890         cmdnamtab->addnode(cmdnamtab, ztrdup(arg0), cn);
(gdb) p arg0
$5 = 0x7ffff7ff2590 "./sudo"
(gdb) bt
#0  hashcmd (arg0=0x7ffff7ff2590 "./sudo", pp=0x7a3f98) at exec.c:890
#1  0x0000000000421507 in findcmd (arg0=0x7ffff7ff2590 "./sudo", docopy=1) at exec.c:776
#2  0x000000000041382c in bin_whence (nam=0x7ffff7ff2580 "type", argv=0x7ffff7ff25a0, ops=0x7fffffffda50, func=0) at builtin.c:3642
#3  0x0000000000407702 in execbuiltin (args=0x7ffff7ff2520, assigns=0x0, bn=0x76ed00 <builtins+4032>) at builtin.c:484
#4  0x000000000042a0e9 in execcmd (state=0x7fffffffe490, input=0, output=0, how=18, last1=1) at exec.c:3660
#5  0x00000000004246be in execpline2 (state=0x7fffffffe490, pcode=131, how=18, input=0, output=0, last1=1) at exec.c:1758
#6  0x000000000042360d in execpline (state=0x7fffffffe490, slcode=5122, how=18, last1=1) at exec.c:1536
#7  0x0000000000422a43 in execlist (state=0x7fffffffe490, dont_change_job=0, exiting=1) at exec.c:1294
#8  0x0000000000422297 in execode (p=0x7ffff7ff24b0, dont_change_job=0, exiting=1, context=0x53acd2 "cmdarg") at exec.c:1085
#9  0x000000000042216f in execstring (s=0x7fffffffe9c5 "type -w ./sudo", dont_change_job=0, exiting=1, context=0x53acd2 "cmdarg") at exec.c:1051
#10 0x00000000004460b9 in init_misc (cmd=0x7fffffffe9c5 "type -w ./sudo", zsh_name=0x7fffffffe9be "zsh") at init.c:1289
#11 0x00000000004478fb in zsh_main (argc=3, argv=0x7fffffffe688) at init.c:1673
#12 0x0000000000406a96 in main (argc=3, argv=0x7fffffffe688) at ./main.c:93

Originally reported as https://github.com/zsh-users/zsh-syntax-highlighting/issues/354.

Re: Running 'type' causes false positive hashed command completion

[Daniel Shahaf]

Daniel Shahaf wrote on Wed, Aug 24, 2016 at 20:58:49 +0000:
> $ zsh -f
> % cd $(mtemp -d)
> % touch sudofoo; chmod +x $_
> % ./sudo<TAB>
> <becomes>
> % ./sudofoo <^C>
> % type -w ./sudo
> ./sudo: none
> % ./sudo<TAB>
> ./sudo    sudofoo*

Add 'autoload compinit && compinit' at the start.

Daniel

P.S. I see somebody fixed the list archives — thanks!

Re: Running 'type' causes false positive hashed command completion

[Bart Schaefer]

On Aug 24,  8:58pm, Daniel Shahaf wrote:
} Subject: Running 'type' causes false positive hashed command completion
}
} $ zsh -f
} % cd $(mtemp -d)
} % touch sudofoo; chmod +x $_
} % ./sudo<TAB>
} <becomes>
} % ./sudofoo <^C>
} % type -w ./sudo
} ./sudo: none
} % ./sudo<TAB>
} ./sudo    sudofoo*
} 
} That's wrong because ./sudo does not exist.

This seems pretty obvious:


diff --git a/Src/exec.c b/Src/exec.c
index ea9214d..9b24d38 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -772,7 +772,7 @@ findcmd(char *arg0, int docopy)
     Cmdnam cn;
 
     cn = (Cmdnam) cmdnamtab->getnode(cmdnamtab, arg0);
-    if (!cn && isset(HASHCMDS))
+    if (!cn && isset(HASHCMDS) && !isrelative(arg0))
 	cn = hashcmd(arg0, path);
     if ((int) strlen(arg0) > PATH_MAX)
 	return NULL;


Can anyone think of a valid case which that breaks?  The doc (under the
PATH_DIRS option) even says "Commands explicitly beginning with `/', `./'
or `../' are not subject to the path search."

Re: Running 'type' causes false positive hashed command completion

[Daniel Shahaf]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

Bart Schaefer wrote on Thu, Aug 25, 2016 at 09:59:00 -0700:
> On Aug 24,  8:58pm, Daniel Shahaf wrote:
> } Subject: Running 'type' causes false positive hashed command completion
> }
> } $ zsh -f
> } % cd $(mtemp -d)
> } % touch sudofoo; chmod +x $_
> } % ./sudo<TAB>
> } <becomes>
> } % ./sudofoo <^C>
> } % type -w ./sudo
> } ./sudo: none
> } % ./sudo<TAB>
> } ./sudo    sudofoo*
> } 
> } That's wrong because ./sudo does not exist.
> 
> This seems pretty obvious:
> 
> 
> diff --git a/Src/exec.c b/Src/exec.c
> index ea9214d..9b24d38 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -772,7 +772,7 @@ findcmd(char *arg0, int docopy)
> -    if (!cn && isset(HASHCMDS))
> +    if (!cn && isset(HASHCMDS) && !isrelative(arg0))
>  	cn = hashcmd(arg0, path);
> 
> Can anyone think of a valid case which that breaks?  The doc (under the
> PATH_DIRS option) even says "Commands explicitly beginning with `/', `./'
> or `../' are not subject to the path search."

I've just noticed another bug (predates this patch):

% setopt pathdirs
% ./pwd 
zsh: no such file or directory: ./pwd
% type ./pwd          
./pwd is /bin/./pwd

That's wrong, 'type' should have returned 'none'.

Patch attached.

Cheers,

Daniel

[-- Attachment #2: 0001-whence-Honor-PATH_DIRS-option-for-arguments-that-sta.patch --]
[-- Type: text/x-diff, Size: 1935 bytes --]

>From d69ba249faeab0e6edb95062482d4f59fde01039 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Fri, 11 Nov 2016 02:25:21 +0000
Subject: [PATCH] whence: Honor PATH_DIRS option for arguments that start with
 './' or '../'.

While here, add some docstrings.
---
 Src/exec.c           | 23 ++++++++++++++++-------
 Test/E01options.ztst |  4 ++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/Src/exec.c b/Src/exec.c
index a01a633..ad80dd0 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -806,14 +806,13 @@ findcmd(char *arg0, int docopy, int default_path)
 	cn = hashcmd(arg0, path);
     if ((int) strlen(arg0) > PATH_MAX)
 	return NULL;
-    for (s = arg0; *s; s++)
-	if (*s == '/') {
-	    RET_IF_COM(arg0);
-	    if (arg0 == s || unset(PATHDIRS)) {
-		return NULL;
-	    }
-	    break;
+    if ((s = strchr(arg0, '/'))) {
+	RET_IF_COM(arg0);
+	if (arg0 == s || unset(PATHDIRS) || !strncmp(arg0, "./", 2) ||
+	    !strncmp(arg0, "../", 3)) {
+	    return NULL;
 	}
+    }
     if (cn) {
 	char nn[PATH_MAX];
 
@@ -848,6 +847,11 @@ findcmd(char *arg0, int docopy, int default_path)
     return NULL;
 }
 
+/*
+ * Return TRUE if the given path denotes an executable regular file, or a
+ * symlink to one.
+ */
+
 /**/
 int
 iscom(char *s)
@@ -877,6 +881,11 @@ isreallycom(Cmdnam cn)
     return iscom(fullnam);
 }
 
+/*
+ * Return TRUE if the given path contains a dot or dot-dot component
+ * and does not start with a slash.
+ */
+
 /**/
 int
 isrelative(char *s)
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 40e96af..45df9f5 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -784,6 +784,10 @@
 >unsetting option...
 ?(eval):14: no such file or directory: pathtestdir/findme
 
+  (setopt pathdirs; path+=( /usr/bin ); type ./env)
+1:whence honours PATH_DIRS option
+>./env not found
+
   setopt posixbuiltins
   PATH= command -v print
   PATH= command -V print