From 59ed780117706eb9b695773a98ac165ed774f394 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Wed, 4 Sep 2019 19:11:51 -0600 Subject: [PATCH 1/9] bpo-38031: Fix a possible assertion failure in _io.FileIO() Use PyErr_Fetch() / _PyErr_ChainExceptions() when calling internal_close() on error. The opener may return an invalid fd, which will cause the close() call in internal_close() to fail. --- Lib/test/test_io.py | 7 +++++++ .../2019-09-04-19-09-49.bpo-38031.Yq4L72.rst | 2 ++ Modules/_io/fileio.c | 6 +++++- 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-09-04-19-09-49.bpo-38031.Yq4L72.rst diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 1fe1cba5167fc6..7ed1858c319ba3 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -875,6 +875,13 @@ def badopener(fname, flags): open('non-existent', 'r', opener=badopener) self.assertEqual(str(cm.exception), 'opener returned -2') + def test_opener_invalid_fd(self): + self.open(support.TESTFN, 'w').close() + fd = os.open(support.TESTFN, os.O_RDONLY) + os.close(fd) + with self.assertRaisesRegex(OSError, 'Bad file descriptor'): + self.open('foo', opener=lambda name, flags: fd) + def test_fileio_closefd(self): # Issue #4841 with self.open(__file__, 'rb') as f1, \ diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-09-04-19-09-49.bpo-38031.Yq4L72.rst b/Misc/NEWS.d/next/Core and Builtins/2019-09-04-19-09-49.bpo-38031.Yq4L72.rst new file mode 100644 index 00000000000000..b5964375962f66 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-09-04-19-09-49.bpo-38031.Yq4L72.rst @@ -0,0 +1,2 @@ +Fix a possible assertion failure in :class:`io.FileIO` when the opener +returns an invalid file descriptor. diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index e4cbbfa9bb8cd4..ceaa1b054fd63f 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -492,8 +492,12 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode, ret = -1; if (!fd_is_own) self->fd = -1; - if (self->fd >= 0) + if (self->fd >= 0) { + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); internal_close(self); + _PyErr_ChainExceptions(exc, val, tb); + } done: Py_CLEAR(stringobj); From 6c0deba13eefd0d524a13686a2557928c4c1224e Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 23 Nov 2022 22:39:19 +0000 Subject: [PATCH 2/9] Apply suggestions from code review --- Lib/test/test_io.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 7ed1858c319ba3..2ac2978a9aad69 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -879,7 +879,9 @@ def test_opener_invalid_fd(self): self.open(support.TESTFN, 'w').close() fd = os.open(support.TESTFN, os.O_RDONLY) os.close(fd) - with self.assertRaisesRegex(OSError, 'Bad file descriptor'): + # fd is now an invalid file descriptor + with self.assertRaises(OSError): + self.assertEqual(exc.errno, errno.EBADF) self.open('foo', opener=lambda name, flags: fd) def test_fileio_closefd(self): From 28c60583bc04e5ccd8dbbd95d99c347b06d4f7be Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 23 Nov 2022 22:43:46 +0000 Subject: [PATCH 3/9] fix test --- Lib/test/test_io.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index d8e8bf4cc25998..3d379e559944aa 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -893,8 +893,8 @@ def test_opener_invalid_fd(self): fd = os.open(support.TESTFN, os.O_RDONLY) os.close(fd) # fd is now an invalid file descriptor - with self.assertRaises(OSError): - self.assertEqual(exc.errno, errno.EBADF) + with self.assertRaises(OSError) as cm: + self.assertEqual(cm.exception.errno, errno.EBADF) self.open('foo', opener=lambda name, flags: fd) def test_fileio_closefd(self): From bc28d4881ce5da2fd92f92ad1e80c6b88803a925 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 23 Nov 2022 23:23:31 +0000 Subject: [PATCH 4/9] support.TESTFN --> os_helper.TESTFN --- Lib/test/test_io.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 3d379e559944aa..b833b36a11f39c 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -889,8 +889,8 @@ def badopener(fname, flags): self.assertEqual(str(cm.exception), 'opener returned -2') def test_opener_invalid_fd(self): - self.open(support.TESTFN, 'w').close() - fd = os.open(support.TESTFN, os.O_RDONLY) + self.open(os_helper.TESTFN, 'w').close() + fd = os.open(os_helper.TESTFN, os.O_RDONLY) os.close(fd) # fd is now an invalid file descriptor with self.assertRaises(OSError) as cm: From 94a3f050661d1d7d7bed172c59b221ac68c7f329 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 23 Nov 2022 23:58:18 +0000 Subject: [PATCH 5/9] fix test --- Lib/test/test_io.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index b833b36a11f39c..316010d3c8d784 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -894,8 +894,8 @@ def test_opener_invalid_fd(self): os.close(fd) # fd is now an invalid file descriptor with self.assertRaises(OSError) as cm: - self.assertEqual(cm.exception.errno, errno.EBADF) self.open('foo', opener=lambda name, flags: fd) + self.assertEqual(cm.exception.errno, errno.EBADF) def test_fileio_closefd(self): # Issue #4841 From b5ae5dbb2b289d5ad8589c3d88d2e17f5e237ed7 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 24 Nov 2022 14:37:56 +0000 Subject: [PATCH 6/9] Implement Victor's suggestions --- Lib/test/test_io.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 316010d3c8d784..127d826ac836cd 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -889,10 +889,10 @@ def badopener(fname, flags): self.assertEqual(str(cm.exception), 'opener returned -2') def test_opener_invalid_fd(self): - self.open(os_helper.TESTFN, 'w').close() - fd = os.open(os_helper.TESTFN, os.O_RDONLY) - os.close(fd) - # fd is now an invalid file descriptor + # Check that OSError is raised with EBADF error code if the opener returns an invalid + # file descriptor (see gh-15688). + + fd = os_helper.make_bad_fd() with self.assertRaises(OSError) as cm: self.open('foo', opener=lambda name, flags: fd) self.assertEqual(cm.exception.errno, errno.EBADF) From 793a8adaf900d91178ed00268edeb208baf6a6d8 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 24 Nov 2022 14:39:38 +0000 Subject: [PATCH 7/9] fix issue number in comment. --- Lib/test/test_io.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 127d826ac836cd..2a3717002b5776 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -890,7 +890,7 @@ def badopener(fname, flags): def test_opener_invalid_fd(self): # Check that OSError is raised with EBADF error code if the opener returns an invalid - # file descriptor (see gh-15688). + # file descriptor (see gh-82212). fd = os_helper.make_bad_fd() with self.assertRaises(OSError) as cm: From f05106d12e783a947797f4206b78d518ba7b62ce Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 24 Nov 2022 23:30:59 +0000 Subject: [PATCH 8/9] remove trailing whitespace --- Lib/test/test_io.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 2a3717002b5776..9d7f911a83d110 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -891,7 +891,6 @@ def badopener(fname, flags): def test_opener_invalid_fd(self): # Check that OSError is raised with EBADF error code if the opener returns an invalid # file descriptor (see gh-82212). - fd = os_helper.make_bad_fd() with self.assertRaises(OSError) as cm: self.open('foo', opener=lambda name, flags: fd) From 05109588a9b0a590c26e6927bad0bda9c25cebc8 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Thu, 24 Nov 2022 18:15:37 -0800 Subject: [PATCH 9/9] Modify a comment (that wasn't written by me) to make it adhere to PEP 8 --- Lib/test/test_io.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 9d7f911a83d110..c927f15aafef72 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -889,8 +889,8 @@ def badopener(fname, flags): self.assertEqual(str(cm.exception), 'opener returned -2') def test_opener_invalid_fd(self): - # Check that OSError is raised with EBADF error code if the opener returns an invalid - # file descriptor (see gh-82212). + # Check that OSError is raised with error code EBADF if the + # opener returns an invalid file descriptor (see gh-82212). fd = os_helper.make_bad_fd() with self.assertRaises(OSError) as cm: self.open('foo', opener=lambda name, flags: fd)