Skip to content

Commit

Permalink
Fix handling of skipped directories
Browse files Browse the repository at this point in the history
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
  • Loading branch information
krader1961 committed Jan 9, 2020
1 parent 5abcbd0 commit 3bc5816
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/cmd/ksh93/sh/path.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/cmd/ksh93/tests/autoload.sh
Original file line number Diff line number Diff line change
@@ -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"
15 changes: 15 additions & 0 deletions src/cmd/ksh93/tests/data/skipped_dir
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions src/cmd/ksh93/tests/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ all_tests = [
['arrays.sh'],
['arrays2.sh'],
['attributes.sh'],
['autoload.sh'],
['basic.sh', 90],
['bracket.sh'],
['builtins.sh'],
Expand Down

0 comments on commit 3bc5816

Please sign in to comment.