From 3bc58164494eecc180e2bad966d7753bfdd1e295 Mon Sep 17 00:00:00 2001
From: Kurtis Rader <krader@skepticism.us>
Date: Wed, 8 Jan 2020 19:14:31 -0800
Subject: [PATCH] Fix handling of skipped directories

The bug in `path_opentype()` fixed by this commit may affect other
scenarios but we know it affects autoloaded functions. Hence the unit
test for that scenario.

Fixes #1454
---
 src/cmd/ksh93/sh/path.c              | 16 +++++++++-------
 src/cmd/ksh93/tests/autoload.sh      | 15 +++++++++++++++
 src/cmd/ksh93/tests/data/skipped_dir | 15 +++++++++++++++
 src/cmd/ksh93/tests/meson.build      |  1 +
 4 files changed, 40 insertions(+), 7 deletions(-)
 create mode 100644 src/cmd/ksh93/tests/autoload.sh
 create mode 100644 src/cmd/ksh93/tests/data/skipped_dir

diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c
index 4d6084a14e02..04a44ebde5ff 100644
--- a/src/cmd/ksh93/sh/path.c
+++ b/src/cmd/ksh93/sh/path.c
@@ -479,28 +479,30 @@ Pathcomp_t *path_get(Shell_t *shp, const char *name) {
 //
 static_fn int path_opentype(Shell_t *shp, const char *name, Pathcomp_t *pp, int fun) {
     int fd = -1;
-    struct stat statb;
-    Pathcomp_t *oldpp;
 
     if (!pp && !shp->pathlist) path_init(shp);
     if (!fun && strchr(name, '/') && sh_isoption(shp, SH_RESTRICTED)) {
         errormsg(SH_DICT, ERROR_exit(1), e_restricted, name);
         __builtin_unreachable();
     }
+
+    // The structure of this loop is slightly odd. It's a consequence of how path_nextcomp() works.
+    Pathcomp_t *next_pp = 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))) continue;
+        pp = next_pp;
+        next_pp = path_nextcomp(shp, pp, name, NULL);
+        if (pp && (pp->flags & PATH_SKIP)) continue;
+        if (fun && (!pp || !(pp->flags & PATH_FPATH))) continue;
         fd = sh_open(path_relative(shp, stkptr(shp->stk, PATH_OFFSET)), O_RDONLY | O_CLOEXEC, 0);
+        struct stat statb;
         if (fd >= 0 && (fstat(fd, &statb) < 0 || S_ISDIR(statb.st_mode))) {
             errno = EISDIR;
             sh_close(fd);
             fd = -1;
         }
-    } while (fd < 0 && pp);
+    } while (fd < 0 && next_pp);
 
     assert(fd < 0 || sh_iovalidfd(shp, fd));
-
     if (fd >= 0 && (fd = sh_iomovefd(shp, fd)) > 0) {
         (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
         shp->fdstatus[fd] |= IOCLEX;
diff --git a/src/cmd/ksh93/tests/autoload.sh b/src/cmd/ksh93/tests/autoload.sh
new file mode 100644
index 000000000000..6aaa206e4cdf
--- /dev/null
+++ b/src/cmd/ksh93/tests/autoload.sh
@@ -0,0 +1,15 @@
+# Verify the behavior of autoloaded functions.
+
+# ====================
+# Verify that directories in the path search list which should be skipped (e.g., because they don't
+# exist) interacts correctly with autoloaded functions.
+#
+# See https://github.com/att/ast/issues/1454
+expect=$"Func cd called with |$TEST_DIR/usr|\n$TEST_DIR/usr"
+actual=$($SHELL "$TEST_ROOT/data/skipped_dir")
+actual_status=$?
+expect_status=0
+[[ $actual_status == $expect_status ]] ||
+    log_error "autoload function skipped dir test wrong status" "$expect_status" "$actual_status"
+[[ $actual == $expect ]] ||
+    log_error "autoload function skipped dir test wrong output" "$expect" "$actual"
diff --git a/src/cmd/ksh93/tests/data/skipped_dir b/src/cmd/ksh93/tests/data/skipped_dir
new file mode 100644
index 000000000000..b8eeddc04fa8
--- /dev/null
+++ b/src/cmd/ksh93/tests/data/skipped_dir
@@ -0,0 +1,15 @@
+# See https://github.com/att/ast/issues/1454
+
+mkdir -p "$TEST_DIR/usr/bin"
+print '#!/bin/sh' >"$TEST_DIR/usr/bin/cd"
+print 'builtin cd "$@"' >>"$TEST_DIR/usr/bin/cd"
+prefix="$TEST_DIR/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="/arglebargle:$PATH:$TEST_DIR/usr/bin:$TEST_DIR/bin"
+cd "$TEST_DIR/usr"
+pwd
diff --git a/src/cmd/ksh93/tests/meson.build b/src/cmd/ksh93/tests/meson.build
index 728b254cf36c..0f91e6445c7b 100644
--- a/src/cmd/ksh93/tests/meson.build
+++ b/src/cmd/ksh93/tests/meson.build
@@ -53,6 +53,7 @@ all_tests = [
     ['arrays.sh'],
     ['arrays2.sh'],
     ['attributes.sh'],
+    ['autoload.sh'],
     ['basic.sh', 90],
     ['bracket.sh'],
     ['builtins.sh'],