From b0d497c0726c118d40dfadea807b32077f067100 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 10 Sep 2016 21:28:07 +0300 Subject: Issue #24693: Changed some RuntimeError's in the zipfile module to more appropriate types. Improved some error messages and debugging output. --- Doc/library/zipfile.rst | 58 +++++++++++++++++++++++++++++++++++------------- Lib/test/test_zipfile.py | 40 ++++++++++++++++----------------- Lib/zipfile.py | 56 +++++++++++++++++++++++----------------------- Misc/NEWS | 3 +++ 4 files changed, 94 insertions(+), 63 deletions(-) diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst index 6c9d207..4b92e44 100644 --- a/Doc/library/zipfile.rst +++ b/Doc/library/zipfile.rst @@ -147,10 +147,10 @@ ZipFile Objects *compression* is the ZIP compression method to use when writing the archive, and should be :const:`ZIP_STORED`, :const:`ZIP_DEFLATED`, :const:`ZIP_BZIP2` or :const:`ZIP_LZMA`; unrecognized - values will cause :exc:`RuntimeError` to be raised. If :const:`ZIP_DEFLATED`, + values will cause :exc:`NotImplementedError` to be raised. If :const:`ZIP_DEFLATED`, :const:`ZIP_BZIP2` or :const:`ZIP_LZMA` is specified but the corresponding module (:mod:`zlib`, :mod:`bz2` or :mod:`lzma`) is not available, :exc:`RuntimeError` - is also raised. The default is :const:`ZIP_STORED`. If *allowZip64* is + is raised. The default is :const:`ZIP_STORED`. If *allowZip64* is ``True`` (the default) zipfile will create ZIP files that use the ZIP64 extensions when the zipfile is larger than 2 GiB. If it is false :mod:`zipfile` will raise an exception when the ZIP file would require ZIP64 extensions. @@ -179,6 +179,10 @@ ZipFile Objects Added support for writing to unseekable streams. Added support for the ``'x'`` mode. + .. versionchanged:: 3.6 + Previously, a plain :exc:`RuntimeError` was raised for unrecognized + compression values. + .. method:: ZipFile.close() @@ -211,7 +215,6 @@ ZipFile Objects can be either the name of a file within the archive or a :class:`ZipInfo` object. The *mode* parameter, if included, must be ``'r'`` (the default) or ``'w'``. *pwd* is the password used to decrypt encrypted ZIP files. - Calling :meth:`.open` on a closed ZipFile will raise a :exc:`RuntimeError`. :meth:`~ZipFile.open` is also a context manager and therefore supports the :keyword:`with` statement:: @@ -230,7 +233,7 @@ ZipFile Objects With ``mode='w'``, a writable file handle is returned, which supports the :meth:`~io.BufferedIOBase.write` method. While a writable file handle is open, attempting to read or write other files in the ZIP file will raise a - :exc:`RuntimeError`. + :exc:`ValueError`. When writing a file, if the file size is not known in advance but may exceed 2 GiB, pass ``force_zip64=True`` to ensure that the header format is @@ -252,6 +255,11 @@ ZipFile Objects :meth:`open` can now be used to write files into the archive with the ``mode='w'`` option. + .. versionchanged:: 3.6 + Calling :meth:`.open` on a closed ZipFile will raise a :exc:`ValueError`. + Previously, a :exc:`RuntimeError` was raised. + + .. method:: ZipFile.extract(member, path=None, pwd=None) Extract a member from the archive to the current working directory; *member* @@ -272,6 +280,10 @@ ZipFile Objects characters (``:``, ``<``, ``>``, ``|``, ``"``, ``?``, and ``*``) replaced by underscore (``_``). + .. versionchanged:: 3.6 + Calling :meth:`extract` on a closed ZipFile will raise a + :exc:`ValueError`. Previously, a :exc:`RuntimeError` was raised. + .. method:: ZipFile.extractall(path=None, members=None, pwd=None) @@ -288,6 +300,10 @@ ZipFile Objects dots ``".."``. This module attempts to prevent that. See :meth:`extract` note. + .. versionchanged:: 3.6 + Calling :meth:`extractall` on a closed ZipFile will raise a + :exc:`ValueError`. Previously, a :exc:`RuntimeError` was raised. + .. method:: ZipFile.printdir() @@ -305,18 +321,24 @@ ZipFile Objects file in the archive, or a :class:`ZipInfo` object. The archive must be open for read or append. *pwd* is the password used for encrypted files and, if specified, it will override the default password set with :meth:`setpassword`. Calling - :meth:`read` on a closed ZipFile will raise a :exc:`RuntimeError`. Calling :meth:`read` on a ZipFile that uses a compression method other than :const:`ZIP_STORED`, :const:`ZIP_DEFLATED`, :const:`ZIP_BZIP2` or :const:`ZIP_LZMA` will raise a :exc:`NotImplementedError`. An error will also be raised if the corresponding compression module is not available. + .. versionchanged:: 3.6 + Calling :meth:`read` on a closed ZipFile will raise a :exc:`ValueError`. + Previously, a :exc:`RuntimeError` was raised. + .. method:: ZipFile.testzip() Read all the files in the archive and check their CRC's and file headers. - Return the name of the first bad file, or else return ``None``. Calling - :meth:`testzip` on a closed ZipFile will raise a :exc:`RuntimeError`. + Return the name of the first bad file, or else return ``None``. + + .. versionchanged:: 3.6 + Calling :meth:`testfile` on a closed ZipFile will raise a + :exc:`ValueError`. Previously, a :exc:`RuntimeError` was raised. .. method:: ZipFile.write(filename, arcname=None, compress_type=None) @@ -326,10 +348,7 @@ ZipFile Objects letter and with leading path separators removed). If given, *compress_type* overrides the value given for the *compression* parameter to the constructor for the new entry. - The archive must be open with mode ``'w'``, ``'x'`` or ``'a'`` -- calling - :meth:`write` on a ZipFile created with mode ``'r'`` will raise a - :exc:`RuntimeError`. Calling :meth:`write` on a closed ZipFile will raise a - :exc:`RuntimeError`. + The archive must be open with mode ``'w'``, ``'x'`` or ``'a'``. .. note:: @@ -348,16 +367,19 @@ ZipFile Objects If ``arcname`` (or ``filename``, if ``arcname`` is not given) contains a null byte, the name of the file in the archive will be truncated at the null byte. + .. versionchanged:: 3.6 + Calling :meth:`write` on a ZipFile created with mode ``'r'`` or + a closed ZipFile will raise a :exc:`ValueError`. Previously, + a :exc:`RuntimeError` was raised. + + .. method:: ZipFile.writestr(zinfo_or_arcname, data[, compress_type]) Write the string *data* to the archive; *zinfo_or_arcname* is either the file name it will be given in the archive, or a :class:`ZipInfo` instance. If it's an instance, at least the filename, date, and time must be given. If it's a name, the date and time is set to the current date and time. - The archive must be opened with mode ``'w'``, ``'x'`` or ``'a'`` -- calling - :meth:`writestr` on a ZipFile created with mode ``'r'`` will raise a - :exc:`RuntimeError`. Calling :meth:`writestr` on a closed ZipFile will - raise a :exc:`RuntimeError`. + The archive must be opened with mode ``'w'``, ``'x'`` or ``'a'``. If given, *compress_type* overrides the value given for the *compression* parameter to the constructor for the new entry, or in the *zinfo_or_arcname* @@ -373,6 +395,12 @@ ZipFile Objects .. versionchanged:: 3.2 The *compress_type* argument. + .. versionchanged:: 3.6 + Calling :meth:`writestr` on a ZipFile created with mode ``'r'`` or + a closed ZipFile will raise a :exc:`ValueError`. Previously, + a :exc:`RuntimeError` was raised. + + The following data attributes are also available: diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py index ef3c3d8..ff4a6d6 100644 --- a/Lib/test/test_zipfile.py +++ b/Lib/test/test_zipfile.py @@ -449,15 +449,15 @@ class StoredTestsWithSourceFile(AbstractTestsWithSourceFile, def test_write_to_readonly(self): """Check that trying to call write() on a readonly ZipFile object - raises a RuntimeError.""" + raises a ValueError.""" with zipfile.ZipFile(TESTFN2, mode="w") as zipfp: zipfp.writestr("somefile.txt", "bogus") with zipfile.ZipFile(TESTFN2, mode="r") as zipfp: - self.assertRaises(RuntimeError, zipfp.write, TESTFN) + self.assertRaises(ValueError, zipfp.write, TESTFN) with zipfile.ZipFile(TESTFN2, mode="r") as zipfp: - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): zipfp.open(TESTFN, mode='w') def test_add_file_before_1980(self): @@ -1210,27 +1210,27 @@ class OtherTests(unittest.TestCase): fp.write("short file") self.assertRaises(zipfile.BadZipFile, zipfile.ZipFile, TESTFN) - def test_closed_zip_raises_RuntimeError(self): + def test_closed_zip_raises_ValueError(self): """Verify that testzip() doesn't swallow inappropriate exceptions.""" data = io.BytesIO() with zipfile.ZipFile(data, mode="w") as zipf: zipf.writestr("foo.txt", "O, for a Muse of Fire!") # This is correct; calling .read on a closed ZipFile should raise - # a RuntimeError, and so should calling .testzip. An earlier + # a ValueError, and so should calling .testzip. An earlier # version of .testzip would swallow this exception (and any other) # and report that the first file in the archive was corrupt. - self.assertRaises(RuntimeError, zipf.read, "foo.txt") - self.assertRaises(RuntimeError, zipf.open, "foo.txt") - self.assertRaises(RuntimeError, zipf.testzip) - self.assertRaises(RuntimeError, zipf.writestr, "bogus.txt", "bogus") + self.assertRaises(ValueError, zipf.read, "foo.txt") + self.assertRaises(ValueError, zipf.open, "foo.txt") + self.assertRaises(ValueError, zipf.testzip) + self.assertRaises(ValueError, zipf.writestr, "bogus.txt", "bogus") with open(TESTFN, 'w') as f: f.write('zipfile test data') - self.assertRaises(RuntimeError, zipf.write, TESTFN) + self.assertRaises(ValueError, zipf.write, TESTFN) def test_bad_constructor_mode(self): """Check that bad modes passed to ZipFile constructor are caught.""" - self.assertRaises(RuntimeError, zipfile.ZipFile, TESTFN, "q") + self.assertRaises(ValueError, zipfile.ZipFile, TESTFN, "q") def test_bad_open_mode(self): """Check that bad modes passed to ZipFile.open are caught.""" @@ -1240,10 +1240,10 @@ class OtherTests(unittest.TestCase): with zipfile.ZipFile(TESTFN, mode="r") as zipf: # read the data to make sure the file is there zipf.read("foo.txt") - self.assertRaises(RuntimeError, zipf.open, "foo.txt", "q") + self.assertRaises(ValueError, zipf.open, "foo.txt", "q") # universal newlines support is removed - self.assertRaises(RuntimeError, zipf.open, "foo.txt", "U") - self.assertRaises(RuntimeError, zipf.open, "foo.txt", "rU") + self.assertRaises(ValueError, zipf.open, "foo.txt", "U") + self.assertRaises(ValueError, zipf.open, "foo.txt", "rU") def test_read0(self): """Check that calling read(0) on a ZipExtFile object returns an empty @@ -1266,7 +1266,7 @@ class OtherTests(unittest.TestCase): def test_bad_compression_mode(self): """Check that bad compression methods passed to ZipFile.open are caught.""" - self.assertRaises(RuntimeError, zipfile.ZipFile, TESTFN, "w", -1) + self.assertRaises(NotImplementedError, zipfile.ZipFile, TESTFN, "w", -1) def test_unsupported_compression(self): # data is declared as shrunk, but actually deflated @@ -1423,15 +1423,15 @@ class OtherTests(unittest.TestCase): with zipf.open('foo', mode='w') as w2: w2.write(msg1) with zipf.open('bar', mode='w') as w1: - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): zipf.open('handle', mode='w') - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): zipf.open('foo', mode='r') - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): zipf.writestr('str', 'abcde') - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): zipf.write(__file__, 'file') - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): zipf.close() w1.write(msg2) with zipf.open('baz', mode='w') as w2: diff --git a/Lib/zipfile.py b/Lib/zipfile.py index 8dd064a..7ba4e59 100644 --- a/Lib/zipfile.py +++ b/Lib/zipfile.py @@ -449,7 +449,7 @@ class ZipInfo (object): elif ln == 0: counts = () else: - raise RuntimeError("Corrupt extra field %s"%(ln,)) + raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln)) idx = 0 @@ -654,7 +654,7 @@ def _check_compression(compression): raise RuntimeError( "Compression requires the (missing) lzma module") else: - raise RuntimeError("That compression method is not supported") + raise NotImplementedError("That compression method is not supported") def _get_compressor(compress_type): @@ -697,7 +697,7 @@ class _SharedFile: def read(self, n=-1): with self._lock: if self._writing(): - raise RuntimeError("Can't read from the ZIP file while there " + raise ValueError("Can't read from the ZIP file while there " "is an open writing handle on it. " "Close the writing handle before trying to read.") self._file.seek(self._pos) @@ -1055,7 +1055,7 @@ class ZipFile: """Open the ZIP file with mode read 'r', write 'w', exclusive create 'x', or append 'a'.""" if mode not in ('r', 'w', 'x', 'a'): - raise RuntimeError("ZipFile requires mode 'r', 'w', 'x', or 'a'") + raise ValueError("ZipFile requires mode 'r', 'w', 'x', or 'a'") _check_compression(compression) @@ -1129,7 +1129,7 @@ class ZipFile: self._didModify = True self.start_dir = self.fp.tell() else: - raise RuntimeError("Mode must be 'r', 'w', 'x', or 'a'") + raise ValueError("Mode must be 'r', 'w', 'x', or 'a'") except: fp = self.fp self.fp = None @@ -1277,7 +1277,7 @@ class ZipFile: def setpassword(self, pwd): """Set default password for encrypted files.""" if pwd and not isinstance(pwd, bytes): - raise TypeError("pwd: expected bytes, got %s" % type(pwd)) + raise TypeError("pwd: expected bytes, got %s" % type(pwd).__name__) if pwd: self.pwd = pwd else: @@ -1291,7 +1291,7 @@ class ZipFile: @comment.setter def comment(self, comment): if not isinstance(comment, bytes): - raise TypeError("comment: expected bytes, got %s" % type(comment)) + raise TypeError("comment: expected bytes, got %s" % type(comment).__name__) # check for valid comment length if len(comment) > ZIP_MAX_COMMENT: import warnings @@ -1323,13 +1323,13 @@ class ZipFile: instance for name, with zinfo.file_size set. """ if mode not in {"r", "w"}: - raise RuntimeError('open() requires mode "r" or "w"') + raise ValueError('open() requires mode "r" or "w"') if pwd and not isinstance(pwd, bytes): - raise TypeError("pwd: expected bytes, got %s" % type(pwd)) + raise TypeError("pwd: expected bytes, got %s" % type(pwd).__name__) if pwd and (mode == "w"): raise ValueError("pwd is only supported for reading files") if not self.fp: - raise RuntimeError( + raise ValueError( "Attempt to use ZIP archive that was already closed") # Make sure we have an info object @@ -1347,7 +1347,7 @@ class ZipFile: return self._open_to_write(zinfo, force_zip64=force_zip64) if self._writing: - raise RuntimeError("Can't read from the ZIP file while there " + raise ValueError("Can't read from the ZIP file while there " "is an open writing handle on it. " "Close the writing handle before trying to read.") @@ -1394,7 +1394,7 @@ class ZipFile: if not pwd: pwd = self.pwd if not pwd: - raise RuntimeError("File %s is encrypted, password " + raise RuntimeError("File %r is encrypted, password " "required for extraction" % name) zd = _ZipDecrypter(pwd) @@ -1412,7 +1412,7 @@ class ZipFile: # compare against the CRC otherwise check_byte = (zinfo.CRC >> 24) & 0xff if h[11] != check_byte: - raise RuntimeError("Bad password for file", name) + raise RuntimeError("Bad password for file %r" % name) return ZipExtFile(zef_file, mode, zinfo, zd, True) except: @@ -1426,9 +1426,9 @@ class ZipFile: "the ZIP file." ) if self._writing: - raise RuntimeError("Can't write to the ZIP file while there is " - "another write handle open on it. " - "Close the first handle before opening another.") + raise ValueError("Can't write to the ZIP file while there is " + "another write handle open on it. " + "Close the first handle before opening another.") # Sizes and CRC are overwritten with correct data after processing the file if not hasattr(zinfo, 'file_size'): @@ -1548,9 +1548,9 @@ class ZipFile: import warnings warnings.warn('Duplicate name: %r' % zinfo.filename, stacklevel=3) if self.mode not in ('w', 'x', 'a'): - raise RuntimeError("write() requires mode 'w', 'x', or 'a'") + raise ValueError("write() requires mode 'w', 'x', or 'a'") if not self.fp: - raise RuntimeError( + raise ValueError( "Attempt to write ZIP archive that was already closed") _check_compression(zinfo.compress_type) if not self._allowZip64: @@ -1569,10 +1569,10 @@ class ZipFile: """Put the bytes from filename into the archive under the name arcname.""" if not self.fp: - raise RuntimeError( + raise ValueError( "Attempt to write to ZIP archive that was already closed") if self._writing: - raise RuntimeError( + raise ValueError( "Can't write to ZIP archive while an open writing handle exists" ) @@ -1628,10 +1628,10 @@ class ZipFile: zinfo = zinfo_or_arcname if not self.fp: - raise RuntimeError( + raise ValueError( "Attempt to write to ZIP archive that was already closed") if self._writing: - raise RuntimeError( + raise ValueError( "Can't write to ZIP archive while an open writing handle exists." ) @@ -1654,9 +1654,9 @@ class ZipFile: return if self._writing: - raise RuntimeError("Can't close the ZIP file while there is " - "an open writing handle on it. " - "Close the writing handle before closing the zip.") + raise ValueError("Can't close the ZIP file while there is " + "an open writing handle on it. " + "Close the writing handle before closing the zip.") try: if self.mode in ('w', 'x', 'a') and self._didModify: # write ending records @@ -1803,7 +1803,7 @@ class PyZipFile(ZipFile): if filterfunc and not filterfunc(pathname): if self.debug: label = 'path' if os.path.isdir(pathname) else 'file' - print('%s "%s" skipped by filterfunc' % (label, pathname)) + print('%s %r skipped by filterfunc' % (label, pathname)) return dir, name = os.path.split(pathname) if os.path.isdir(pathname): @@ -1834,7 +1834,7 @@ class PyZipFile(ZipFile): elif ext == ".py": if filterfunc and not filterfunc(path): if self.debug: - print('file "%s" skipped by filterfunc' % path) + print('file %r skipped by filterfunc' % path) continue fname, arcname = self._get_codename(path[0:-3], basename) @@ -1851,7 +1851,7 @@ class PyZipFile(ZipFile): if ext == ".py": if filterfunc and not filterfunc(path): if self.debug: - print('file "%s" skipped by filterfunc' % path) + print('file %r skipped by filterfunc' % path) continue fname, arcname = self._get_codename(path[0:-3], basename) diff --git a/Misc/NEWS b/Misc/NEWS index 42821a4..27a3d8f 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -135,6 +135,9 @@ Core and Builtins Library ------- +- Issue #24693: Changed some RuntimeError's in the zipfile module to more + appropriate types. Improved some error messages and debugging output. + - Issue #17909: ``json.load`` and ``json.loads`` now support binary input encoded as UTF-8, UTF-16 or UTF-32. Patch by Serhiy Storchaka. -- cgit v0.12