From deb45c9861f8752a5763bbeea211eec4bbf9b42b Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Sun, 7 Jul 2019 00:05:36 -0700 Subject: [PATCH] [spec/command_] Only search for executable files. Command execution, command -v, and type -t now only search for executable files. 'source' searches for non-executables, and is not cached. Also re-organize spec tests. --- core/process.py | 5 +++-- demo/path-cache.sh | 2 +- osh/cmd_exec.py | 2 +- osh/state.py | 13 +++++++++++-- spec/builtin-bash.test.sh | 12 +++++------- spec/builtin-eval-source.test.sh | 19 +++++++++++++++++++ spec/builtins2.test.sh | 26 ++++++++++++++++++++++++++ spec/command_.test.sh | 6 +++--- spec/posix.test.sh | 20 +++----------------- 9 files changed, 72 insertions(+), 33 deletions(-) diff --git a/core/process.py b/core/process.py index 9a228a2436..f9cf1047b3 100644 --- a/core/process.py +++ b/core/process.py @@ -469,8 +469,9 @@ def _Exec(self, argv0_path, argv, argv0_spid, environ, should_retry): except OSError as e: # Run with /bin/sh when ENOEXEC error (no shebang). All shells do this. if e.errno == errno.ENOEXEC and should_retry: - self._Exec('/bin/sh', ['/bin/sh', argv0_path] + argv[1:], argv0_spid, - environ, False) + new_argv = ['/bin/sh', argv0_path] + new_argv.extend(argv[1:]) + self._Exec('/bin/sh', new_argv, argv0_spid, environ, False) # NO RETURN # Would be nice: when the path is relative and ENOENT: print PWD and do diff --git a/demo/path-cache.sh b/demo/path-cache.sh index 5b6332f163..ffd0313433 100755 --- a/demo/path-cache.sh +++ b/demo/path-cache.sh @@ -13,7 +13,7 @@ set -o errexit sh-demo() { local sh=$1 - local syscalls='execve,stat,lstat' + local syscalls='execve,stat,lstat,access' #local syscalls='execve' strace -ff -e $syscalls -- $sh -c 'whoami; whoami' } diff --git a/osh/cmd_exec.py b/osh/cmd_exec.py index 785263f439..62115079a6 100755 --- a/osh/cmd_exec.py +++ b/osh/cmd_exec.py @@ -219,7 +219,7 @@ def _Source(self, arg_vec): except IndexError: raise args.UsageError('missing required argument') - resolved = self.search_path.Lookup(path) + resolved = self.search_path.Lookup(path, exec_required=False) if resolved is None: resolved = path try: diff --git a/osh/state.py b/osh/state.py index 99b779c999..60662eed20 100644 --- a/osh/state.py +++ b/osh/state.py @@ -46,7 +46,7 @@ def __init__(self, mem): self.mem = mem self.cache = {} - def Lookup(self, name): + def Lookup(self, name, exec_required=True): """ Returns the path itself (for relative path), the resolve path, or None. """ @@ -66,7 +66,16 @@ def Lookup(self, name): for path_dir in path_list: full_path = os_path.join(path_dir, name) - if path_stat.exists(full_path): + + # NOTE: dash and bash only check for EXISTENCE in 'command -v' (and 'type + # -t'). OSH follows mksh and zsh. Note that we can still get EPERM if + # the permissions are changed between check and use. + if exec_required: + found = posix.access(full_path, posix.X_OK) + else: + found = path_stat.exists(full_path) # for 'source' + + if found: return full_path return None diff --git a/spec/builtin-bash.test.sh b/spec/builtin-bash.test.sh index a195efc1c0..764968966d 100644 --- a/spec/builtin-bash.test.sh +++ b/spec/builtin-bash.test.sh @@ -44,18 +44,16 @@ file file ## END -#### type -t and command -v with non-executable file still finds it - -# PATH resolution is different - +#### type -t doesn't find non-executable (like command -v) PATH="$TMP:$PATH" touch $TMP/non-executable type -t non-executable -command -v not-executable | grep -o /not-executable -## STDOUT: +## stdout-json: "" +## status: 1 +## BUG bash STDOUT: file -/not-executable ## END +## BUG bash status: 0 #### type -t -> not found type -t echo ZZZ find = diff --git a/spec/builtin-eval-source.test.sh b/spec/builtin-eval-source.test.sh index b7f9b04277..7a2043cedc 100755 --- a/spec/builtin-eval-source.test.sh +++ b/spec/builtin-eval-source.test.sh @@ -155,6 +155,16 @@ status=127 ## N-I mksh stdout-json: "" ## N-I mksh status: 1 +#### source looks in PATH for files +mkdir -p dir +echo "echo hi" > dir/cmd +PATH="dir:$PATH" +. cmd +rm dir/cmd +## STDOUT: +hi +## END + #### source finds files in PATH before current dir cd $TMP mkdir -p dir @@ -167,6 +177,15 @@ echo status=$? path status=0 ## END + +#### source works for files in subdirectory +mkdir -p dir +echo "echo path" > dir/cmd +. dir/cmd +rm dir/cmd +## STDOUT: +path +## END #### exit within eval (regression) eval 'exit 42' diff --git a/spec/builtins2.test.sh b/spec/builtins2.test.sh index 022b4d66c7..492066e1b0 100644 --- a/spec/builtins2.test.sh +++ b/spec/builtins2.test.sh @@ -57,6 +57,32 @@ myfunc status=1 ## END +#### command -v doesn't find non-executable file +# PATH resolution is different + +PATH="$TMP:$PATH" +touch $TMP/non-executable $TMP/executable +chmod +x $TMP/executable + +command -v non-executable | grep -o /non-executable +echo status=$? + +command -v executable | grep -o /executable +echo status=$? + +## STDOUT: +status=1 +/executable +status=0 +## END + +## BUG bash/dash STDOUT: +/non-executable +status=0 +/executable +status=0 +## END + #### command skips function lookup seq() { echo "$@" diff --git a/spec/command_.test.sh b/spec/command_.test.sh index bfde5522dd..933cbed1bb 100644 --- a/spec/command_.test.sh +++ b/spec/command_.test.sh @@ -187,9 +187,9 @@ status=0 status=0 ## END -#### hash -r with additional args -hash -r extra >/dev/null # avoid weird output with mksh +#### hash -r doesn't allow additional args +hash -r whoami >/dev/null # avoid weird output with mksh echo status=$? ## stdout: status=1 ## OK osh stdout: status=2 -## BUG dash stdout: status=0 +## BUG dash/bash stdout: status=0 diff --git a/spec/posix.test.sh b/spec/posix.test.sh index 83439683aa..4cce79229e 100644 --- a/spec/posix.test.sh +++ b/spec/posix.test.sh @@ -140,22 +140,8 @@ one EOF1 three EOF2 -## stdout-json: "one\ntwo\nthree\n" - -#### source works for files in subdirectory -mkdir -p dir -echo "echo path" > dir/cmd -. dir/cmd -rm dir/cmd -## STDOUT: -path - -#### source looks in PATH for files -mkdir -p dir -echo "echo hi" > dir/cmd -PATH="dir:$PATH" -. cmd -rm dir/cmd ## STDOUT: -hi +one +two +three ## END