Fix handling of skipped directories when autoloading functions

McDutchie · McDutchie · commit eee47df42337 · 2020-06-12T01:45:15.000+02:00
Fix a bug in autoloading functions. Directories in the path search list which should be skipped (e.g. because they don't exist) did not interact correctly with autoloaded functions, so that a function to autoload was not always found. Details: att#1454 Fix backported (and cleaned up) from: att@3bc58164 src/cmd/ksh93/sh/path.c: - path_opentype(): Fix the path search loop so that entries marked with PATH_SKIP are handled correctly. src/cmd/ksh93/tests/functions.sh: - Add regression test verifying an autoloaded function with a PATH that triggered the bug. The bug in path_opentype() fixed by this commit may affect other scenarios but we know it affects autoloaded functions. Hence the test for that scenario. (cherry picked from commit a27903165775309f4f032de5d42ec1785f14cfbc)
diff --git a/NEWS b/NEWS
@@ -4,6 +4,14 @@ For full details, see the git log at:
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2020-05-31:
+
+- Fix a bug in autoloading functions. Directories in the path search list
+  which should be skipped (e.g. because they don't exist) did not interact
+  correctly with autoloaded functions, so that a function to autoload was
+  not always found correctly.
+  Details: https://github.com/att/ast/issues/1454
+
 2020-05-30:
 
 - Fix POSIX compliance of 'test'/'[' exit status on error. The command now
diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h
@@ -17,4 +17,4 @@
 *                  David Korn <dgk@research.att.com>                   *
 *                                                                      *
 ***********************************************************************/
-#define SH_RELEASE	"93u+m 2020-05-30"
+#define SH_RELEASE	"93u+m 2020-05-31"
diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c
@@ -508,20 +508,24 @@ static int	path_opentype(Shell_t *shp,const char *name, register Pathcomp_t *pp,
 {
 	register int fd= -1;
 	struct stat statb;
-	Pathcomp_t *oldpp;
+	Pathcomp_t *nextpp;
+
 	if(!pp && !shp->pathlist)
 		path_init(shp);
 	if(!fun && strchr(name,'/'))
 	{
 		if(sh_isoption(SH_RESTRICTED))
 			errormsg(SH_DICT,ERROR_exit(1),e_restricted,name);
 	}
+
+	nextpp = pp;
 	do
 	{
-		pp = path_nextcomp(shp,oldpp=pp,name,0);
-		while(oldpp && (oldpp->flags&PATH_SKIP))
-			oldpp = oldpp->next;
-		if(fun && (!oldpp || !(oldpp->flags&PATH_FPATH)))
+		pp = nextpp;
+		nextpp = path_nextcomp(shp,pp,name,0);
+		if(pp && (pp->flags&PATH_SKIP))
+			continue;
+		if(fun && (!pp || !(pp->flags&PATH_FPATH)))
 			continue;
 		if((fd = sh_open(path_relative(shp,stakptr(PATH_OFFSET)),O_RDONLY,0)) >= 0)
 		{
@@ -533,7 +537,8 @@ static int	path_opentype(Shell_t *shp,const char *name, register Pathcomp_t *pp,
 			}
 		}
 	}
-	while( fd<0 && pp);
+	while(fd<0 && nextpp);
+
 	if(fd>=0 && (fd = sh_iomovefd(fd)) > 0)
 	{
 		fcntl(fd,F_SETFD,FD_CLOEXEC);
diff --git a/src/cmd/ksh93/tests/functions.sh b/src/cmd/ksh93/tests/functions.sh
@@ -1213,4 +1213,33 @@ rc=$?
 exp=$((256+$(kill -l TERM) ))
 [[  $rc == "$exp" ]] || err_exit "expected exitval $exp got $rc"
 
+# ======
+# Verify that directories in the path search list which should be skipped
+# (e.g. because they don't exist) interact correctly with autoloaded functions.
+# See https://github.com/att/ast/issues/1454
+expect="Func cd called with |$tmp/usr|
+$tmp/usr"
+actual=$(
+	set -- wrong args passed
+	mkdir -p "$tmp/usr/bin"
+	print 'echo "wrong file executed ($*)"' >"$tmp/usr/bin/cd"
+	prefix=$tmp/ksh.$$
+
+	FPATH=$prefix/bad:$prefix/functions
+	mkdir -p "$prefix/functions"
+	print 'function cd { echo "Func cd called with |$*|"; command cd "$@"; }' >"$prefix/functions/cd"
+	typeset -fu cd
+
+	PATH=$tmp/arglebargle:$PATH:$tmp/usr/bin:$tmp/bin
+	cd "$tmp/usr"
+	pwd
+)
+actual_status=$?
+expect_status=0
+[[ $actual_status == "$expect_status" ]] ||
+    err_exit "autoload function skipped dir test wrong status (expected $expect_status, got $actual_status)"
+[[ $actual == "$expect" ]] ||
+    err_exit "autoload function skipped dir test wrong output (expected $(printf %q "$expect"), got $(printf %q "$actual"))"
+
+# ======
 exit $((Errors<125?Errors:125))

Original file line number	Diff line number	Diff line change
`@@ -508,20 +508,24 @@ static int path_opentype(Shell_t shp,const char name, register Pathcomp_t *pp,`
`508`	`508`	`{`
`509`	`509`	`register int fd= -1;`
`510`	`510`	`struct stat statb;`
`511`		`- Pathcomp_t *oldpp;`
	`511`	`+ Pathcomp_t *nextpp;`
	`512`	`+`
`512`	`513`	`if(!pp && !shp->pathlist)`
`513`	`514`	`path_init(shp);`
`514`	`515`	`if(!fun && strchr(name,'/'))`
`515`	`516`	`{`
`516`	`517`	`if(sh_isoption(SH_RESTRICTED))`
`517`	`518`	`errormsg(SH_DICT,ERROR_exit(1),e_restricted,name);`
`518`	`519`	`}`
	`520`	`+`
	`521`	`+ nextpp = pp;`
`519`	`522`	`do`
`520`	`523`	`{`
`521`		`- pp = path_nextcomp(shp,oldpp=pp,name,0);`
`522`		`- while(oldpp && (oldpp->flags&PATH_SKIP))`
`523`		`- oldpp = oldpp->next;`
`524`		`- if(fun && (!oldpp \|\| !(oldpp->flags&PATH_FPATH)))`
	`524`	`+ pp = nextpp;`
	`525`	`+ nextpp = path_nextcomp(shp,pp,name,0);`
	`526`	`+ if(pp && (pp->flags&PATH_SKIP))`
	`527`	`+ continue;`
	`528`	`+ if(fun && (!pp \|\| !(pp->flags&PATH_FPATH)))`
`525`	`529`	`continue;`
`526`	`530`	`if((fd = sh_open(path_relative(shp,stakptr(PATH_OFFSET)),O_RDONLY,0)) >= 0)`
`527`	`531`	`{`
`@@ -533,7 +537,8 @@ static int path_opentype(Shell_t shp,const char name, register Pathcomp_t *pp,`
`533`	`537`	`}`
`534`	`538`	`}`
`535`	`539`	`}`
`536`		`- while( fd<0 && pp);`
	`540`	`+ while(fd<0 && nextpp);`
	`541`	`+`
`537`	`542`	`if(fd>=0 && (fd = sh_iomovefd(fd)) > 0)`
`538`	`543`	`{`
`539`	`544`	`fcntl(fd,F_SETFD,FD_CLOEXEC);`