Skip to content

Commit

Permalink
[spec/command_] Only search for executable files.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Andy Chu committed Jul 7, 2019
1 parent a89c986 commit deb45c9
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 33 deletions.
5 changes: 3 additions & 2 deletions core/process.py
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion demo/path-cache.sh
Expand Up @@ -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'
}
Expand Down
2 changes: 1 addition & 1 deletion osh/cmd_exec.py
Expand Up @@ -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:
Expand Down
13 changes: 11 additions & 2 deletions osh/state.py
Expand Up @@ -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.
"""
Expand All @@ -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
Expand Down
12 changes: 5 additions & 7 deletions spec/builtin-bash.test.sh
Expand Up @@ -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 =
Expand Down
19 changes: 19 additions & 0 deletions spec/builtin-eval-source.test.sh
Expand Up @@ -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
Expand All @@ -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'
Expand Down
26 changes: 26 additions & 0 deletions spec/builtins2.test.sh
Expand Up @@ -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 "$@"
Expand Down
6 changes: 3 additions & 3 deletions spec/command_.test.sh
Expand Up @@ -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
20 changes: 3 additions & 17 deletions spec/posix.test.sh
Expand Up @@ -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

0 comments on commit deb45c9

Please sign in to comment.