From d713ba88be673c902881e2e19b621488e4b5ff8c Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 3 Dec 2021 17:51:35 -0500 Subject: [PATCH 1/7] initial --- Lib/shutil.py | 5 +++++ Lib/test/test_shutil.py | 6 ++++++ Lib/test/testzip.zip | Bin 0 -> 466 bytes 3 files changed, 11 insertions(+) create mode 100644 Lib/test/testzip.zip diff --git a/Lib/shutil.py b/Lib/shutil.py index 949e024853c1d2f..d768aa76cf5eb86 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -1164,12 +1164,14 @@ def _unpack_zipfile(filename, extract_dir): raise ReadError("%s is not a zip file" % filename) zip = zipfile.ZipFile(filename) + skipped = 0 try: for info in zip.infolist(): name = info.filename # don't extract absolute paths or ones with .. in them if name.startswith('/') or '..' in name: + skipped += 1 continue targetpath = os.path.join(extract_dir, *name.split('/')) @@ -1183,6 +1185,9 @@ def _unpack_zipfile(filename, extract_dir): open(targetpath, 'wb') as target: copyfileobj(source, target) finally: + if skipped: + warnings.warn(f'unpack {filename}: {skipped} files skipped' + ' (due to absolute path or `..` path component)') zip.close() def _unpack_tarfile(filename, extract_dir): diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 7669b94ac35a688..dc47c84bf65def8 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1610,6 +1610,12 @@ def check_unpack_archive_with_converter(self, format, converter): self.assertRaises(shutil.ReadError, unpack_archive, converter(TESTFN)) self.assertRaises(ValueError, unpack_archive, converter(TESTFN), format='xxx') + def x(self): + tmpdir2 = self.mkdtemp() + with self.assertWarnsRegex(RuntimeWarning, '2 skipped'): + unpack_archive(pathlib.Path('Lib/test/testzip.zip'), pathlib.Path(tmpdir2)) + self.assertEqual(rlistdir(tmpdir2), ['test']) + def test_unpack_archive_tar(self): self.check_unpack_archive('tar') diff --git a/Lib/test/testzip.zip b/Lib/test/testzip.zip new file mode 100644 index 0000000000000000000000000000000000000000..a6a6b8fdd0ff24f97447998a5f75f64461af389f GIT binary patch literal 466 zcmWIWW@h1H0Dlwe_yVJJy0E(s0cWMH<6Ta^gHr4`%^j4WRn85meZfa(I^ zx>um-W(Vrl)6<6=U<1?)!Z;1k?nSj;7-&FfacWU9*o;yj3t^bh+*OGn3^5GkUZ`sr zndF#p`9=cjF9wDsjUXn(X{->xVK@t65~k0PO|k=;1otP4OfWW(x#hGF$H W&|s|oWn}~Tgc%4w0O>Oz4g&xVw@ZWo literal 0 HcmV?d00001 From 5bde783b6f5c84d21610c316b21cbc4b106ccfe0 Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 3 Dec 2021 19:03:27 -0500 Subject: [PATCH 2/7] add test; fix logic and add test zip file --- Lib/shutil.py | 3 ++- Lib/test/test_shutil.py | 4 ++-- Lib/test/testzip.zip | Bin 466 -> 302 bytes 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index d768aa76cf5eb86..ed5d4a518154379 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -10,6 +10,7 @@ import fnmatch import collections import errno +import warnings try: import zlib @@ -1186,7 +1187,7 @@ def _unpack_zipfile(filename, extract_dir): copyfileobj(source, target) finally: if skipped: - warnings.warn(f'unpack {filename}: {skipped} files skipped' + warnings.warn(f'unpack {filename}: {skipped} file(s) skipped' ' (due to absolute path or `..` path component)') zip.close() diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index dc47c84bf65def8..b28543a9161cb57 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1610,9 +1610,9 @@ def check_unpack_archive_with_converter(self, format, converter): self.assertRaises(shutil.ReadError, unpack_archive, converter(TESTFN)) self.assertRaises(ValueError, unpack_archive, converter(TESTFN), format='xxx') - def x(self): + def test_unpack_archive_zip_warn_skipped(self): tmpdir2 = self.mkdtemp() - with self.assertWarnsRegex(RuntimeWarning, '2 skipped'): + with self.assertWarnsRegex(UserWarning, '1 file\(s\) skipped'): unpack_archive(pathlib.Path('Lib/test/testzip.zip'), pathlib.Path(tmpdir2)) self.assertEqual(rlistdir(tmpdir2), ['test']) diff --git a/Lib/test/testzip.zip b/Lib/test/testzip.zip index a6a6b8fdd0ff24f97447998a5f75f64461af389f..f25050560af1b662b3722d12f42bb800f8bef895 100644 GIT binary patch literal 302 zcmWIWW@h1H00FLv&B0&>lwe_yVJJy0E(s0cWMEESyebidODnh;7+JnDGBB`+0M!M+ zb@QU><^bx})6)kTU<5WH3#c1}F-%}&l4Hi@0tvW-mNbHx5cjY`+=Is?OxGZrWCt_} Y;WnUYSlz|S2C|U}2xkE41`vk<04Po^YybcN literal 466 zcmWIWW@h1H0Dlwe_yVJJy0E(s0cWMH<6Ta^gHr4`%^j4WRn85meZfa(I^ zx>um-W(Vrl)6<6=U<1?)!Z;1k?nSj;7-&FfacWU9*o;yj3t^bh+*OGn3^5GkUZ`sr zndF#p`9=cjF9wDsjUXn(X{->xVK@t65~k0PO|k=;1otP4OfWW(x#hGF$H W&|s|oWn}~Tgc%4w0O>Oz4g&xVw@ZWo From 14b7e08d41730e73437b2d61579b1683657569a0 Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 3 Dec 2021 19:05:51 -0500 Subject: [PATCH 3/7] add news --- .../next/Library/2021-12-03-19-05-39.bpo-20907.f1_XQ1.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-12-03-19-05-39.bpo-20907.f1_XQ1.rst diff --git a/Misc/NEWS.d/next/Library/2021-12-03-19-05-39.bpo-20907.f1_XQ1.rst b/Misc/NEWS.d/next/Library/2021-12-03-19-05-39.bpo-20907.f1_XQ1.rst new file mode 100644 index 000000000000000..941cbbad90c2894 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-03-19-05-39.bpo-20907.f1_XQ1.rst @@ -0,0 +1,2 @@ +Added warning for skipped files in :func:`shutil.unpack_archive` using *zip* +format. From 6ca11606e7515d8d6f420e6c4d46f4af6a195f5c Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 3 Dec 2021 19:09:02 -0500 Subject: [PATCH 4/7] move warnings import --- Lib/shutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index ed5d4a518154379..5ff67b2453e1b70 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -10,7 +10,6 @@ import fnmatch import collections import errno -import warnings try: import zlib @@ -1187,6 +1186,7 @@ def _unpack_zipfile(filename, extract_dir): copyfileobj(source, target) finally: if skipped: + import warnings warnings.warn(f'unpack {filename}: {skipped} file(s) skipped' ' (due to absolute path or `..` path component)') zip.close() From 9ff16a226bca204f16712f9fb4d06a391746377e Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 3 Dec 2021 19:38:03 -0500 Subject: [PATCH 5/7] add note to the docs --- Doc/library/shutil.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst index 22d6dba9e1a9c61..9c1724fb1570153 100644 --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -657,6 +657,10 @@ provided. They rely on the :mod:`zipfile` and :mod:`tarfile` modules. registered for that extension. In case none is found, a :exc:`ValueError` is raised. + Note that with *zip* format, absolute paths and paths containing a ``..`` + component, are not extracted. If you need such paths extracted, consider + using :func:`ZipFile.extractall`. + .. audit-event:: shutil.unpack_archive filename,extract_dir,format shutil.unpack_archive .. versionchanged:: 3.7 From 54d3c4ed9f13a4bce3d52a9341b5cc2f64eac1e7 Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 3 Dec 2021 19:45:50 -0500 Subject: [PATCH 6/7] fix test --- Lib/test/test_shutil.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index b28543a9161cb57..6d531a80f5d73b3 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1613,7 +1613,8 @@ def check_unpack_archive_with_converter(self, format, converter): def test_unpack_archive_zip_warn_skipped(self): tmpdir2 = self.mkdtemp() with self.assertWarnsRegex(UserWarning, '1 file\(s\) skipped'): - unpack_archive(pathlib.Path('Lib/test/testzip.zip'), pathlib.Path(tmpdir2)) + fn = support.findfile("testzip.zip") + unpack_archive(pathlib.Path(fn), pathlib.Path(tmpdir2)) self.assertEqual(rlistdir(tmpdir2), ['test']) def test_unpack_archive_tar(self): From 70c8f295a8b2330b279ec805fb5004eaea36a37e Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 10 Dec 2021 23:53:56 -0500 Subject: [PATCH 7/7] use logging.warning instead of warnings.warn --- Lib/shutil.py | 6 ++++-- Lib/test/test_shutil.py | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index 5ff67b2453e1b70..df1e05dc102b96b 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -10,6 +10,7 @@ import fnmatch import collections import errno +import logging try: import zlib @@ -1186,8 +1187,9 @@ def _unpack_zipfile(filename, extract_dir): copyfileobj(source, target) finally: if skipped: - import warnings - warnings.warn(f'unpack {filename}: {skipped} file(s) skipped' + import logging + logging.getLogger(__file__) + logging.warning(f'unpack {filename}: {skipped} file(s) skipped' ' (due to absolute path or `..` path component)') zip.close() diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 6d531a80f5d73b3..c2b42c24fd30ec6 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1612,9 +1612,10 @@ def check_unpack_archive_with_converter(self, format, converter): def test_unpack_archive_zip_warn_skipped(self): tmpdir2 = self.mkdtemp() - with self.assertWarnsRegex(UserWarning, '1 file\(s\) skipped'): + with self.assertLogs(level='WARNING') as cm: fn = support.findfile("testzip.zip") unpack_archive(pathlib.Path(fn), pathlib.Path(tmpdir2)) + self.assertIn('1 file(s) skipped', cm.output[0]) self.assertEqual(rlistdir(tmpdir2), ['test']) def test_unpack_archive_tar(self):