#14399: zipfile now correctly handles comments added to empty zipfiles.

Patch by Serhiy Storchaka.

diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py
index 7ebb66376d80fdb43ce76e81d4881e9243b1d557..76fcc7048e0fd58af9ea573441c09465dfaee019 100644 (file)
--- a/Lib/test/test_zipfile.py
+++ b/Lib/test/test_zipfile.py
@@ -908,6 +908,22 @@ class OtherTests(unittest.TestCase):
         with zipfile.ZipFile(TESTFN, mode="r") as zipf:
             self.assertEqual(zipf.comment, comment2)
 
+    def test_change_comment_in_empty_archive(self):
+        with zipfile.ZipFile(TESTFN, "a", zipfile.ZIP_STORED) as zipf:
+            self.assertFalse(zipf.filelist)
+            zipf.comment = b"this is a comment"
+        with zipfile.ZipFile(TESTFN, "r") as zipf:
+            self.assertEqual(zipf.comment, b"this is a comment")
+
+    def test_change_comment_in_nonempty_archive(self):
+        with zipfile.ZipFile(TESTFN, "w", zipfile.ZIP_STORED) as zipf:
+            zipf.writestr("foo.txt", "O, for a Muse of Fire!")
+        with zipfile.ZipFile(TESTFN, "a", zipfile.ZIP_STORED) as zipf:
+            self.assertTrue(zipf.filelist)
+            zipf.comment = b"this is a comment"
+        with zipfile.ZipFile(TESTFN, "r") as zipf:
+            self.assertEqual(zipf.comment, b"this is a comment")
+
     def check_testzip_with_bad_crc(self, compression):
         """Tests that files with bad CRCs return their name from testzip."""
         zipdata = self.zips_with_bad_crc[compression]

diff --git a/Lib/zipfile.py b/Lib/zipfile.py
index d9181f2393fc4a0ee39c75f97f7413cc4730f3dd..05c31438d7fdc150a2d322ee7ba82616183ce85a 100644 (file)
--- a/Lib/zipfile.py
+++ b/Lib/zipfile.py
@@ -651,7 +651,7 @@ class ZipExtFile(io.BufferedIOBase):
 
 
 
-class ZipFile:
+class ZipFile(object):
     """ Class with methods to open, read, write, close, list zip files.
 
     z = ZipFile(file, mode="r", compression=ZIP_STORED, allowZip64=False)
@@ -690,7 +690,7 @@ class ZipFile:
         self.compression = compression  # Method of compression
         self.mode = key = mode.replace('b', '')[0]
         self.pwd = None
-        self.comment = ''
+        self._comment = ''
 
         # Check if we were passed a file-like object
         if isinstance(file, basestring):
@@ -765,7 +765,7 @@ class ZipFile:
             print endrec
         size_cd = endrec[_ECD_SIZE]             # bytes in central directory
         offset_cd = endrec[_ECD_OFFSET]         # offset of central directory
-        self.comment = endrec[_ECD_COMMENT]     # archive comment
+        self._comment = endrec[_ECD_COMMENT]    # archive comment
 
         # "concat" is zero, unless zip was concatenated to another file
         concat = endrec[_ECD_LOCATION] - size_cd - offset_cd
@@ -864,6 +864,22 @@ class ZipFile:
         """Set default password for encrypted files."""
         self.pwd = pwd
 
+    @property
+    def comment(self):
+        """The comment text associated with the ZIP file."""
+        return self._comment
+
+    @comment.setter
+    def comment(self, comment):
+        # check for valid comment length
+        if len(comment) >= ZIP_MAX_COMMENT:
+            if self.debug:
+                print('Archive comment is too long; truncating to %d bytes'
+                        % ZIP_MAX_COMMENT)
+            comment = comment[:ZIP_MAX_COMMENT]
+        self._comment = comment
+        self._didModify = True
+
     def read(self, name, pwd=None):
         """Return file bytes (as a string) for name."""
         return self.open(name, "r", pwd).read()
@@ -1243,18 +1259,11 @@ class ZipFile:
                 centDirSize = min(centDirSize, 0xFFFFFFFF)
                 centDirOffset = min(centDirOffset, 0xFFFFFFFF)
 
-            # check for valid comment length
-            if len(self.comment) >= ZIP_MAX_COMMENT:
-                if self.debug > 0:
-                    msg = 'Archive comment is too long; truncating to %d bytes' \
-                          % ZIP_MAX_COMMENT
-                self.comment = self.comment[:ZIP_MAX_COMMENT]
-
             endrec = struct.pack(structEndArchive, stringEndArchive,
                                  0, 0, centDirCount, centDirCount,
-                                 centDirSize, centDirOffset, len(self.comment))
+                                 centDirSize, centDirOffset, len(self._comment))
             self.fp.write(endrec)
-            self.fp.write(self.comment)
+            self.fp.write(self._comment)
             self.fp.flush()
 
         if not self._filePassed:

diff --git a/Misc/ACKS b/Misc/ACKS
index e36e626ca3124fff7ee4909508b6a6b0d57366e1..af0cd5f51cffe0ef3eb51fbaf5f1d073e8b07dbb 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -814,6 +814,7 @@ Richard Stoakley
 Peter Stoehr
 Casper Stoel
 Michael Stone
+Serhiy Storchaka
 Ken Stox
 Patrick Strawderman
 Dan Stromberg

diff --git a/Misc/NEWS b/Misc/NEWS
index 8aa7315b96804d6670f84daf158bc2cec59f17a5..64ecbd68b6118d0df65f0517349f09f2743e5aad 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -44,6 +44,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #14399: zipfile now correctly adds a comment even when the zipfile
+  being created is otherwise empty.  As a consequence of this fix, ZipFile is
+  now a new style class.
+
 - Issue #7978: SocketServer now restarts the select() call when EINTR is
   returned.  This avoids crashing the server loop when a signal is received.
   Patch by Jerzy Kozera.