Merge branch 'more-exacting-which' into issue-44-solaris-try-3

3 files changed, 228 insertions, 81 deletions

diff --git a/pexpect/__init__.py b/pexpect/__init__.py
index f27e3fe..7220437 100644
--- a/pexpect/__init__.py
+++ b/pexpect/__init__.py
@@ -80,6 +80,7 @@ try:
     import traceback
     import signal
     import codecs
+    import stat
 except ImportError:  # pragma: no cover
     err = sys.exc_info()[1]
     raise ImportError(str(err) + '''
@@ -1957,15 +1958,48 @@ class searcher_re(object):
         return best_index
 
 
+def is_exe(fname):
+    # follow symlinks,
+    fpath = os.path.realpath(fname)
+
+    # return False for non-files (directories, fifo, etc.)
+    if not os.path.isfile(fpath):
+        return False
+
+    # On Solaris, etc., "If the process has appropriate privileges, an
+    # implementation may indicate success for X_OK even if none of the
+    # execute file permission bits are set."
+    #
+    # For this reason, it is necessary to explicitly check st_mode
+
+    # get file mode using os.stat, and check if `other',
+    # that is anybody, may read and execute.
+    mode = os.stat(fpath).st_mode
+    if mode & stat.S_IROTH and mode & stat.S_IXOTH:
+        return True
+
+    # get current user's group ids, and check if `group',
+    # when matching ours, may read and execute.
+    user_gids = os.getgroups() + [os.getgid()]
+    if (os.stat(fpath).st_gid in user_gids and
+            mode & stat.S_IRGRP and mode & stat.S_IXGRP):
+        return True
+
+    # finally, if file owner matches our effective userid,
+    # check if `user', may read and execute.
+    user_gids = os.getgroups() + [os.getgid()]
+    if (os.stat(fpath).st_uid == os.geteuid() and
+            mode & stat.S_IRUSR and mode & stat.S_IXUSR):
+        return True
+
+
 def which(filename):
     '''This takes a given filename; tries to find it in the environment path;
     then checks if it is executable. This returns the full path to the filename
     if found and executable. Otherwise this returns None.'''
 
     # Special case where filename contains an explicit path.
-    if (os.path.dirname(filename) != '' and
-            os.access(filename, os.X_OK) and
-            os.path.isfile(os.path.realpath(filename))):
+    if os.path.dirname(filename) != '' and is_exe(filename):
         return filename
     if 'PATH' not in os.environ or os.environ['PATH'] == '':
         p = os.defpath
@@ -1974,8 +2008,7 @@ def which(filename):
     pathlist = p.split(os.pathsep)
     for path in pathlist:
         ff = os.path.join(path, filename)
-        if (os.access(ff, os.X_OK) and
-                os.path.isfile(os.path.realpath(ff))):
+        if is_exe(ff):
             return ff
     return None
 
diff --git a/tests/test_misc.py b/tests/test_misc.py
index 7569070..66d943b 100755
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -21,9 +21,7 @@ PEXPECT LICENSE
 import pexpect
 import unittest
 from . import PexpectTestCase
-import os
 import sys
-import tempfile
 import re
 import signal
 import time
@@ -252,80 +250,6 @@ class TestCaseMisc(PexpectTestCase.PexpectTestCase):
         assert default!=tmpdir, "'default' and 'tmpdir' should be different"
         assert (b'tmp' in tmpdir), "'tmp' should be returned by 'pwd' command"
 
-    def test_basic_which(self):
-        # should find at least 'ls' program, and it should begin with '/'
-        exercise = pexpect.which("ls")
-        assert exercise is not None and exercise.startswith('/')
-
-    def test_absolute_which(self):
-        # make up a path and insert first a non-executable,
-        # then, make it executable, and assert we may which() find it.
-        fname = 'gcc'
-        bin_dir = tempfile.mkdtemp()
-        bin_path = os.path.join(bin_dir, fname)
-        save_path = os.environ['PATH']
-        try:
-            # setup
-            os.environ['PATH'] = bin_dir
-            with open(bin_path, 'w') as fp:
-                fp.write('#!/bin/sh\necho hello, world\n')
-            os.chmod(bin_path, 0o400)
-
-            # it should not be found because it is not executable
-            assert pexpect.which(fname) is None, fname
-
-            # but now it should -- because it is executable
-            os.chmod(bin_path, 0o700)
-            assert pexpect.which(fname) == bin_path, (fname, bin_path)
-
-        finally:
-            # restore,
-            os.environ['PATH'] = save_path
-            # destroy scratch files and folders,
-            if os.path.exists(bin_path):
-                os.unlink(bin_path)
-            if os.path.exists(bin_dir):
-                os.rmdir(bin_dir)
-
-    def test_which_should_not_match_folders(self):
-        # make up a path and insert a folder, which is 'executable', which
-        # a naive implementation might match (previously pexpect versions
-        # 3.2 and sh versions 1.0.8, reported by @lcm337.)
-        fname = 'g++'
-        bin_dir = tempfile.mkdtemp()
-        bin_dir2 = os.path.join(bin_dir, fname)
-        save_path = os.environ['PATH']
-        try:
-            os.environ['PATH'] = bin_dir
-            os.mkdir(bin_dir2, 0o755)
-            # it should not be found because it is not executable *file*,
-            # but rather, has the executable bit set, as a good folder
-            # should -- it shouldn't be returned because it fails isdir()
-            exercise = pexpect.which(fname)
-            assert exercise is None, exercise
-
-        finally:
-            # restore,
-            os.environ['PATH'] = save_path
-            # destroy scratch folders,
-            for _dir in (bin_dir2, bin_dir,):
-                if os.path.exists(_dir):
-                    os.rmdir(_dir)
-
-    def test_which (self):
-        p = os.defpath
-        ep = os.environ['PATH']
-        os.defpath = ":/tmp"
-        os.environ['PATH'] = ":/tmp"
-        wp = pexpect.which ("ticker.py")
-        assert wp == 'ticker.py', "Should return a string. Returned %s" % wp
-        os.defpath = "/tmp"
-        os.environ['PATH'] = "/tmp"
-        wp = pexpect.which ("ticker.py")
-        assert wp == None, "Executable should not be found. Returned %s" % wp
-        os.defpath = p
-        os.environ['PATH'] = ep
-
     def test_searcher_re (self):
         # This should be done programatically, if we copied and pasted output,
         # there wouldnt be a whole lot to test, really, other than our ability
diff --git a/tests/test_which.py b/tests/test_which.py
new file mode 100644
index 0000000..83575fb
--- /dev/null
+++ b/tests/test_which.py
@@ -0,0 +1,190 @@
+import tempfile
+import os
+
+import pexpect
+from . import PexpectTestCase
+
+
+class TestCaseWhich(PexpectTestCase.PexpectTestCase):
+    " Tests for pexpect.which(). "
+
+    def test_which_finds_ls(self):
+        " which() can find ls(1). "
+        exercise = pexpect.which("ls")
+        assert exercise is not None
+        assert exercise.startswith('/')
+
+    def test_os_defpath_which(self):
+        " which() finds an executable in $PATH and returns its abspath. "
+        fname = 'cc'
+        bin_dir = tempfile.mkdtemp()
+        bin_path = os.path.join(bin_dir, fname)
+        save_path = os.environ['PATH']
+        save_defpath = os.defpath
+
+        try:
+            # setup
+            os.environ['PATH'] = ''
+            os.defpath = bin_dir
+            with open(bin_path, 'w') as fp:
+                pass
+
+            # given non-executable,
+            os.chmod(bin_path, 0o400)
+
+            # exercise absolute and relative,
+            assert pexpect.which(bin_path) is None
+            assert pexpect.which(fname) is None
+
+            # given executable,
+            os.chmod(bin_path, 0o700)
+
+            # exercise absolute and relative,
+            assert pexpect.which(bin_path) == bin_path
+            assert pexpect.which(fname) == bin_path
+
+        finally:
+            # restore,
+            os.environ['PATH'] = save_path
+            os.defpath = save_defpath
+
+            # destroy scratch files and folders,
+            if os.path.exists(bin_path):
+                os.unlink(bin_path)
+            if os.path.exists(bin_dir):
+                os.rmdir(bin_dir)
+
+    def test_path_search_which(self):
+        " which() finds an executable in $PATH and returns its abspath. "
+        fname = 'gcc'
+        bin_dir = tempfile.mkdtemp()
+        bin_path = os.path.join(bin_dir, fname)
+        save_path = os.environ['PATH']
+        try:
+            # setup
+            os.environ['PATH'] = bin_dir
+            with open(bin_path, 'w') as fp:
+                pass
+
+            # given non-executable,
+            os.chmod(bin_path, 0o400)
+
+            # exercise absolute and relative,
+            assert pexpect.which(bin_path) is None
+            assert pexpect.which(fname) is None
+
+            # given executable,
+            os.chmod(bin_path, 0o700)
+
+            # exercise absolute and relative,
+            assert pexpect.which(bin_path) == bin_path
+            assert pexpect.which(fname) == bin_path
+
+        finally:
+            # restore,
+            os.environ['PATH'] = save_path
+
+            # destroy scratch files and folders,
+            if os.path.exists(bin_path):
+                os.unlink(bin_path)
+            if os.path.exists(bin_dir):
+                os.rmdir(bin_dir)
+
+    def test_which_follows_symlink(self):
+        " which() follows symlinks and returns its path. "
+        fname = 'original'
+        symname = 'extra-crispy'
+        bin_dir = tempfile.mkdtemp()
+        bin_path = os.path.join(bin_dir, fname)
+        sym_path = os.path.join(bin_dir, symname)
+        save_path = os.environ['PATH']
+        try:
+            # setup
+            os.environ['PATH'] = bin_dir
+            with open(bin_path, 'w') as fp:
+                pass
+            os.chmod(bin_path, 0o400)
+            os.symlink(bin_path, sym_path)
+
+            # should not be found because symlink points to non-executable
+            assert pexpect.which(symname) is None
+
+            # but now it should -- because it is executable
+            os.chmod(bin_path, 0o700)
+            assert pexpect.which(symname) == sym_path
+
+        finally:
+            # restore,
+            os.environ['PATH'] = save_path
+
+            # destroy scratch files, symlinks, and folders,
+            if os.path.exists(sym_path):
+                os.unlink(sym_path)
+            if os.path.exists(bin_path):
+                os.unlink(bin_path)
+            if os.path.exists(bin_dir):
+                os.rmdir(bin_dir)
+
+    def test_which_should_not_match_folders(self):
+        " Which does not match folders, even though they are executable. "
+        # make up a path and insert a folder that is 'executable', a naive
+        # implementation might match (previously pexpect versions 3.2 and
+        # sh versions 1.0.8, reported by @lcm337.)
+        fname = 'g++'
+        bin_dir = tempfile.mkdtemp()
+        bin_dir2 = os.path.join(bin_dir, fname)
+        save_path = os.environ['PATH']
+        try:
+            os.environ['PATH'] = bin_dir
+            os.mkdir(bin_dir2, 0o755)
+            # should not be found because it is not executable *file*,
+            # but rather, has the executable bit set, as a good folder
+            # should -- it should not be returned because it fails isdir()
+            exercise = pexpect.which(fname)
+            assert exercise is None
+
+        finally:
+            # restore,
+            os.environ['PATH'] = save_path
+            # destroy scratch folders,
+            for _dir in (bin_dir2, bin_dir,):
+                if os.path.exists(_dir):
+                    os.rmdir(_dir)
+
+    def test_which_should_match_other_group_user(self):
+        " which() returns executables by other, group, and user ownership. "
+        # create an executable and test that it is found using which() for
+        # each of the 'other', 'group', and 'user' permission bits.
+        fname = 'g77'
+        bin_dir = tempfile.mkdtemp()
+        bin_path = os.path.join(bin_dir, fname)
+        save_path = os.environ['PATH']
+        try:
+            # setup
+            os.environ['PATH'] = bin_dir
+            with open(bin_path, 'w') as fp:
+                fp.write('#!/bin/sh\necho hello, world\n')
+            for should_match, mode in ((False, 0o000),
+                                       (True,  0o005),
+                                       (True,  0o050),
+                                       (True,  0o500),
+                                       (False, 0o004),
+                                       (False, 0o040),
+                                       (False, 0o400)):
+                os.chmod(bin_path, mode)
+
+                if not should_match:
+                    # should not be found because it is not executable
+                    assert pexpect.which(fname) is None
+                else:
+                    # should match full path
+                    assert pexpect.which(fname) == bin_path
+
+        finally:
+            # restore,
+            os.environ['PATH'] = save_path
+            # destroy scratch files and folders,
+            if os.path.exists(bin_path):
+                os.unlink(bin_path)
+            if os.path.exists(bin_dir):
+                os.rmdir(bin_dir)