gh-152845: Keep EFS flag for a file loaded from the archive

This is a regression introduced by gh-84353/gh-150091, where a file
with ASCII filename and UTF-8 comment lost its EFS flag when the
central directory was rewritten in append mode, causing an unexpected
metadata change and rendering its comment at risk of mis-decoding.

Introduce an internal `_metadata_encoding` attribute for `ZipInfo` to
make sure that files read from an archive keep the original encoding
and EFS flag, while any newly added file enforces EFS when having a
non-ASCII filename or comment.

Author: danny0838

Lib/test/test_zipfile/test_core.py

@@ -1440,15 +1440,7 @@ class ZstdWriterTests(AbstractWriterTests, unittest.TestCase):
 
 def comparable_zinfo(zinfo):
     """Return a dict of public ZipInfo attributes for assertEqual comparison."""
-    attrs = {k: getattr(zinfo, k) for k in _ZINFO_PUBLIC_KEYS}
-
-    # Since patch gh-84353, the _MASK_UTF_FILENAME (0x800) bit may be
-    # changed when writing to the end record depending on whether filename
-    # can be encoded with ascii or cp437. Skip checking this bit by
-    # pretending it's always set.
-    attrs['flag_bits'] |= 0x800
-
-    return attrs
+    return {k: getattr(zinfo, k) for k in _ZINFO_PUBLIC_KEYS}
 
 _struct_pack = struct.pack
 
@@ -5710,7 +5702,8 @@ def setUp(self):
         with open(TESTFN, "wb") as tf:
             tf.write(data)
 
-    def _test_read(self, zipfp, expected_names, expected_content):
+    def _test_read(self, zipfp, expected_names, expected_content,
+                   expected_comments=None, expected_efs_flags=None):
         # Check the namelist
         names = zipfp.namelist()
         self.assertEqual(names, expected_names)
@@ -5720,6 +5713,17 @@ def _test_read(self, zipfp, expected_names, expected_content):
         names = [zi.filename for zi in infos]
         self.assertEqual(names, expected_names)
 
+        if expected_comments is not None:
+            comments = [zi.comment for zi in infos]
+            self.assertEqual(comments, expected_comments)
+
+        if expected_efs_flags is not None:
+            efs_flags = [
+                bool(zi.flag_bits & zipfile._MASK_UTF_FILENAME)
+                for zi in infos
+            ]
+            self.assertEqual(efs_flags, expected_efs_flags)
+
         # check getinfo
         for name, content in zip(expected_names, expected_content):
             info = zipfp.getinfo(name)
@@ -5774,6 +5778,51 @@ def test_read_after_append(self):
         with zipfile.ZipFile(TESTFN, "r", metadata_encoding='shift_jis') as zipfp:
             self._test_read(zipfp, expected_names, expected_content)
 
