Skip to content

Commit 0fafcd4

Browse files
serhiy-storchakaserhiy-storchaka
committed
bpo-33721: Make some os.path functions be tolerant to invalid paths.
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 unrepresntative at the OS level.
1 parent 17a0088 commit 0fafcd4

10 files changed

Lines changed: 88 additions & 46 deletions

File tree

‎Doc/whatsnew/3.8.rst‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,18 @@ New Modules
9090
Improved Modules
9191
================
9292

93+
os.path
94+
-------
95+
96+
:mod:`os.path` functions that return the boolean result like
97+
:func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`,
98+
:func:`~os.path.isfile`, :func:`~os.path.islink`, and :func:`~os.path.ismount`
99+
now return ``False`` instead of raising :exc:`ValueError` or its subclasses
100+
:exc:`UnicodeEncodeError` and :exc:`UnicodeDecodeError` for paths that contain
101+
characters or bytes unrepresntative at the OS level.
102+
(Contributed by Serhiy Storchaka in :issue:`33721`.)
103+
104+
93105
Optimizations
94106
=============
95107

‎Lib/genericpath.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def exists(path):
1717
"""Test whether a path exists. Returns False for broken symbolic links"""
1818
try:
1919
os.stat(path)
20-
except OSError:
20+
except (OSError, ValueError):
2121
return False
2222
return True
2323

@@ -28,7 +28,7 @@ def isfile(path):
2828
"""Test whether a path is a regular file"""
2929
try:
3030
st = os.stat(path)
31-
except OSError:
31+
except (OSError, ValueError):
3232
return False
3333
return stat.S_ISREG(st.st_mode)
3434

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

‎Lib/macpath.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def lexists(path):
138138

139139
try:
140140
st = os.lstat(path)
141-
except OSError:
141+
except (OSError, ValueError):
142142
return False
143143
return True
144144

‎Lib/ntpath.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ def islink(path):
229229
"""
230230
try:
231231
st = os.lstat(path)
232-
except (OSError, AttributeError):
232+
except (OSError, ValueError, AttributeError):
233233
return False
234234
return stat.S_ISLNK(st.st_mode)
235235

@@ -239,7 +239,7 @@ def lexists(path):
239239
"""Test whether a path exists. Returns True for broken symbolic links"""
240240
try:
241241
st = os.lstat(path)
242-
except OSError:
242+
except (OSError, ValueError):
243243
return False
244244
return True
245245

@@ -521,7 +521,7 @@ def abspath(path):
521521
path = os.fspath(path)
522522
try:
523523
path = _getfullpathname(path)
524-
except OSError:
524+
except (OSError, ValueError):
525525
pass # Bad path - return unchanged.
526526
elif isinstance(path, bytes):
527527
path = os.getcwdb()

‎Lib/posixpath.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ def islink(path):
169169
"""Test whether a path is a symbolic link"""
170170
try:
171171
st = os.lstat(path)
172-
except (OSError, AttributeError):
172+
except (OSError, ValueError, AttributeError):
173173
return False
174174
return stat.S_ISLNK(st.st_mode)
175175

@@ -179,7 +179,7 @@ def lexists(path):
179179
"""Test whether a path exists. Returns True for broken symbolic links"""
180180
try:
181181
os.lstat(path)
182-
except OSError:
182+
except (OSError, ValueError):
183183
return False
184184
return True
185185

@@ -191,7 +191,7 @@ def ismount(path):
191191
"""Test whether a path is a mount point"""
192192
try:
193193
s1 = os.lstat(path)
194-
except OSError:
194+
except (OSError, ValueError):
195195
# It doesn't exist -- so not a mount point. :-)
196196
return False
197197
else:
@@ -206,7 +206,7 @@ def ismount(path):
206206
parent = realpath(parent)
207207
try:
208208
s2 = os.lstat(parent)
209-
except OSError:
209+
except (OSError, ValueError):
210210
return False
211211

212212
dev1 = s1.st_dev

‎Lib/test/test_genericpath.py‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,20 @@ def test_exists(self):
138138
self.assertIs(self.pathmodule.exists(filename), True)
139139
self.assertIs(self.pathmodule.exists(bfilename), True)
140140

141+
self.assertIs(self.pathmodule.exists(filename + '\udfff'), False)
142+
self.assertIs(self.pathmodule.exists(bfilename + b'\xff'), False)
143+
self.assertIs(self.pathmodule.exists(filename + '\x00'), False)
144+
self.assertIs(self.pathmodule.exists(bfilename + b'\x00'), False)
145+
141146
if self.pathmodule is not genericpath:
142147
self.assertIs(self.pathmodule.lexists(filename), True)
143148
self.assertIs(self.pathmodule.lexists(bfilename), True)
144149

150+
self.assertIs(self.pathmodule.lexists(filename + '\udfff'), False)
151+
self.assertIs(self.pathmodule.lexists(bfilename + b'\xff'), False)
152+
self.assertIs(self.pathmodule.lexists(filename + '\x00'), False)
153+
self.assertIs(self.pathmodule.lexists(bfilename + b'\x00'), False)
154+
145155
@unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()")
146156
def test_exists_fd(self):
147157
r, w = os.pipe()
@@ -158,6 +168,11 @@ def test_isdir(self):
158168
self.assertIs(self.pathmodule.isdir(filename), False)
159169
self.assertIs(self.pathmodule.isdir(bfilename), False)
160170

171+
self.assertIs(self.pathmodule.isdir(filename + '\udfff'), False)
172+
self.assertIs(self.pathmodule.isdir(bfilename + b'\xff'), False)
173+
self.assertIs(self.pathmodule.isdir(filename + '\x00'), False)
174+
self.assertIs(self.pathmodule.isdir(bfilename + b'\x00'), False)
175+
161176
try:
162177
create_file(filename)
163178
self.assertIs(self.pathmodule.isdir(filename), False)
@@ -178,6 +193,11 @@ def test_isfile(self):
178193
self.assertIs(self.pathmodule.isfile(filename), False)
179194
self.assertIs(self.pathmodule.isfile(bfilename), False)
180195

196+
self.assertIs(self.pathmodule.isfile(filename + '\udfff'), False)
197+
self.assertIs(self.pathmodule.isfile(bfilename + b'\xff'), False)
198+
self.assertIs(self.pathmodule.isfile(filename + '\x00'), False)
199+
self.assertIs(self.pathmodule.isfile(bfilename + b'\x00'), False)
200+
181201
try:
182202
create_file(filename)
183203
self.assertIs(self.pathmodule.isfile(filename), True)
@@ -294,22 +314,24 @@ class TestGenericTest(GenericTest, unittest.TestCase):
294314
def test_invalid_paths(self):
295315
for attr in GenericTest.common_attributes:
296316
# os.path.commonprefix doesn't raise ValueError
297-
if attr == 'commonprefix':
317+
if attr in 'commonprefix':
298318
continue
299319
func = getattr(self.pathmodule, attr)
300320
with self.subTest(attr=attr):
301-
try:
321+
if attr in ('exists', 'isdir', 'isfile'):
302322
func('/tmp\udfffabcds')
303-
except (OSError, UnicodeEncodeError):
304-
pass
305-
try:
306323
func(b'/tmp\xffabcds')
307-
except (OSError, UnicodeDecodeError):
308-
pass
309-
with self.assertRaisesRegex(ValueError, 'embedded null'):
310324
func('/tmp\x00abcds')
311-
with self.assertRaisesRegex(ValueError, 'embedded null'):
312325
func(b'/tmp\x00abcds')
326+
else:
327+
with self.assertRaises((OSError, UnicodeEncodeError)):
328+
func('/tmp\udfffabcds')
329+
with self.assertRaises((OSError, UnicodeDecodeError)):
330+
func(b'/tmp\xffabcds')
331+
with self.assertRaisesRegex(ValueError, 'embedded null'):
332+
func('/tmp\x00abcds')
333+
with self.assertRaisesRegex(ValueError, 'embedded null'):
334+
func(b'/tmp\x00abcds')
313335

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

‎Lib/test/test_posixpath.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,11 @@ def test_dirname(self):
153153
def test_islink(self):
154154
self.assertIs(posixpath.islink(support.TESTFN + "1"), False)
155155
self.assertIs(posixpath.lexists(support.TESTFN + "2"), False)
156+
156157
with open(support.TESTFN + "1", "wb") as f:
157158
f.write(b"foo")
158159
self.assertIs(posixpath.islink(support.TESTFN + "1"), False)
160+
159161
if support.can_symlink():
160162
os.symlink(support.TESTFN + "1", support.TESTFN + "2")
161163
self.assertIs(posixpath.islink(support.TESTFN + "2"), True)
@@ -164,6 +166,11 @@ def test_islink(self):
164166
self.assertIs(posixpath.exists(support.TESTFN + "2"), False)
165167
self.assertIs(posixpath.lexists(support.TESTFN + "2"), True)
166168

169+
self.assertIs(posixpath.islink(support.TESTFN + "\udfff"), False)
170+
self.assertIs(posixpath.islink(os.fsencode(support.TESTFN) + b"\xff"), False)
171+
self.assertIs(posixpath.islink(support.TESTFN + "\x00"), False)
172+
self.assertIs(posixpath.islink(os.fsencode(support.TESTFN) + b"\x00"), False)
173+
167174
def test_ismount(self):
168175
self.assertIs(posixpath.ismount("/"), True)
169176
self.assertIs(posixpath.ismount(b"/"), True)
@@ -177,6 +184,11 @@ def test_ismount_non_existent(self):
177184
finally:
178185
safe_rmdir(ABSTFN)
179186

187+
self.assertIs(posixpath.ismount('/\udfff'), False)
188+
self.assertIs(posixpath.ismount(b'/\xff'), False)
189+
self.assertIs(posixpath.ismount('/\x00'), False)
190+
self.assertIs(posixpath.ismount(b'/\x00'), False)
191+
180192
@unittest.skipUnless(support.can_symlink(),
181193
"Test requires symlink support")
182194
def test_ismount_symlinks(self):

‎Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
:mod:`os.path` functions that return the boolean result like
2+
:func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`,
3+
:func:`~os.path.isfile`, :func:`~os.path.islink`, and
4+
:func:`~os.path.ismount` now return ``False`` instead of raising
5+
:exc:`ValueError` or its subclasses :exc:`UnicodeEncodeError` and
6+
:exc:`UnicodeDecodeError` for paths that contain characters or bytes
7+
unrepresntative at the OS level.

