From 7e11002d92394ea4c4b6b318d8e192744ffe3b73 Mon Sep 17 00:00:00 2001 From: Som Date: Tue, 11 Aug 2026 19:00:32 +0000 Subject: [PATCH 1/3] gh-98758: Fix shutil.unpack_archive masking real filesystem errors unpack_archive() raised a generic ReadError("Unknown archive format") whenever the filename's extension didn't match a registered format -- including when the real problem was that the file didn't exist or wasn't readable, masking FileNotFoundError/ PermissionError the way open() would correctly raise them. When _find_unpack_format() finds no match, stat the path first (surfaces FileNotFoundError et al.) and, only for regular files, briefly open() it (surfaces PermissionError) before falling back to ReadError. Skipping the open() probe for non-regular files avoids a hang on FIFOs (a blocking read-open with no writer) and preserves the existing ReadError behavior for directories. --- Lib/shutil.py | 11 ++++ Lib/test/test_shutil.py | 51 ++++++++++++++++++- ...6-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst | 4 ++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst diff --git a/Lib/shutil.py b/Lib/shutil.py index ab75ba9da8894b6..613bddad2a4e08f 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -1424,6 +1424,17 @@ def unpack_archive(filename, extract_dir=None, format=None, *, filter=None): # we need to look at the registered unpackers supported extensions format = _find_unpack_format(filename) if format is None: + # Surface the real error (e.g. FileNotFoundError, PermissionError) + # when filename itself is the problem, instead of masking it with + # a generic "unknown format" message. os.stat() alone catches a + # missing path but not an unreadable *regular* file, so also + # open() regular files specifically -- opening a FIFO/socket/etc. + # could block indefinitely, and directories already fail the + # is-a-regular-file check the same way they always have. + st = os.stat(filename) + if stat.S_ISREG(st.st_mode): + with open(filename, 'rb'): + pass raise ReadError("Unknown archive format '{0}'".format(filename)) func = _UNPACK_FORMATS[format][1] diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 06ebdf9b68f20fc..210b091a1b2f833 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -2237,11 +2237,60 @@ def check_unpack_archive_with_converter(self, format, converter, **kwargs): **kwargs) self.assertEqual(rlistdir(tmpdir3), expected) - with self.assertRaises(shutil.ReadError): + # gh-98758: a nonexistent file must raise FileNotFoundError, not the + # generic ReadError that used to mask it. + with self.assertRaises(FileNotFoundError): unpack_archive(converter(TESTFN), **kwargs) with self.assertRaises(ValueError): unpack_archive(converter(TESTFN), format='xxx', **kwargs) + def test_unpack_archive_unknown_format_existing_file(self): + # gh-98758: an accessible file with a genuinely unrecognized + # extension must still raise ReadError, unlike a missing file. + tmpdir = self.mkdtemp() + filename = os.path.join(tmpdir, 'archive.unknownext') + os_helper.create_empty_file(filename) + with self.assertRaises(shutil.ReadError): + unpack_archive(filename) + + def test_unpack_archive_unknown_format_directory(self): + # gh-98758: a directory is not a regular file, so the accessibility + # probe must not attempt to open() it -- it should still raise + # ReadError (not IsADirectoryError), unchanged from before the fix. + tmpdir = self.mkdtemp() + dirname = os.path.join(tmpdir, 'archive.unknownext') + os.mkdir(dirname) + with self.assertRaises(shutil.ReadError): + unpack_archive(dirname) + + @unittest.skipUnless(hasattr(os, 'mkfifo'), 'requires os.mkfifo') + def test_unpack_archive_unknown_format_fifo(self): + # gh-98758: a FIFO is not a regular file, so the accessibility probe + # must not attempt to open() it -- opening a FIFO for reading blocks + # until a writer appears, which would hang unpack_archive() instead + # of promptly raising ReadError as before the fix. + tmpdir = self.mkdtemp() + fifo_path = os.path.join(tmpdir, 'archive.unknownext') + os.mkfifo(fifo_path) + with self.assertRaises(shutil.ReadError): + unpack_archive(fifo_path) + + @unittest.skipIf(sys.platform[:6] == 'cygwin', + "This test can't be run on Cygwin (issue #1071513).") + @os_helper.skip_if_dac_override + @os_helper.skip_unless_working_chmod + def test_unpack_archive_permission_denied(self): + # gh-98758: a file that exists but can't be read must raise + # PermissionError, not the generic ReadError. + tmpdir = self.mkdtemp() + filename = os.path.join(tmpdir, 'archive.unknownext') + os_helper.create_empty_file(filename) + old_mode = os.stat(filename).st_mode + os.chmod(filename, 0) + self.addCleanup(os.chmod, filename, old_mode) + with self.assertRaises(PermissionError): + unpack_archive(filename) + def check_unpack_tarball(self, format): self.check_unpack_archive(format, filter='fully_trusted') self.check_unpack_archive(format, filter='data') diff --git a/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst b/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst new file mode 100644 index 000000000000000..0d2af644a644825 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst @@ -0,0 +1,4 @@ +:func:`shutil.unpack_archive` now propagates the real underlying error +(such as :exc:`FileNotFoundError` or :exc:`PermissionError`) when the +input file is missing or inaccessible, instead of masking it with a +generic :exc:`shutil.ReadError` about an unrecognized archive format. From 80bbc49c125faf37e05302efc083d068ff33f338 Mon Sep 17 00:00:00 2001 From: Som Date: Tue, 11 Aug 2026 19:10:46 +0000 Subject: [PATCH 2/3] gh-98758: Fix unresolvable :exc: role in NEWS entry shutil.ReadError has no documented exception entry in the docs (only tarfile.ReadError does), so Sphinx's py:exc cross-reference role can't resolve it, failing the Docs CI check. Use plain code markup instead. --- .../next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst b/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst index 0d2af644a644825..3325c9982f33c16 100644 --- a/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst +++ b/Misc/NEWS.d/next/Library/2026-08-11-19-05-00.gh-issue-98758.5S1Ijz.rst @@ -1,4 +1,4 @@ :func:`shutil.unpack_archive` now propagates the real underlying error (such as :exc:`FileNotFoundError` or :exc:`PermissionError`) when the input file is missing or inaccessible, instead of masking it with a -generic :exc:`shutil.ReadError` about an unrecognized archive format. +generic ``shutil.ReadError`` about an unrecognized archive format. From 608b7f69922e8471866595edefb84d9bf78553d8 Mon Sep 17 00:00:00 2001 From: Som Date: Wed, 12 Aug 2026 01:59:17 +0000 Subject: [PATCH 3/3] gh-98758: Skip permission-denied test on Windows os.chmod() on Windows can only toggle the read-only attribute, not actually restrict read access, so chmod(path, 0) followed by open(path, 'rb') still succeeds there -- the test's premise doesn't hold on that platform. --- Lib/test/test_shutil.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 210b091a1b2f833..9ca7fe925bdd2a7 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -2277,6 +2277,9 @@ def test_unpack_archive_unknown_format_fifo(self): @unittest.skipIf(sys.platform[:6] == 'cygwin', "This test can't be run on Cygwin (issue #1071513).") + @unittest.skipIf(sys.platform == 'win32', + "os.chmod() cannot make a file unreadable on Windows, " + "only read-only (see the os.chmod() docs)") @os_helper.skip_if_dac_override @os_helper.skip_unless_working_chmod def test_unpack_archive_permission_denied(self):