+    def test_append_keep_efs_flag(self):
+        """Files loaded from an archive should keep original EFS flags when
+        rewriting metadata in append mode."""
+        names = ['file1', 'file2', 'file3', 'file4']
+        contents = [b'content1', b'content2', b'content3', b'content4']
+        comments = ['\u4e00'.encode('utf-8'), b'foo', '\u4e8c'.encode('shift_jis'), b'bar']
+        efs_flags = [True, True, False, False]
+
+        def mock_encode(self):
+            if efs_flags[i]:
+                zinfo.flag_bits |= zipfile._MASK_UTF_FILENAME
+            return (self.filename.encode('ascii'), self.flag_bits)
+
+        with mock.patch('zipfile.ZipInfo._encodeFilenameFlags', mock_encode), \
+             zipfile.ZipFile(TESTFN, "w") as zipfp:
+            for i, name in enumerate(names):
+                zinfo = zipfile.ZipInfo(name)
+                zinfo.comment = comments[i]
+                zipfp.writestr(zinfo, contents[i])
+
+        with zipfile.ZipFile(TESTFN, "a") as zipfp:
+            # trigger archive rewriting
+            zipfp.comment = b'com'
+
+        with zipfile.ZipFile(TESTFN, "r") as zipfp:
+            self.assertEqual(zipfp.comment, b'com')
+            self._test_read(zipfp, names, contents, comments, efs_flags)
+
+    def test_write_enforce_efs_flag(self):
+        """New files should enforce EFS flag if name or comment contains non-ASCII chars"""
+        names = ['\u4e00', '\u4e8c', 'file3', 'file4']
+        contents = [b'content1', b'content2', b'content3', b'content4']
+        comments = ['\u4e00'.encode('utf-8'), b'foo', '\u4e8c'.encode('utf-8'), b'bar']
+        efs_flags = [True, True, True, False]
+
+        with zipfile.ZipFile(TESTFN, "w") as zipfp:
+            for i, name in enumerate(names):
+                zinfo = zipfile.ZipInfo(name)
+                zinfo.comment = comments[i]
+                zipfp.writestr(zinfo, contents[i])
+            self.assertEqual(zipfp.namelist(), names)
+
+        with zipfile.ZipFile(TESTFN, "r") as zipfp:
+            self._test_read(zipfp, names, contents, comments, efs_flags)
+
     def test_write_with_metadata_encoding(self):
         ZF = zipfile.ZipFile
         for mode in ("w", "x", "a"):

Lib/zipfile/__init__.py

@@ -453,6 +453,7 @@ class ZipInfo:
         'file_size',
         '_raw_time',
         '_end_offset',
+        '_metadata_encoding',
     )
 
     def __init__(self, filename="NoName", date_time=(1980,1,1,0,0,0)):
@@ -488,6 +489,7 @@ def __init__(self, filename="NoName", date_time=(1980,1,1,0,0,0)):
         self.compress_size = 0          # Size of the compressed file
         self.file_size = 0              # Size of the uncompressed file
         self._end_offset = None         # Start of the next local header or central directory
+        self._metadata_encoding = None  # Metadata encoding when read
         # Other attributes are set by class ZipFile:
         # header_offset         Byte offset to the file header
         # CRC                   CRC-32 of the uncompressed file
@@ -575,13 +577,20 @@ def FileHeader(self, zip64=None):
 
     def _encodeFilenameFlags(self):
         if self.flag_bits & _MASK_UTF_FILENAME:
-            encoding = 'ascii'
+            return self.filename.encode('utf-8'), self.flag_bits
+
+        encoding = self._metadata_encoding
+        if encoding:
+            # Use the original encoding for a legacy file loaded from archive.
+            return self.filename.encode(encoding), self.flag_bits
         else:
-            encoding = 'cp437'
-        try:
-            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
+            # Enforce EFS if either filename or comment is non-ASCII for a newly
+            # added file.
+            try:
+                self.comment.decode('ascii')
+                return self.filename.encode('ascii'), self.flag_bits
+            except (UnicodeEncodeError, UnicodeDecodeError):
+                return self.filename.encode('utf-8'), self.flag_bits | _MASK_UTF_FILENAME
 
     def _decodeExtra(self, filename_crc):
         # Try to decode the extra field.
@@ -2072,6 +2081,10 @@ def _RealGetContents(self):
                             t>>11, (t>>5)&0x3F, (t&0x1F) * 2 )
             x._decodeExtra(orig_filename_crc)
             x.header_offset = x.header_offset + concat
+
+            # Enforce cp437 for a legacy file if not specified
+            x._metadata_encoding = self.metadata_encoding or 'cp437'
+
             self.filelist.append(x)
             self.NameToInfo[x.filename] = x
 
@@ -2286,7 +2299,7 @@ def _open_to_write(self, zinfo, force_zip64=False):
         zinfo.compress_size = 0
         zinfo.CRC = 0
 
-        zinfo.flag_bits = _MASK_UTF_FILENAME
+        zinfo.flag_bits = 0x00
         if zinfo.compress_type == ZIP_LZMA:
             # Compressed data includes an end-of-stream (EOS) marker
             zinfo.flag_bits |= _MASK_COMPRESS_OPTION_1