gh-84353: Preserve non-UTF-8 filenames when appending to ZipFile

Author: serhiy-storchaka

Lib/test/test_zipfile/test_core.py

@@ -3639,29 +3639,23 @@ def test_read_with_unsuitable_metadata_encoding(self):
 
     def test_read_after_append(self):
         newname = '\u56db'  # Han 'four'
-        expected_names = [name.encode('shift_jis').decode('cp437')
-                          for name in self.file_names[:2]] + self.file_names[2:]
-        expected_names.append(newname)
-        expected_content = (*self.file_content, b"newcontent")
+        newname2 = 'fünf'  # encodeable in cp437
+        expected_names = [*self.file_names, newname, newname2]
+        bad_expected_names = [name.encode('shift_jis').decode('cp437')
+                              if i < 2 else name
+                              for i, name in enumerate(expected_names)]
+        expected_content = (*self.file_content, b"newcontent", b"newcontent2")
 
         with zipfile.ZipFile(TESTFN, "a") as zipfp:
             zipfp.writestr(newname, "newcontent")
-            self.assertEqual(sorted(zipfp.namelist()), sorted(expected_names))
+            zipfp.writestr(newname2, "newcontent2")
+            self.assertEqual(sorted(zipfp.namelist()), sorted(bad_expected_names))
 
         with zipfile.ZipFile(TESTFN, "r") as zipfp:
-            self._test_read(zipfp, expected_names, expected_content)
+            self._test_read(zipfp, bad_expected_names, expected_content)
 
         with zipfile.ZipFile(TESTFN, "r", metadata_encoding='shift_jis') as zipfp:
-            self.assertEqual(sorted(zipfp.namelist()), sorted(expected_names))
-            for i, (name, content) in enumerate(zip(expected_names, expected_content)):
-                info = zipfp.getinfo(name)
-                self.assertEqual(info.filename, name)
-                self.assertEqual(info.file_size, len(content))
-                if i < 2:
-                    with self.assertRaises(zipfile.BadZipFile):
-                        zipfp.read(name)
-                else:
-                    self.assertEqual(zipfp.read(name), content)
+            self._test_read(zipfp, expected_names, expected_content)
 
     def test_write_with_metadata_encoding(self):
         ZF = zipfile.ZipFile
@@ -3670,6 +3664,20 @@ def test_write_with_metadata_encoding(self):
                                         "^metadata_encoding is only"):
                 ZF("nonesuch.zip", mode, metadata_encoding="shift_jis")
 
+    def test_add_comment(self):
+        with zipfile.ZipFile(TESTFN, "r") as zipfp:
+            bad_expected_names = zipfp.namelist()
+
+        with zipfile.ZipFile(TESTFN, "a") as zipfp:
+            zipfp.comment = b'comment'
+            self.assertEqual(zipfp.namelist(), bad_expected_names)
+
+        with zipfile.ZipFile(TESTFN, "r") as zipfp:
+            self._test_read(zipfp, bad_expected_names, self.file_content)
+
+        with zipfile.ZipFile(TESTFN, "r", metadata_encoding='shift_jis') as zipfp:
+            self._test_read(zipfp, self.file_names, self.file_content)
+
     def test_cli_with_metadata_encoding(self):
         errmsg = "Non-conforming encodings not supported with -c."
         args = ["--metadata-encoding=shift_jis", "-c", "nonesuch", "nonesuch"]

Lib/zipfile/__init__.py

@@ -515,7 +515,7 @@ def __repr__(self):
         result.append('>')
         return ''.join(result)
 
