From 652fbf88c4c422ff17fefd4dcb5e06b5c0e26e74 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 5 Feb 2024 22:51:11 +0200 Subject: [PATCH] gh-82626: Emit a warning when bool is used as a file descriptor (GH-111275) --- Doc/whatsnew/3.13.rst | 5 +++++ Lib/_pyio.py | 5 +++++ Lib/test/test_fileio.py | 8 ++++++++ Lib/test/test_genericpath.py | 6 ++++++ Lib/test/test_os.py | 14 ++++++++++++++ Lib/test/test_posix.py | 7 +++++++ .../2023-10-24-19-19-54.gh-issue-82626._hfLRf.rst | 2 ++ Modules/_io/fileio.c | 7 +++++++ Modules/faulthandler.c | 7 +++++++ Modules/posixmodule.c | 7 +++++++ Objects/fileobject.c | 7 +++++++ 11 files changed, 75 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2023-10-24-19-19-54.gh-issue-82626._hfLRf.rst diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 0770e28d230..9bac36ba0bf 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -145,6 +145,11 @@ Other Language Changes is rejected when the global is used in the :keyword:`else` block. (Contributed by Irit Katriel in :gh:`111123`.) +* Many functions now emit a warning if a boolean value is passed as + a file descriptor argument. + This can help catch some errors earlier. + (Contributed by Serhiy Storchaka in :gh:`82626`.) + * Added a new environment variable :envvar:`PYTHON_FROZEN_MODULES`. It determines whether or not frozen modules are ignored by the import machinery, equivalent of the :option:`-X frozen_modules <-X>` command-line option. diff --git a/Lib/_pyio.py b/Lib/_pyio.py index df2c29bfa9c..8a0d0dc4b1a 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -1495,6 +1495,11 @@ class FileIO(RawIOBase): if isinstance(file, float): raise TypeError('integer argument expected, got float') if isinstance(file, int): + if isinstance(file, bool): + import warnings + warnings.warn("bool is used as a file descriptor", + RuntimeWarning, stacklevel=2) + file = int(file) fd = file if fd < 0: raise ValueError('negative file descriptor') diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py index 06d9b454add..06d5a8abf32 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -484,6 +484,14 @@ class OtherFileTests: import msvcrt self.assertRaises(OSError, msvcrt.get_osfhandle, make_bad_fd()) + def testBooleanFd(self): + for fd in False, True: + with self.assertWarnsRegex(RuntimeWarning, + 'bool is used as a file descriptor') as cm: + f = self.FileIO(fd, closefd=False) + f.close() + self.assertEqual(cm.filename, __file__) + def testBadModeArgument(self): # verify that we get a sensible error message for bad mode argument bad_mode = "qwerty" diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py index b77cd4c67d6..f407ee3caf1 100644 --- a/Lib/test/test_genericpath.py +++ b/Lib/test/test_genericpath.py @@ -165,6 +165,12 @@ class GenericTest: os.close(w) self.assertFalse(self.pathmodule.exists(r)) + def test_exists_bool(self): + for fd in False, True: + with self.assertWarnsRegex(RuntimeWarning, + 'bool is used as a file descriptor'): + self.pathmodule.exists(fd) + def test_isdir(self): filename = os_helper.TESTFN bfilename = os.fsencode(filename) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 86af1a8ed8e..2c8823ae47c 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2195,12 +2195,15 @@ class Win32ErrorTests(unittest.TestCase): class TestInvalidFD(unittest.TestCase): singles = ["fchdir", "dup", "fdatasync", "fstat", "fstatvfs", "fsync", "tcgetpgrp", "ttyname"] + singles_fildes = {"fchdir", "fdatasync", "fsync"} #singles.append("close") #We omit close because it doesn't raise an exception on some platforms def get_single(f): def helper(self): if hasattr(os, f): self.check(getattr(os, f)) + if f in self.singles_fildes: + self.check_bool(getattr(os, f)) return helper for f in singles: locals()["test_"+f] = get_single(f) @@ -2214,8 +2217,16 @@ class TestInvalidFD(unittest.TestCase): self.fail("%r didn't raise an OSError with a bad file descriptor" % f) + def check_bool(self, f, *args, **kwargs): + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + for fd in False, True: + with self.assertRaises(RuntimeWarning): + f(fd, *args, **kwargs) + def test_fdopen(self): self.check(os.fdopen, encoding="utf-8") + self.check_bool(os.fdopen, encoding="utf-8") @unittest.skipUnless(hasattr(os, 'isatty'), 'test needs os.isatty()') def test_isatty(self): @@ -2277,11 +2288,14 @@ class TestInvalidFD(unittest.TestCase): def test_fpathconf(self): self.check(os.pathconf, "PC_NAME_MAX") self.check(os.fpathconf, "PC_NAME_MAX") + self.check_bool(os.pathconf, "PC_NAME_MAX") + self.check_bool(os.fpathconf, "PC_NAME_MAX") @unittest.skipUnless(hasattr(os, 'ftruncate'), 'test needs os.ftruncate()') def test_ftruncate(self): self.check(os.truncate, 0) self.check(os.ftruncate, 0) + self.check_bool(os.truncate, 0) @unittest.skipUnless(hasattr(os, 'lseek'), 'test needs os.lseek()') def test_lseek(self): diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 72e348fbbdc..a45f620e18d 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -1514,6 +1514,13 @@ class TestPosixDirFd(unittest.TestCase): self.assertRaises(OverflowError, posix.stat, name, dir_fd=10**20) + for fd in False, True: + with self.assertWarnsRegex(RuntimeWarning, + 'bool is used as a file descriptor') as cm: + with self.assertRaises(OSError): + posix.stat('nonexisting', dir_fd=fd) + self.assertEqual(cm.filename, __file__) + @unittest.skipUnless(os.utime in os.supports_dir_fd, "test needs dir_fd support in os.utime()") def test_utime_dir_fd(self): with self.prepare_file() as (dir_fd, name, fullname): diff --git a/Misc/NEWS.d/next/Library/2023-10-24-19-19-54.gh-issue-82626._hfLRf.rst b/Misc/NEWS.d/next/Library/2023-10-24-19-19-54.gh-issue-82626._hfLRf.rst new file mode 100644 index 00000000000..92a66b5bf0f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-10-24-19-19-54.gh-issue-82626._hfLRf.rst @@ -0,0 +1,2 @@ +Many functions now emit a warning if a boolean value is passed as a file +descriptor argument. diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 9cf268ca0b2..6bb156e41fe 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -269,6 +269,13 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode, self->fd = -1; } + if (PyBool_Check(nameobj)) { + if (PyErr_WarnEx(PyExc_RuntimeWarning, + "bool is used as a file descriptor", 1)) + { + return -1; + } + } fd = PyLong_AsInt(nameobj); if (fd < 0) { if (!PyErr_Occurred()) { diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index a2e3c2300b3..95d646c9c65 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -119,6 +119,13 @@ faulthandler_get_fileno(PyObject **file_ptr) } } else if (PyLong_Check(file)) { + if (PyBool_Check(file)) { + if (PyErr_WarnEx(PyExc_RuntimeWarning, + "bool is used as a file descriptor", 1)) + { + return -1; + } + } fd = PyLong_AsInt(file); if (fd == -1 && PyErr_Occurred()) return -1; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 40ff131b119..22891135bde 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -969,6 +969,13 @@ _fd_converter(PyObject *o, int *p) int overflow; long long_value; + if (PyBool_Check(o)) { + if (PyErr_WarnEx(PyExc_RuntimeWarning, + "bool is used as a file descriptor", 1)) + { + return 0; + } + } PyObject *index = _PyNumber_Index(o); if (index == NULL) { return 0; diff --git a/Objects/fileobject.c b/Objects/fileobject.c index 5522eba34ea..e30ab952dff 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -174,6 +174,13 @@ PyObject_AsFileDescriptor(PyObject *o) PyObject *meth; if (PyLong_Check(o)) { + if (PyBool_Check(o)) { + if (PyErr_WarnEx(PyExc_RuntimeWarning, + "bool is used as a file descriptor", 1)) + { + return -1; + } + } fd = PyLong_AsInt(o); } else if (PyObject_GetOptionalAttr(o, &_Py_ID(fileno), &meth) < 0) {