gh-82626: Emit a warning when bool is used as a file descriptor (GH-111275)

This commit is contained in:
Serhiy Storchaka 2024-02-05 22:51:11 +02:00 committed by GitHub
parent 09096a1647
commit 652fbf88c4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 75 additions and 0 deletions

View File

@ -145,6 +145,11 @@ Other Language Changes
is rejected when the global is used in the :keyword:`else` block. is rejected when the global is used in the :keyword:`else` block.
(Contributed by Irit Katriel in :gh:`111123`.) (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 * Added a new environment variable :envvar:`PYTHON_FROZEN_MODULES`. It
determines whether or not frozen modules are ignored by the import machinery, determines whether or not frozen modules are ignored by the import machinery,
equivalent of the :option:`-X frozen_modules <-X>` command-line option. equivalent of the :option:`-X frozen_modules <-X>` command-line option.

View File

@ -1495,6 +1495,11 @@ class FileIO(RawIOBase):
if isinstance(file, float): if isinstance(file, float):
raise TypeError('integer argument expected, got float') raise TypeError('integer argument expected, got float')
if isinstance(file, int): 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 fd = file
if fd < 0: if fd < 0:
raise ValueError('negative file descriptor') raise ValueError('negative file descriptor')

View File

@ -484,6 +484,14 @@ class OtherFileTests:
import msvcrt import msvcrt
self.assertRaises(OSError, msvcrt.get_osfhandle, make_bad_fd()) 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): def testBadModeArgument(self):
# verify that we get a sensible error message for bad mode argument # verify that we get a sensible error message for bad mode argument
bad_mode = "qwerty" bad_mode = "qwerty"

View File

@ -165,6 +165,12 @@ class GenericTest:
os.close(w) os.close(w)
self.assertFalse(self.pathmodule.exists(r)) 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): def test_isdir(self):
filename = os_helper.TESTFN filename = os_helper.TESTFN
bfilename = os.fsencode(filename) bfilename = os.fsencode(filename)

View File

@ -2195,12 +2195,15 @@ class Win32ErrorTests(unittest.TestCase):
class TestInvalidFD(unittest.TestCase): class TestInvalidFD(unittest.TestCase):
singles = ["fchdir", "dup", "fdatasync", "fstat", singles = ["fchdir", "dup", "fdatasync", "fstat",
"fstatvfs", "fsync", "tcgetpgrp", "ttyname"] "fstatvfs", "fsync", "tcgetpgrp", "ttyname"]
singles_fildes = {"fchdir", "fdatasync", "fsync"}
#singles.append("close") #singles.append("close")
#We omit close because it doesn't raise an exception on some platforms #We omit close because it doesn't raise an exception on some platforms
def get_single(f): def get_single(f):
def helper(self): def helper(self):
if hasattr(os, f): if hasattr(os, f):
self.check(getattr(os, f)) self.check(getattr(os, f))
if f in self.singles_fildes:
self.check_bool(getattr(os, f))
return helper return helper
for f in singles: for f in singles:
locals()["test_"+f] = get_single(f) 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" self.fail("%r didn't raise an OSError with a bad file descriptor"
% f) % 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): def test_fdopen(self):
self.check(os.fdopen, encoding="utf-8") self.check(os.fdopen, encoding="utf-8")
self.check_bool(os.fdopen, encoding="utf-8")
@unittest.skipUnless(hasattr(os, 'isatty'), 'test needs os.isatty()') @unittest.skipUnless(hasattr(os, 'isatty'), 'test needs os.isatty()')
def test_isatty(self): def test_isatty(self):
@ -2277,11 +2288,14 @@ class TestInvalidFD(unittest.TestCase):
def test_fpathconf(self): def test_fpathconf(self):
self.check(os.pathconf, "PC_NAME_MAX") self.check(os.pathconf, "PC_NAME_MAX")
self.check(os.fpathconf, "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()') @unittest.skipUnless(hasattr(os, 'ftruncate'), 'test needs os.ftruncate()')
def test_ftruncate(self): def test_ftruncate(self):
self.check(os.truncate, 0) self.check(os.truncate, 0)
self.check(os.ftruncate, 0) self.check(os.ftruncate, 0)
self.check_bool(os.truncate, 0)
@unittest.skipUnless(hasattr(os, 'lseek'), 'test needs os.lseek()') @unittest.skipUnless(hasattr(os, 'lseek'), 'test needs os.lseek()')
def test_lseek(self): def test_lseek(self):

View File

@ -1514,6 +1514,13 @@ class TestPosixDirFd(unittest.TestCase):
self.assertRaises(OverflowError, self.assertRaises(OverflowError,
posix.stat, name, dir_fd=10**20) 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()") @unittest.skipUnless(os.utime in os.supports_dir_fd, "test needs dir_fd support in os.utime()")
def test_utime_dir_fd(self): def test_utime_dir_fd(self):
with self.prepare_file() as (dir_fd, name, fullname): with self.prepare_file() as (dir_fd, name, fullname):

View File

@ -0,0 +1,2 @@
Many functions now emit a warning if a boolean value is passed as a file
descriptor argument.

View File

@ -269,6 +269,13 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
self->fd = -1; 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); fd = PyLong_AsInt(nameobj);
if (fd < 0) { if (fd < 0) {
if (!PyErr_Occurred()) { if (!PyErr_Occurred()) {

View File

@ -119,6 +119,13 @@ faulthandler_get_fileno(PyObject **file_ptr)
} }
} }
else if (PyLong_Check(file)) { 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); fd = PyLong_AsInt(file);
if (fd == -1 && PyErr_Occurred()) if (fd == -1 && PyErr_Occurred())
return -1; return -1;

View File

@ -969,6 +969,13 @@ _fd_converter(PyObject *o, int *p)
int overflow; int overflow;
long long_value; 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); PyObject *index = _PyNumber_Index(o);
if (index == NULL) { if (index == NULL) {
return 0; return 0;

View File

@ -174,6 +174,13 @@ PyObject_AsFileDescriptor(PyObject *o)
PyObject *meth; PyObject *meth;
if (PyLong_Check(o)) { 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); fd = PyLong_AsInt(o);
} }
else if (PyObject_GetOptionalAttr(o, &_Py_ID(fileno), &meth) < 0) { else if (PyObject_GetOptionalAttr(o, &_Py_ID(fileno), &meth) < 0) {