-    def FileHeader(self, zip64=None):
+    def FileHeader(self, zip64=None, metadata_encoding=None):
         """Return the per-file header as a bytes object.
 
         When the optional zip64 arg is None rather than a bool, we will
@@ -557,17 +557,19 @@ def FileHeader(self, zip64=None):
 
         self.extract_version = max(min_version, self.extract_version)
         self.create_version = max(min_version, self.create_version)
-        filename, flag_bits = self._encodeFilenameFlags()
+        filename, flag_bits = self._encodeFilenameFlags(metadata_encoding)
         header = struct.pack(structFileHeader, stringFileHeader,
                              self.extract_version, self.reserved, flag_bits,
                              self.compress_type, dostime, dosdate, CRC,
                              compress_size, file_size,
                              len(filename), len(extra))
         return header + filename + extra
 
-    def _encodeFilenameFlags(self):
+    def _encodeFilenameFlags(self, encoding):
+        if not encoding or self.flag_bits & _MASK_UTF_FILENAME:
+            encoding = 'ascii'
         try:
-            return self.filename.encode('ascii'), self.flag_bits
+            return self.filename.encode(encoding), self.flag_bits & ~_MASK_UTF_FILENAME
         except UnicodeEncodeError:
             return self.filename.encode('utf-8'), self.flag_bits | _MASK_UTF_FILENAME
 
@@ -1370,7 +1372,7 @@ def close(self):
                 # Preserve current position in file
                 self._zipfile.start_dir = self._fileobj.tell()
                 self._fileobj.seek(self._zinfo.header_offset)
-                self._fileobj.write(self._zinfo.FileHeader(self._zip64))
+                self._fileobj.write(self._zinfo.FileHeader(self._zip64, self._zipfile.metadata_encoding))
                 self._fileobj.seek(self._zipfile.start_dir)
 
             # Successfully written: Add file to our caches
@@ -1571,6 +1573,8 @@ def _RealGetContents(self):
             else:
                 # Historical ZIP filename encoding
                 filename = filename.decode(self.metadata_encoding or 'cp437')
+                if not self.metadata_encoding and not filename.isascii():
+                    self.metadata_encoding = "cp437"
             # Create ZipInfo instance to store file information
             x = ZipInfo(filename)
             x.extra = fp.read(centdir[_CD_EXTRA_FIELD_LENGTH])
@@ -1808,7 +1812,7 @@ def _open_to_write(self, zinfo, force_zip64=False):
         zinfo.compress_size = 0
         zinfo.CRC = 0
 
-        zinfo.flag_bits = 0x00
+        zinfo.flag_bits = _MASK_UTF_FILENAME
         if zinfo.compress_type == ZIP_LZMA:
             # Compressed data includes an end-of-stream (EOS) marker
             zinfo.flag_bits |= _MASK_COMPRESS_OPTION_1
@@ -1830,7 +1834,7 @@ def _open_to_write(self, zinfo, force_zip64=False):
         self._writecheck(zinfo)
         self._didModify = True
 
-        self.fp.write(zinfo.FileHeader(zip64))
+        self.fp.write(zinfo.FileHeader(zip64, self.metadata_encoding))
 
         self._writing = True
         return _ZipWriteFile(self, zinfo, zip64)
@@ -2062,7 +2066,7 @@ def mkdir(self, zinfo_or_directory_name, mode=511):
 
             self.filelist.append(zinfo)
             self.NameToInfo[zinfo.filename] = zinfo
-            self.fp.write(zinfo.FileHeader(False))
+            self.fp.write(zinfo.FileHeader(False, self.metadata_encoding))
             self.start_dir = self.fp.tell()
 
     def __del__(self):
@@ -2133,7 +2137,7 @@ def _write_end_record(self):
 
             extract_version = max(min_version, zinfo.extract_version)
             create_version = max(min_version, zinfo.create_version)
-            filename, flag_bits = zinfo._encodeFilenameFlags()
+            filename, flag_bits = zinfo._encodeFilenameFlags(self.metadata_encoding)
             centdir = struct.pack(structCentralDir,
                                   stringCentralDir, create_version,
                                   zinfo.create_system, extract_version, zinfo.reserved,

Misc/NEWS.d/next/Library/2026-05-19-19-00-49.gh-issue-84353.ZU5zaQ.rst

@@ -0,0 +1,2 @@
+Preserve non-ASCII filenames encoded not in UTF-8 when appending to
+:class:`zipfile.ZipFile`.