Skip to content

Commit

Permalink
bpo-33721: Make some os.path functions and pathlib.Path methods be t…
Browse files Browse the repository at this point in the history
…olerant to invalid paths. (python#7695)

Such functions as os.path.exists(), os.path.lexists(), os.path.isdir(),
os.path.isfile(), os.path.islink(), and os.path.ismount() now return False
instead of raising ValueError or its subclasses UnicodeEncodeError
and UnicodeDecodeError for paths that contain characters or bytes
unrepresentative at the OS level.
  • Loading branch information
serhiy-storchaka authored Sep 18, 2018
1 parent 7bdf282 commit 0185f34
Show file tree
Hide file tree
Showing 15 changed files with 181 additions and 54 deletions.
8 changes: 8 additions & 0 deletions Doc/library/os.path.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ the :mod:`glob` module.)
* :mod:`macpath` for old-style MacOS paths


.. versionchanged:: 3.8

:func:`exists`, :func:`lexists`, :func:`isdir`, :func:`isfile`,
:func:`islink`, and :func:`ismount` now return ``False`` instead of
raising an exception for paths that contain characters or bytes
unrepresentable at the OS level.


.. function:: abspath(path)

Return a normalized absolutized version of the pathname *path*. On most
Expand Down
12 changes: 11 additions & 1 deletion Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,17 @@ Methods

