From a263718a19eb7828b114a2634d2fe8895c42dbf6 Mon Sep 17 00:00:00 2001
From: Tim Hatch <tim@timhatch.com>
Date: Sun, 21 Aug 2022 10:33:00 -0700
Subject: [PATCH 1/6] zipfile: handle extras after a zip64 extra

Previously, any data _after_ the zip64 extra would be removed.

With many new tests.

Fixes #88233
---
 Lib/test/test_zipfile.py | 52 ++++++++++++++++++++++++++++++++++++++++
 Lib/zipfile.py           |  2 ++
 2 files changed, 54 insertions(+)

diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py
index 6f6f4bc26b0d40..9d4925d26bd12e 100644
--- a/Lib/test/test_zipfile.py
+++ b/Lib/test/test_zipfile.py
@@ -3425,6 +3425,58 @@ def test_cli_with_metadata_encoding_extract(self):
         for name in self.file_names:
             self.assertIn(name, listing)
 
+class StripExtraTests(unittest.TestCase):
+    # Note: all of the "z" characters are technically invalid, but up to 3 bytes
+    # at the end of the extra will be passed through as they are too short to
+    # encode a valid extra.
+    def test_no_data(self):
+        s = struct.Struct("<HH")
+        a = s.pack(1, 0)  # 1=zip64 exta signature
+        b = s.pack(2, 0)
+        c = s.pack(3, 0)
+
+        self.assertEqual(b'', zipfile._strip_extra(a, (1,)))
+        self.assertEqual(b, zipfile._strip_extra(b, (1,)))
+        self.assertEqual(b+b"z", zipfile._strip_extra(b+b"z", (1,)))
+
+        self.assertEqual(b+c, zipfile._strip_extra(a+b+c, (1,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+a+c, (1,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+c+a, (1,)))
+
+    def test_with_data(self):
+        s = struct.Struct("<HH")
+        a = s.pack(1, 1) + b"a"  # 1=zip64 exta signature
+        b = s.pack(2, 2) + b"bb"
+        c = s.pack(3, 3) + b"ccc"
+
+        self.assertEqual(b"", zipfile._strip_extra(a, (1,)))
+        self.assertEqual(b, zipfile._strip_extra(b, (1,)))
+        self.assertEqual(b+b"z", zipfile._strip_extra(b+b"z", (1,)))
+
+        self.assertEqual(b+c, zipfile._strip_extra(a+b+c, (1,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+a+c, (1,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+c+a, (1,)))
+
+    def test_multiples(self):
+        s = struct.Struct("<HH")
+        a = s.pack(1, 1) + b"a"  # 1=zip64 exta signature
+        b = s.pack(2, 2) + b"bb"
+        c = s.pack(3, 3) + b"ccc"
+
+        self.assertEqual(b"", zipfile._strip_extra(a+a, (1,)))
+        self.assertEqual(b"", zipfile._strip_extra(a+a+a, (1,)))
+        self.assertEqual(b"z", zipfile._strip_extra(a+a+b"z", (1,)))
+        self.assertEqual(b+b"z", zipfile._strip_extra(a+a+b+b"z", (1,)))
+
+        self.assertEqual(b, zipfile._strip_extra(a+a+b, (1,)))
+        self.assertEqual(b, zipfile._strip_extra(a+b+a, (1,)))
+        self.assertEqual(b, zipfile._strip_extra(b+a+a, (1,)))
+
+    def test_too_short(self):
+        self.assertEqual(b"", zipfile._strip_extra(b"", (1,)))
+        self.assertEqual(b"z", zipfile._strip_extra(b"z", (1,)))
+        self.assertEqual(b"zz", zipfile._strip_extra(b"zz", (1,)))
+        self.assertEqual(b"zzz", zipfile._strip_extra(b"zzz", (1,)))
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/Lib/zipfile.py b/Lib/zipfile.py
index b6465373c107d5..7fb53f3836987d 100644
--- a/Lib/zipfile.py
+++ b/Lib/zipfile.py
@@ -211,6 +211,8 @@ def _strip_extra(extra, xids):
         i = j
     if not modified:
         return extra
+    if start != len(extra):
+        buffer.append(extra[start:])
     return b''.join(buffer)
 
 def _check_zipfile(fp):

From f995644c3968a2c730aab96cd16f5360a880e9e2 Mon Sep 17 00:00:00 2001
From: Tim Hatch <tim@timhatch.com>
Date: Mon, 5 Sep 2022 12:17:42 -0700
Subject: [PATCH 2/6] Add NEWS entry for bugfix

---
 .../next/Library/2022-09-05-12-17-34.gh-issue-88233.gff9qJ.rst  | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 Misc/NEWS.d/next/Library/2022-09-05-12-17-34.gh-issue-88233.gff9qJ.rst

diff --git a/Misc/NEWS.d/next/Library/2022-09-05-12-17-34.gh-issue-88233.gff9qJ.rst b/Misc/NEWS.d/next/Library/2022-09-05-12-17-34.gh-issue-88233.gff9qJ.rst
new file mode 100644
index 00000000000000..806f7011edc398
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-09-05-12-17-34.gh-issue-88233.gff9qJ.rst
@@ -0,0 +1,2 @@
+Correctly preserve "extra" fields in ``zipfile`` regardless of their
+ordering relative to a zip64 "extra."

From 9c58352606a3bf37def684b4fea2da6d568a6f9a Mon Sep 17 00:00:00 2001
From: "Jason R. Coombs" <jaraco@jaraco.com>
Date: Mon, 20 Feb 2023 11:28:43 -0500
Subject: [PATCH 3/6] Fix typo

---
 Lib/test/test_zipfile/test_core.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py
index 1877fb5d796fcc..c6cec2ac9c879f 100644
--- a/Lib/test/test_zipfile/test_core.py
+++ b/Lib/test/test_zipfile/test_core.py
@@ -3015,7 +3015,7 @@ class StripExtraTests(unittest.TestCase):
     # encode a valid extra.
     def test_no_data(self):
         s = struct.Struct("<HH")
-        a = s.pack(1, 0)  # 1=zip64 exta signature
+        a = s.pack(1, 0)  # 1=zip64 extra signature
         b = s.pack(2, 0)
         c = s.pack(3, 0)
 
@@ -3029,7 +3029,7 @@ def test_no_data(self):
 
     def test_with_data(self):
         s = struct.Struct("<HH")
-        a = s.pack(1, 1) + b"a"  # 1=zip64 exta signature
+        a = s.pack(1, 1) + b"a"  # 1=zip64 extra signature
         b = s.pack(2, 2) + b"bb"
         c = s.pack(3, 3) + b"ccc"
 
@@ -3043,7 +3043,7 @@ def test_with_data(self):
 
     def test_multiples(self):
         s = struct.Struct("<HH")
-        a = s.pack(1, 1) + b"a"  # 1=zip64 exta signature
+        a = s.pack(1, 1) + b"a"  # 1=zip64 extra signature
         b = s.pack(2, 2) + b"bb"
         c = s.pack(3, 3) + b"ccc"
 

From 72f6e4d0df4b47a6b2a6f91f58d481331983cfd1 Mon Sep 17 00:00:00 2001
From: "Jason R. Coombs" <jaraco@jaraco.com>
Date: Mon, 20 Feb 2023 11:29:20 -0500
Subject: [PATCH 4/6] Tweak style to conform to best conventions. Remove unused
 variable.

---
 Lib/test/test_zipfile/test_core.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py
index c6cec2ac9c879f..9f569cb6f4ba99 100644
--- a/Lib/test/test_zipfile/test_core.py
+++ b/Lib/test/test_zipfile/test_core.py
@@ -3009,6 +3009,7 @@ def test_cli_with_metadata_encoding_extract(self):
         for name in self.file_names:
             self.assertIn(name, listing)
 
+
 class StripExtraTests(unittest.TestCase):
     # Note: all of the "z" characters are technically invalid, but up to 3 bytes
     # at the end of the extra will be passed through as they are too short to
@@ -3045,7 +3046,6 @@ def test_multiples(self):
         s = struct.Struct("<HH")
         a = s.pack(1, 1) + b"a"  # 1=zip64 extra signature
         b = s.pack(2, 2) + b"bb"
-        c = s.pack(3, 3) + b"ccc"
 
         self.assertEqual(b"", zipfile._strip_extra(a+a, (1,)))
         self.assertEqual(b"", zipfile._strip_extra(a+a+a, (1,)))
@@ -3062,5 +3062,6 @@ def test_too_short(self):
         self.assertEqual(b"zz", zipfile._strip_extra(b"zz", (1,)))
         self.assertEqual(b"zzz", zipfile._strip_extra(b"zzz", (1,)))
 
+
 if __name__ == "__main__":
     unittest.main()

From b1dad5963da756b17060bd953ff968ba0c4c0a69 Mon Sep 17 00:00:00 2001
From: "Jason R. Coombs" <jaraco@jaraco.com>
Date: Mon, 20 Feb 2023 11:31:12 -0500
Subject: [PATCH 5/6] Reflow comment to 72 characters per PEP 8.

---
 Lib/test/test_zipfile/test_core.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py
index 9f569cb6f4ba99..6c6cae2c36022d 100644
--- a/Lib/test/test_zipfile/test_core.py
+++ b/Lib/test/test_zipfile/test_core.py
@@ -3011,9 +3011,9 @@ def test_cli_with_metadata_encoding_extract(self):
 
 
 class StripExtraTests(unittest.TestCase):
-    # Note: all of the "z" characters are technically invalid, but up to 3 bytes
-    # at the end of the extra will be passed through as they are too short to
-    # encode a valid extra.
+    # Note: all of the "z" characters are technically invalid, but up
+    # to 3 bytes at the end of the extra will be passed through as they
+    # are too short to encode a valid extra.
     def test_no_data(self):
         s = struct.Struct("<HH")
         a = s.pack(1, 0)  # 1=zip64 extra signature

From 61ad114307b6a021669f46f5a4ace9200d702c3b Mon Sep 17 00:00:00 2001
From: "Jason R. Coombs" <jaraco@jaraco.com>
Date: Mon, 20 Feb 2023 11:35:47 -0500
Subject: [PATCH 6/6] Replace literal and ambiguous comment with a
 self-documenting constant.

---
 Lib/test/test_zipfile/test_core.py | 61 +++++++++++++++++-------------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py
index 6c6cae2c36022d..e23f5c2a8556f2 100644
--- a/Lib/test/test_zipfile/test_core.py
+++ b/Lib/test/test_zipfile/test_core.py
@@ -3014,53 +3014,62 @@ class StripExtraTests(unittest.TestCase):
     # Note: all of the "z" characters are technically invalid, but up
     # to 3 bytes at the end of the extra will be passed through as they
     # are too short to encode a valid extra.
+
+    ZIP64_EXTRA = 1
+
     def test_no_data(self):
         s = struct.Struct("<HH")
-        a = s.pack(1, 0)  # 1=zip64 extra signature
+        a = s.pack(self.ZIP64_EXTRA, 0)
         b = s.pack(2, 0)
         c = s.pack(3, 0)
 
-        self.assertEqual(b'', zipfile._strip_extra(a, (1,)))
-        self.assertEqual(b, zipfile._strip_extra(b, (1,)))
-        self.assertEqual(b+b"z", zipfile._strip_extra(b+b"z", (1,)))
+        self.assertEqual(b'', zipfile._strip_extra(a, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b, zipfile._strip_extra(b, (self.ZIP64_EXTRA,)))
+        self.assertEqual(
+            b+b"z", zipfile._strip_extra(b+b"z", (self.ZIP64_EXTRA,)))
 
-        self.assertEqual(b+c, zipfile._strip_extra(a+b+c, (1,)))
-        self.assertEqual(b+c, zipfile._strip_extra(b+a+c, (1,)))
-        self.assertEqual(b+c, zipfile._strip_extra(b+c+a, (1,)))
+        self.assertEqual(b+c, zipfile._strip_extra(a+b+c, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+a+c, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+c+a, (self.ZIP64_EXTRA,)))
 
     def test_with_data(self):
         s = struct.Struct("<HH")
-        a = s.pack(1, 1) + b"a"  # 1=zip64 extra signature
+        a = s.pack(self.ZIP64_EXTRA, 1) + b"a"
         b = s.pack(2, 2) + b"bb"
         c = s.pack(3, 3) + b"ccc"
 
-        self.assertEqual(b"", zipfile._strip_extra(a, (1,)))
-        self.assertEqual(b, zipfile._strip_extra(b, (1,)))
-        self.assertEqual(b+b"z", zipfile._strip_extra(b+b"z", (1,)))
+        self.assertEqual(b"", zipfile._strip_extra(a, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b, zipfile._strip_extra(b, (self.ZIP64_EXTRA,)))
+        self.assertEqual(
+            b+b"z", zipfile._strip_extra(b+b"z", (self.ZIP64_EXTRA,)))
 
-        self.assertEqual(b+c, zipfile._strip_extra(a+b+c, (1,)))
-        self.assertEqual(b+c, zipfile._strip_extra(b+a+c, (1,)))
-        self.assertEqual(b+c, zipfile._strip_extra(b+c+a, (1,)))
+        self.assertEqual(b+c, zipfile._strip_extra(a+b+c, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+a+c, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b+c, zipfile._strip_extra(b+c+a, (self.ZIP64_EXTRA,)))
 
     def test_multiples(self):
         s = struct.Struct("<HH")
-        a = s.pack(1, 1) + b"a"  # 1=zip64 extra signature
+        a = s.pack(self.ZIP64_EXTRA, 1) + b"a"
         b = s.pack(2, 2) + b"bb"
 
-        self.assertEqual(b"", zipfile._strip_extra(a+a, (1,)))
-        self.assertEqual(b"", zipfile._strip_extra(a+a+a, (1,)))
-        self.assertEqual(b"z", zipfile._strip_extra(a+a+b"z", (1,)))
-        self.assertEqual(b+b"z", zipfile._strip_extra(a+a+b+b"z", (1,)))
+        self.assertEqual(b"", zipfile._strip_extra(a+a, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b"", zipfile._strip_extra(a+a+a, (self.ZIP64_EXTRA,)))
+        self.assertEqual(
+            b"z", zipfile._strip_extra(a+a+b"z", (self.ZIP64_EXTRA,)))
+        self.assertEqual(
+            b+b"z", zipfile._strip_extra(a+a+b+b"z", (self.ZIP64_EXTRA,)))
 
-        self.assertEqual(b, zipfile._strip_extra(a+a+b, (1,)))
-        self.assertEqual(b, zipfile._strip_extra(a+b+a, (1,)))
-        self.assertEqual(b, zipfile._strip_extra(b+a+a, (1,)))
+        self.assertEqual(b, zipfile._strip_extra(a+a+b, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b, zipfile._strip_extra(a+b+a, (self.ZIP64_EXTRA,)))
+        self.assertEqual(b, zipfile._strip_extra(b+a+a, (self.ZIP64_EXTRA,)))
 
     def test_too_short(self):
-        self.assertEqual(b"", zipfile._strip_extra(b"", (1,)))
-        self.assertEqual(b"z", zipfile._strip_extra(b"z", (1,)))
-        self.assertEqual(b"zz", zipfile._strip_extra(b"zz", (1,)))
-        self.assertEqual(b"zzz", zipfile._strip_extra(b"zzz", (1,)))
+        self.assertEqual(b"", zipfile._strip_extra(b"", (self.ZIP64_EXTRA,)))
+        self.assertEqual(b"z", zipfile._strip_extra(b"z", (self.ZIP64_EXTRA,)))
+        self.assertEqual(
+            b"zz", zipfile._strip_extra(b"zz", (self.ZIP64_EXTRA,)))
+        self.assertEqual(
+            b"zzz", zipfile._strip_extra(b"zzz", (self.ZIP64_EXTRA,)))
 
 
 if __name__ == "__main__":