From 2240ac1eae2dd8674748239af9c61e5ab4faeb2c Mon Sep 17 00:00:00 2001 From: Richard Oudkerk Date: Fri, 6 Jul 2012 12:05:32 +0100 Subject: Issue #15261: Stop os.stat(fd) crashing on Windows when fd not open. --- Doc/library/os.path.rst | 11 ++++++++--- Lib/test/test_genericpath.py | 10 ++++++++++ Lib/test/test_os.py | 13 +++++++++++++ Misc/NEWS | 2 ++ Modules/posixmodule.c | 7 ++++--- 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst index 7821628..20a84b6 100644 --- a/Doc/library/os.path.rst +++ b/Doc/library/os.path.rst @@ -70,11 +70,16 @@ applications should use string objects to access all files. .. function:: exists(path) - Return ``True`` if *path* refers to an existing path. Returns ``False`` for - broken symbolic links. On some platforms, this function may return ``False`` if - permission is not granted to execute :func:`os.stat` on the requested file, even + Return ``True`` if *path* refers to an existing path or an open + file descriptor. Returns ``False`` for broken symbolic links. On + some platforms, this function may return ``False`` if permission is + not granted to execute :func:`os.stat` on the requested file, even if the *path* physically exists. + .. versionchanged:: 3.3 + *path* can now be an integer: ``True`` is returned if it is an + open file descriptor, ``False`` otherwise. + .. function:: lexists(path) diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py index ebb8396..fc3d44c 100644 --- a/Lib/test/test_genericpath.py +++ b/Lib/test/test_genericpath.py @@ -146,6 +146,16 @@ class GenericTest(unittest.TestCase): f.close() support.unlink(support.TESTFN) + @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") + def test_exists_fd(self): + r, w = os.pipe() + try: + self.assertTrue(self.pathmodule.exists(r)) + finally: + os.close(r) + os.close(w) + self.assertFalse(self.pathmodule.exists(r)) + def test_isdir(self): self.assertIs(self.pathmodule.isdir(support.TESTFN), False) f = open(support.TESTFN, "wb") diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 57de993..7c73f1e 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -473,6 +473,19 @@ class StatAttributeTests(unittest.TestCase): return self.fail("Could not stat pagefile.sys") + @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") + def test_15261(self): + # Verify that stat'ing a closed fd does not cause crash + r, w = os.pipe() + try: + os.stat(r) # should not raise error + finally: + os.close(r) + os.close(w) + with self.assertRaises(OSError) as ctx: + os.stat(r) + self.assertEqual(ctx.exception.errno, errno.EBADF) + from test import mapping_tests class EnvironTests(mapping_tests.BasicTestMappingProtocol): diff --git a/Misc/NEWS b/Misc/NEWS index a78f72b..19488dc 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -23,6 +23,8 @@ Core and Builtins Library ------- +- Issue #15261: Stop os.stat(fd) crashing on Windows when fd not open. + - Issue #15166: Implement imp.get_tag() using sys.implementation.cache_tag. - Issue #15210: Catch KeyError when imprortlib.__init__ can't find diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 1bd5e97..b99a5fe 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1829,7 +1829,10 @@ win32_fstat(int file_number, struct win32_stat *result) HANDLE h; int type; - h = (HANDLE)_get_osfhandle(file_number); + if (!_PyVerify_fd(file_number)) + h = INVALID_HANDLE_VALUE; + else + h = (HANDLE)_get_osfhandle(file_number); /* Protocol violation: we explicitly clear errno, instead of setting it to a POSIX error. Callers should use GetLastError. */ @@ -8244,8 +8247,6 @@ posix_fstat(PyObject *self, PyObject *args) /* on OpenVMS we must ensure that all bytes are written to the file */ fsync(fd); #endif - if (!_PyVerify_fd(fd)) - return posix_error(); Py_BEGIN_ALLOW_THREADS res = FSTAT(fd, &st); Py_END_ALLOW_THREADS -- cgit v0.12