Concrete paths provide the following methods in addition to pure paths
methods. Many of these methods can raise an :exc:`OSError` if a system
call fails (for example because the path doesn't exist):
call fails (for example because the path doesn't exist).

.. versionchanged:: 3.8

:meth:`~Path.exists()`, :meth:`~Path.is_dir()`, :meth:`~Path.is_file()`,
:meth:`~Path.is_mount()`, :meth:`~Path.is_symlink()`,
:meth:`~Path.is_block_device()`, :meth:`~Path.is_char_device()`,
:meth:`~Path.is_fifo()`, :meth:`~Path.is_socket()` now return ``False``
instead of raising an exception for paths that contain characters
unrepresentable at the OS level.


.. classmethod:: Path.cwd()

Expand Down
25 changes: 25 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,31 @@ New Modules
Improved Modules
================

os.path
-------

:mod:`os.path` functions that return a boolean result like
:func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`,
:func:`~os.path.isfile`, :func:`~os.path.islink`, and :func:`~os.path.ismount`
now return ``False`` instead of raising :exc:`ValueError` or its subclasses
:exc:`UnicodeEncodeError` and :exc:`UnicodeDecodeError` for paths that contain
characters or bytes unrepresentable at the OS level.
(Contributed by Serhiy Storchaka in :issue:`33721`.)

pathlib
-------

:mod:`pathlib.Path` methods that return a boolean result like
:meth:`~pathlib.Path.exists()`, :meth:`~pathlib.Path.is_dir()`,
:meth:`~pathlib.Path.is_file()`, :meth:`~pathlib.Path.is_mount()`,
:meth:`~pathlib.Path.is_symlink()`, :meth:`~pathlib.Path.is_block_device()`,
:meth:`~pathlib.Path.is_char_device()`, :meth:`~pathlib.Path.is_fifo()`,
:meth:`~pathlib.Path.is_socket()` now return ``False`` instead of raising
:exc:`ValueError` or its subclass :exc:`UnicodeEncodeError` for paths that
contain characters unrepresentable at the OS level.
(Contributed by Serhiy Storchaka in :issue:`33721`.)


Optimizations
=============

Expand Down
6 changes: 3 additions & 3 deletions Lib/genericpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def exists(path):
"""Test whether a path exists. Returns False for broken symbolic links"""
try:
os.stat(path)
except OSError:
except (OSError, ValueError):
return False
return True

Expand All @@ -28,7 +28,7 @@ def isfile(path):
"""Test whether a path is a regular file"""
try:
st = os.stat(path)
except OSError:
except (OSError, ValueError):
return False
return stat.S_ISREG(st.st_mode)

Expand All @@ -40,7 +40,7 @@ def isdir(s):
"""Return true if the pathname refers to an existing directory."""
try:
st = os.stat(s)
except OSError:
except (OSError, ValueError):
return False
return stat.S_ISDIR(st.st_mode)

Expand Down
2 changes: 1 addition & 1 deletion Lib/macpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def lexists(path):

try:
st = os.lstat(path)
except OSError:
except (OSError, ValueError):
return False
return True

Expand Down
6 changes: 3 additions & 3 deletions Lib/ntpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def islink(path):
"""
try:
st = os.lstat(path)
except (OSError, AttributeError):
except (OSError, ValueError, AttributeError):
return False
return stat.S_ISLNK(st.st_mode)

Expand All @@ -239,7 +239,7 @@ def lexists(path):
"""Test whether a path exists. Returns True for broken symbolic links"""
try:
st = os.lstat(path)
except OSError:
except (OSError, ValueError):
return False
return True

Expand Down Expand Up @@ -524,7 +524,7 @@ def abspath(path):
"""Return the absolute version of a path."""
try:
return _getfullpathname(path)
except OSError:
except (OSError, ValueError):
return _abspath_fallback(path)

# realpath is a no-op on systems without islink support
Expand Down
24 changes: 24 additions & 0 deletions Lib/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,9 @@ def exists(self):
if e.errno not in _IGNORED_ERROS:
raise
return False
except ValueError:
# Non-encodable path
return False
return True

def is_dir(self):
Expand All @@ -1345,6 +1348,9 @@ def is_dir(self):
# Path doesn't exist or is a broken symlink
# (see https://bitbucket.org/pitrou/pathlib/issue/12/)
return False
except ValueError:
# Non-encodable path
return False

def is_file(self):
"""
Expand All @@ -1359,6 +1365,9 @@ def is_file(self):
# Path doesn't exist or is a broken symlink
# (see https://bitbucket.org/pitrou/pathlib/issue/12/)
return False
except ValueError:
# Non-encodable path
return False

def is_mount(self):
"""
Expand Down Expand Up @@ -1392,6 +1401,9 @@ def is_symlink(self):
raise
# Path doesn't exist
return False
except ValueError:
# Non-encodable path
return False

def is_block_device(self):
"""
Expand All @@ -1405,6 +1417,9 @@ def is_block_device(self):
# Path doesn't exist or is a broken symlink
# (see https://bitbucket.org/pitrou/pathlib/issue/12/)
return False
except ValueError:
# Non-encodable path
return False

def is_char_device(self):
"""
Expand All @@ -1418,6 +1433,9 @@ def is_char_device(self):
# Path doesn't exist or is a broken symlink
# (see https://bitbucket.org/pitrou/pathlib/issue/12/)
return False
except ValueError:
# Non-encodable path
return False

def is_fifo(self):
"""
Expand All @@ -1431,6 +1449,9 @@ def is_fifo(self):
# Path doesn't exist or is a broken symlink
# (see https://bitbucket.org/pitrou/pathlib/issue/12/)
return False
except ValueError:
# Non-encodable path
return False

def is_socket(self):
"""
Expand All @@ -1444,6 +1465,9 @@ def is_socket(self):
# Path doesn't exist or is a broken symlink
# (see https://bitbucket.org/pitrou/pathlib/issue/12/)
return False
except ValueError:
# Non-encodable path
return False

def expanduser(self):
""" Return a new path with expanded ~ and ~user constructs
Expand Down
8 changes: 4 additions & 4 deletions Lib/posixpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def islink(path):
"""Test whether a path is a symbolic link"""
try:
st = os.lstat(path)
except (OSError, AttributeError):
except (OSError, ValueError, AttributeError):
return False
return stat.S_ISLNK(st.st_mode)

Expand All @@ -179,7 +179,7 @@ def lexists(path):
"""Test whether a path exists. Returns True for broken symbolic links"""
try:
os.lstat(path)
except OSError:
except (OSError, ValueError):
return False
return True

Expand All @@ -191,7 +191,7 @@ def ismount(path):
"""Test whether a path is a mount point"""
try:
s1 = os.lstat(path)
except OSError:
except (OSError, ValueError):
# It doesn't exist -- so not a mount point. :-)
return False
else:
Expand All @@ -206,7 +206,7 @@ def ismount(path):
parent = realpath(parent)
try:
s2 = os.lstat(parent)
except OSError:
except (OSError, ValueError):
return False

dev1 = s1.st_dev
Expand Down
38 changes: 30 additions & 8 deletions Lib/test/test_genericpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,20 @@ def test_exists(self):
self.assertIs(self.pathmodule.exists(filename), True)
self.assertIs(self.pathmodule.exists(bfilename), True)

self.assertIs(self.pathmodule.exists(filename + '\udfff'), False)
self.assertIs(self.pathmodule.exists(bfilename + b'\xff'), False)
self.assertIs(self.pathmodule.exists(filename + '\x00'), False)
self.assertIs(self.pathmodule.exists(bfilename + b'\x00'), False)

if self.pathmodule is not genericpath:
self.assertIs(self.pathmodule.lexists(filename), True)
self.assertIs(self.pathmodule.lexists(bfilename), True)

self.assertIs(self.pathmodule.lexists(filename + '\udfff'), False)
self.assertIs(self.pathmodule.lexists(bfilename + b'\xff'), False)
self.assertIs(self.pathmodule.lexists(filename + '\x00'), False)
self.assertIs(self.pathmodule.lexists(bfilename + b'\x00'), False)

@unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()")
def test_exists_fd(self):
r, w = os.pipe()
Expand All @@ -158,6 +168,11 @@ def test_isdir(self):
self.assertIs(self.pathmodule.isdir(filename), False)
self.assertIs(self.pathmodule.isdir(bfilename), False)

self.assertIs(self.pathmodule.isdir(filename + '\udfff'), False)
self.assertIs(self.pathmodule.isdir(bfilename + b'\xff'), False)
self.assertIs(self.pathmodule.isdir(filename + '\x00'), False)
self.assertIs(self.pathmodule.isdir(bfilename + b'\x00'), False)

try:
create_file(filename)
self.assertIs(self.pathmodule.isdir(filename), False)
Expand All @@ -178,6 +193,11 @@ def test_isfile(self):
self.assertIs(self.pathmodule.isfile(filename), False)
self.assertIs(self.pathmodule.isfile(bfilename), False)

self.assertIs(self.pathmodule.isfile(filename + '\udfff'), False)
self.assertIs(self.pathmodule.isfile(bfilename + b'\xff'), False)
self.assertIs(self.pathmodule.isfile(filename + '\x00'), False)
self.assertIs(self.pathmodule.isfile(bfilename + b'\x00'), False)

try:
create_file(filename)
self.assertIs(self.pathmodule.isfile(filename), True)
Expand Down Expand Up @@ -298,18 +318,20 @@ def test_invalid_paths(self):
continue
func = getattr(self.pathmodule, attr)
with self.subTest(attr=attr):
try:
if attr in ('exists', 'isdir', 'isfile'):
func('/tmp\udfffabcds')
except (OSError, UnicodeEncodeError):
pass
try:
func(b'/tmp\xffabcds')
except (OSError, UnicodeDecodeError):
pass
with self.assertRaisesRegex(ValueError, 'embedded null'):
func('/tmp\x00abcds')
with self.assertRaisesRegex(ValueError, 'embedded null'):
func(b'/tmp\x00abcds')
else:
with self.assertRaises((OSError, UnicodeEncodeError)):
func('/tmp\udfffabcds')
with self.assertRaises((OSError, UnicodeDecodeError)):
func(b'/tmp\xffabcds')
with self.assertRaisesRegex(ValueError, 'embedded null'):
func('/tmp\x00abcds')
with self.assertRaisesRegex(ValueError, 'embedded null'):
func(b'/tmp\x00abcds')

# Following TestCase is not supposed to be run from test_genericpath.
# It is inherited by other test modules (macpath, ntpath, posixpath).
Expand Down
Loading

0 comments on commit 0185f34

Please sign in to comment.