‎Modules/clinic/posixmodule.c.h‎

Lines changed: 1 addition & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Modules/posixmodule.c‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3854,22 +3854,32 @@ os__getfinalpathname_impl(PyObject *module, path_t *path)
38543854
/*[clinic input]
38553855
os._isdir
38563856
3857-
path: path_t
3857+
path as arg: object
38583858
/
38593859
38603860
Return true if the pathname refers to an existing directory.
38613861
[clinic start generated code]*/
38623862

38633863
static PyObject *
3864-
os__isdir_impl(PyObject *module, path_t *path)
3865-
/*[clinic end generated code: output=75f56f32720836cb input=5e0800149c0ad95f]*/
3864+
os__isdir(PyObject *module, PyObject *arg)
3865+
/*[clinic end generated code: output=404f334d85d4bf25 input=36cb6785874d479e]*/
38663866
{
38673867
DWORD attributes;
3868+
path_t path = PATH_T_INITIALIZE("_isdir", "path", 0, 0);
3869+
3870+
if (!path_converter(arg, &path)) {
3871+
if (PyErr_ExceptionMatches(PyExc_ValueError)) {
3872+
PyErr_Clear();
3873+
Py_RETURN_FALSE;
3874+
}
3875+
return NULL;
3876+
}
38683877

38693878
Py_BEGIN_ALLOW_THREADS
3870-
attributes = GetFileAttributesW(path->wide);
3879+
attributes = GetFileAttributesW(path.wide);
38713880
Py_END_ALLOW_THREADS
38723881

3882+
path_cleanup(&path);
38733883
if (attributes == INVALID_FILE_ATTRIBUTES)
38743884
Py_RETURN_FALSE;
38753885

0 commit comments

Comments
 (0)