From 82d9904edf5be390a348ce5fef82ec8dab27b1e3 Mon Sep 17 00:00:00 2001 From: Duane Griffin Date: Thu, 1 May 2025 15:57:01 +1200 Subject: [PATCH 1/4] gh-96531: tempfile.TemporaryFile should honour write-only mode If the `O_TMPFILE` is available and works `tempfile.TemporaryFile` will always open the temporary file as readable, even if the mode is write only. Fix this by masking off `O_RDWR` and setting `O_WRONLY` if "r" or "+" is not specified in the mode. --- Lib/tempfile.py | 3 +++ Lib/test/test_tempfile.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index cadb0bed3cce3b..9d7adaaa55eb43 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -651,6 +651,9 @@ def TemporaryFile(mode='w+b', buffering=-1, encoding=None, prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir) flags = _bin_openflags + if "r" not in mode and "+" not in mode: + flags = (flags & ~_os.O_RDWR) | _os.O_WRONLY + if _O_TMPFILE_WORKS: fd = None def opener(*args): diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index d46d3c0f040601..28788225ba996c 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1565,6 +1565,21 @@ def roundtrip(input, *args, **kwargs): roundtrip("\u039B", "w+", encoding="utf-16") roundtrip("foo\r\n", "w+", newline="") + def test_wronly_mode(self): + with mock.patch('os.open', wraps=os.open) as spy: + with tempfile.TemporaryFile(mode='wb') as fileobj: + fileobj.write(b'abc') + fileobj.seek(0) + self.assertRaises(io.UnsupportedOperation, fileobj.read) + with self.assertRaises(OSError): + os.read(fileobj.fileno(), 1) + + flags = spy.call_args[0][1] + self.assertEqual(flags & os.O_RDONLY, 0x0) + self.assertEqual(flags & os.O_RDWR, 0x0) + self.assertEqual(flags & os.O_WRONLY, os.O_WRONLY) + self.assertEqual(flags & os.O_EXCL, os.O_EXCL) + def test_bad_mode(self): dir = tempfile.mkdtemp() self.addCleanup(os_helper.rmtree, dir) From 29e2a0d9d2b5d466efcded790605c8c85133d534 Mon Sep 17 00:00:00 2001 From: Duane Griffin Date: Thu, 1 May 2025 16:16:41 +1200 Subject: [PATCH 2/4] Add blurb --- .../next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst diff --git a/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst b/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst new file mode 100644 index 00000000000000..4df3ed49c277f4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst @@ -0,0 +1,3 @@ +When using ``O_TMPFILE``, open :class:`tempfile.TemporaryFile` file descriptors +with ``O_WRONLY`` and not ``O_RDWR`` when a write-only mode is given. This will +prevent low-level reads on their underlying file descriptor. From 9639f6a89e40ed063f6319ebc3cfc421a6d354fc Mon Sep 17 00:00:00 2001 From: Duane Griffin Date: Thu, 1 May 2025 20:01:12 +1200 Subject: [PATCH 3/4] Extend fix to NamedTemporaryFile --- Lib/tempfile.py | 13 +++--- Lib/test/test_tempfile.py | 42 ++++++++++--------- ...5-05-01-16-10-47.gh-issue-96531.4WHMzW.rst | 6 +-- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 9d7adaaa55eb43..4da3e9af7de7d6 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -57,6 +57,12 @@ if hasattr(_os, 'O_BINARY'): _bin_openflags |= _os.O_BINARY +def _get_bin_openflags(mode): + if "r" in mode or "+" in mode: + return _bin_openflags + else: + return (_bin_openflags & ~_os.O_RDWR) | _os.O_WRONLY + if hasattr(_os, 'TMP_MAX'): TMP_MAX = _os.TMP_MAX else: @@ -583,7 +589,7 @@ def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir) - flags = _bin_openflags + flags = _get_bin_openflags(mode) # Setting O_TEMPORARY in the flags causes the OS to delete # the file when it is closed. This is only supported by Windows. @@ -650,10 +656,7 @@ def TemporaryFile(mode='w+b', buffering=-1, encoding=None, prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir) - flags = _bin_openflags - if "r" not in mode and "+" not in mode: - flags = (flags & ~_os.O_RDWR) | _os.O_WRONLY - + flags = _get_bin_openflags(mode) if _O_TMPFILE_WORKS: fd = None def opener(*args): diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 28788225ba996c..15b65392998999 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -940,14 +940,30 @@ def test_many(self): # We test _TemporaryFileWrapper by testing NamedTemporaryFile. -class TestNamedTemporaryFile(BaseTestCase): +class BaseTemporaryFileTestCases: + def test_wronly_mode(self): + with mock.patch('os.open', wraps=os.open) as spy: + with self.do_create(mode='wb') as fileobj: + fileobj.write(b'abc') + fileobj.seek(0) + self.assertRaises(io.UnsupportedOperation, fileobj.read) + with self.assertRaises(OSError): + os.read(fileobj.fileno(), 1) + + flags = spy.call_args[0][1] + self.assertEqual(flags & os.O_RDONLY, 0x0) + self.assertEqual(flags & os.O_RDWR, 0x0) + self.assertEqual(flags & os.O_WRONLY, os.O_WRONLY) + self.assertEqual(flags & os.O_EXCL, os.O_EXCL) + +class TestNamedTemporaryFile(BaseTestCase, BaseTemporaryFileTestCases): """Test NamedTemporaryFile().""" - def do_create(self, dir=None, pre="", suf="", delete=True): + def do_create(self, dir=None, pre="", suf="", delete=True, mode='w+b'): if dir is None: dir = tempfile.gettempdir() file = tempfile.NamedTemporaryFile(dir=dir, prefix=pre, suffix=suf, - delete=delete) + delete=delete, mode=mode) self.nameCheck(file.name, dir, pre, suf) return file @@ -1519,9 +1535,12 @@ def test_class_getitem(self): if tempfile.NamedTemporaryFile is not tempfile.TemporaryFile: - class TestTemporaryFile(BaseTestCase): + class TestTemporaryFile(BaseTestCase, BaseTemporaryFileTestCases): """Test TemporaryFile().""" + def do_create(self, *args, **kwargs): + return tempfile.TemporaryFile(*args, **kwargs) + def test_basic(self): # TemporaryFile can create files # No point in testing the name params - the file has no name. @@ -1565,21 +1584,6 @@ def roundtrip(input, *args, **kwargs): roundtrip("\u039B", "w+", encoding="utf-16") roundtrip("foo\r\n", "w+", newline="") - def test_wronly_mode(self): - with mock.patch('os.open', wraps=os.open) as spy: - with tempfile.TemporaryFile(mode='wb') as fileobj: - fileobj.write(b'abc') - fileobj.seek(0) - self.assertRaises(io.UnsupportedOperation, fileobj.read) - with self.assertRaises(OSError): - os.read(fileobj.fileno(), 1) - - flags = spy.call_args[0][1] - self.assertEqual(flags & os.O_RDONLY, 0x0) - self.assertEqual(flags & os.O_RDWR, 0x0) - self.assertEqual(flags & os.O_WRONLY, os.O_WRONLY) - self.assertEqual(flags & os.O_EXCL, os.O_EXCL) - def test_bad_mode(self): dir = tempfile.mkdtemp() self.addCleanup(os_helper.rmtree, dir) diff --git a/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst b/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst index 4df3ed49c277f4..5cf6dc57f28f6f 100644 --- a/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst +++ b/Misc/NEWS.d/next/Library/2025-05-01-16-10-47.gh-issue-96531.4WHMzW.rst @@ -1,3 +1,3 @@ -When using ``O_TMPFILE``, open :class:`tempfile.TemporaryFile` file descriptors -with ``O_WRONLY`` and not ``O_RDWR`` when a write-only mode is given. This will -prevent low-level reads on their underlying file descriptor. +Use the ``O_WRONLY`` flag instead of ``O_RDWR`` when opening file descriptors +for :class:`tempfile.TemporaryFile` and :class:`tempfile.NamedTemporaryFile` +objects created with a write-only mode). From d6ade24bbb0aa8b06c10c29a1d25da721bbce6b6 Mon Sep 17 00:00:00 2001 From: Duane Griffin Date: Thu, 1 May 2025 20:38:25 +1200 Subject: [PATCH 4/4] Fix WASI/musl issue where O_RDWR is (O_RDONLY | O_WRONLY) --- Lib/test/test_tempfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 15b65392998999..5bd02ba4fe04f2 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -952,7 +952,7 @@ def test_wronly_mode(self): flags = spy.call_args[0][1] self.assertEqual(flags & os.O_RDONLY, 0x0) - self.assertEqual(flags & os.O_RDWR, 0x0) + self.assertNotEqual(flags & os.O_RDWR, os.O_RDWR) self.assertEqual(flags & os.O_WRONLY, os.O_WRONLY) self.assertEqual(flags & os.O_EXCL, os.O_EXCL)