Skip to content

Commit

Permalink
Ignore PermissionError when checking cwd during import
Browse files Browse the repository at this point in the history
On macOS `getcwd(3)` can return EACCES if a path component isn't readable,
resulting in PermissionError. `PathFinder.find_spec()` now catches these and
ignores them - the same treatment as a missing/deleted cwd.

Introduces 2 new `test.support.os_helper` context managers

- `save_mode(path, ...)` - restores the mode of a path on exit
- `save_cwd(...)` - restores the current directory on exit

Unlike `change_cwd(path)` (which always performs a `chdir(path)`),
`save_cwd()` allows the code inside the `with` block to control this. The new
context manager just restores the original working directory afterward.

This is allows finer control of exception handling and robust environment
restoration across platforms in `FinderTests.test_permission_error_cwd()`.
  • Loading branch information
moreati committed Jun 21, 2024
1 parent 55596ae commit c48c021
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 6 deletions.
8 changes: 4 additions & 4 deletions Doc/reference/import.rst
Original file line number Diff line number Diff line change
Expand Up @@ -871,10 +871,10 @@ module.

The current working directory -- denoted by an empty string -- is handled
slightly differently from other entries on :data:`sys.path`. First, if the
current working directory is found to not exist, no value is stored in
:data:`sys.path_importer_cache`. Second, the value for the current working
directory is looked up fresh for each module lookup. Third, the path used for
:data:`sys.path_importer_cache` and returned by
current working directory cannot be determined or is found to not exist, no
value is stored in :data:`sys.path_importer_cache`. Second, the value for the
current working directory is looked up fresh for each module lookup. Third,
the path used for :data:`sys.path_importer_cache` and returned by
:meth:`importlib.machinery.PathFinder.find_spec` will be the actual current
working directory and not the empty string.

Expand Down
2 changes: 1 addition & 1 deletion Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,7 @@ def _path_importer_cache(cls, path):
if path == '':
try:
path = _os.getcwd()
except FileNotFoundError:
except (FileNotFoundError, PermissionError):
# Don't cache the failure as the cwd can easily change to
# a valid directory later on.
return None
Expand Down
56 changes: 55 additions & 1 deletion Lib/test/support/os_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,33 @@ def skip_unless_working_chmod(test):
return test if ok else unittest.skip(msg)(test)


@contextlib.contextmanager
def save_mode(path, *, quiet=False):
"""Context manager that restores the mode (permissions) of path on exit.
Arguments:
path: Path of the file to restore mode of.
quiet: if False (the default), the context manager raises an exception
on error. Otherwise, it issues only a warning and keeps the current
working directory the same.
"""
saved_mode = os.stat(path)
try:
yield
finally:
try:
os.chmod(path, saved_mode.st_mode)
except OSError as exc:
if not quiet:
raise
warnings.warn(f'tests may fail, unable to restore the mode of '
f'{path!r} to {saved_mode.st_mode}: {exc}',
RuntimeWarning, stacklevel=3)


# Check whether the current effective user has the capability to override
# DAC (discretionary access control). Typically user root is able to
# bypass file read, write, and execute permission checks. The capability
Expand Down Expand Up @@ -508,9 +535,36 @@ def temp_dir(path=None, quiet=False):
rmtree(path)


@contextlib.contextmanager
def save_cwd(*, quiet=False):
"""Return a context manager that restores the current working directory on
exit.
Arguments:
quiet: if False (the default), the context manager raises an exception
on error. Otherwise, it issues only a warning and keeps the current
working directory the same.
"""
saved_dir = os.getcwd()
try:
yield
finally:
try:
os.chdir(saved_dir)
except OSError as exc:
if not quiet:
raise
warnings.warn(f'tests may fail, unable to restore the current '
f'working directory to {saved_dir!r}: {exc}',
RuntimeWarning, stacklevel=3)


@contextlib.contextmanager
def change_cwd(path, quiet=False):
"""Return a context manager that changes the current working directory.
"""Return a context manager that changes the current working directory on
entry, and restores it on exit.
Arguments:
Expand Down
20 changes: 20 additions & 0 deletions Lib/test/test_importlib/import_/test_path.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from test.support import os_helper
from test.test_importlib import util

importlib = util.import_importlib('importlib')
Expand Down Expand Up @@ -153,6 +154,25 @@ def test_deleted_cwd(self):
# Do not want FileNotFoundError raised.
self.assertIsNone(self.machinery.PathFinder.find_spec('whatever'))

@os_helper.skip_unless_working_chmod
def test_permission_error_cwd(self):
# gh-115911
with (
os_helper.temp_dir() as new_dir,
os_helper.save_mode(new_dir),
os_helper.save_cwd(),
util.import_state(path=['']),
):
os.chdir(new_dir)
try:
os.chmod(new_dir, 0o000)
except OSError:
self.skipTest("platform does not allow "
"changing mode of the cwd")

# Do not want PermissionError raised.
self.assertIsNone(self.machinery.PathFinder.find_spec('whatever'))

def test_invalidate_caches_finders(self):
# Finders with an invalidate_caches() method have it called.
class FakeFinder:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
If the current working directory cannot be determined due to permissions,
then import will no longer raise :exc:`PermissionError`. Patch by Alex
Willmer.

0 comments on commit c48c021

Please sign in to comment.