FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[3.11] gh-103861: Fix Zip64 extensions not being properly applied in some cases (GH-103863) by gpshead · Pull Request #104534 · python/cpython · GitHub

/ cpython Public

[3.11] gh-103861: Fix Zip64 extensions not being properly applied in some cases (GH-103863) #104534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gpshead merged 1 commit into from
May 17, 2023
+172 −15
Conversation 0 Commits 1 Checks 0 Files changed 3
Merged

[3.11] gh-103861: Fix Zip64 extensions not being properly applied in some cases (GH-103863) #104534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .py  (2) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
153 changes: 153 additions & 0 deletions Lib/test/test_zipfile.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,159 @@ def test_generated_valid_zip64_extra(self):
self.assertEqual(zinfo.header_offset, expected_header_offset)
self.assertEqual(zf.read(zinfo), expected_content)

def test_force_zip64(self):
"""Test that forcing zip64 extensions correctly notes this in the zip file"""

# GH-103861 describes an issue where forcing a small file to use zip64
# extensions would add a zip64 extra record, but not change the data
# sizes to 0xFFFFFFFF to indicate to the extractor that the zip64
# record should be read. Additionally, it would not set the required
# version to indicate that zip64 extensions are required to extract it.
# This test replicates the situation and reads the raw data to specifically ensure:
# - The required extract version is always >= ZIP64_VERSION
# - The compressed and uncompressed size in the file headers are both
# 0xFFFFFFFF (ie. point to zip64 record)
# - The zip64 record is provided and has the correct sizes in it
# Other aspects of the zip are checked as well, but verifying the above is the main goal.
# Because this is hard to verify by parsing the data as a zip, the raw
# bytes are checked to ensure that they line up with the zip spec.
# The spec for this can be found at: https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
# The relevent sections for this test are:
# - 4.3.7 for local file header
# - 4.5.3 for zip64 extra field

data = io.BytesIO()
with zipfile.ZipFile(data, mode="w", allowZip64=True) as zf:
with zf.open("text.txt", mode="w", force_zip64=True) as zi:
zi.write(b"_")

zipdata = data.getvalue()

# pull out and check zip information
(
header, vers, os, flags, comp, csize, usize, fn_len,
ex_total_len, filename, ex_id, ex_len, ex_usize, ex_csize, cd_sig
) = struct.unpack("<4sBBHH8xIIHH8shhQQx4s", zipdata[:63])

self.assertEqual(header, b"PK\x03\x04") # local file header
self.assertGreaterEqual(vers, zipfile.ZIP64_VERSION) # requires zip64 to extract
self.assertEqual(os, 0) # compatible with MS-DOS
self.assertEqual(flags, 0) # no flags
self.assertEqual(comp, 0) # compression method = stored
self.assertEqual(csize, 0xFFFFFFFF) # sizes are in zip64 extra
self.assertEqual(usize, 0xFFFFFFFF)
self.assertEqual(fn_len, 8) # filename len
self.assertEqual(ex_total_len, 20) # size of extra records
self.assertEqual(ex_id, 1) # Zip64 extra record
self.assertEqual(ex_len, 16) # 16 bytes of data
self.assertEqual(ex_usize, 1) # uncompressed size
self.assertEqual(ex_csize, 1) # compressed size
self.assertEqual(cd_sig, b"PK\x01\x02") # ensure the central directory header is next

z = zipfile.ZipFile(io.BytesIO(zipdata))
zinfos = z.infolist()
self.assertEqual(len(zinfos), 1)
self.assertGreaterEqual(zinfos[0].extract_version, zipfile.ZIP64_VERSION) # requires zip64 to extract

def test_unseekable_zip_unknown_filesize(self):
"""Test that creating a zip with/without seeking will raise a RuntimeError if zip64 was required but not used"""

def make_zip(fp):
with zipfile.ZipFile(fp, mode="w", allowZip64=True) as zf:
with zf.open("text.txt", mode="w", force_zip64=False) as zi:
zi.write(b"_" * (zipfile.ZIP64_LIMIT + 1))

self.assertRaises(RuntimeError, make_zip, io.BytesIO())
self.assertRaises(RuntimeError, make_zip, Unseekable(io.BytesIO()))

def test_zip64_required_not_allowed_fail(self):
"""Test that trying to add a large file to a zip that doesn't allow zip64 extensions fails on add"""
def make_zip(fp):
with zipfile.ZipFile(fp, mode="w", allowZip64=False) as zf:
# pretend zipfile.ZipInfo.from_file was used to get the name and filesize
info = zipfile.ZipInfo("text.txt")
info.file_size = zipfile.ZIP64_LIMIT + 1
zf.open(info, mode="w")

self.assertRaises(zipfile.LargeZipFile, make_zip, io.BytesIO())
self.assertRaises(zipfile.LargeZipFile, make_zip, Unseekable(io.BytesIO()))

def test_unseekable_zip_known_filesize(self):
"""Test that creating a zip without seeking will use zip64 extensions if the file size is provided up-front"""

# This test ensures that the zip will use a zip64 data descriptor (same
# as a regular data descriptor except the sizes are 8 bytes instead of
# 4) record to communicate the size of a file if the zip is being
# written to an unseekable stream.
# Because this sort of thing is hard to verify by parsing the data back
# in as a zip, this test looks at the raw bytes created to ensure that
# the correct data has been generated.
# The spec for this can be found at: https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
# The relevent sections for this test are:
# - 4.3.7 for local file header
# - 4.3.9 for the data descriptor
# - 4.5.3 for zip64 extra field

file_size = zipfile.ZIP64_LIMIT + 1

def make_zip(fp):
with zipfile.ZipFile(fp, mode="w", allowZip64=True) as zf:
# pretend zipfile.ZipInfo.from_file was used to get the name and filesize
info = zipfile.ZipInfo("text.txt")
info.file_size = file_size
with zf.open(info, mode="w", force_zip64=False) as zi:
zi.write(b"_" * file_size)
return fp

# check seekable file information
seekable_data = make_zip(io.BytesIO()).getvalue()
(
header, vers, os, flags, comp, csize, usize, fn_len,
ex_total_len, filename, ex_id, ex_len, ex_usize, ex_csize,
cd_sig
) = struct.unpack("<4sBBHH8xIIHH8shhQQ{}x4s".format(file_size), seekable_data[:62 + file_size])

self.assertEqual(header, b"PK\x03\x04") # local file header
self.assertGreaterEqual(vers, zipfile.ZIP64_VERSION) # requires zip64 to extract
self.assertEqual(os, 0) # compatible with MS-DOS
self.assertEqual(flags, 0) # no flags set
self.assertEqual(comp, 0) # compression method = stored
self.assertEqual(csize, 0xFFFFFFFF) # sizes are in zip64 extra
self.assertEqual(usize, 0xFFFFFFFF)
self.assertEqual(fn_len, 8) # filename len
self.assertEqual(ex_total_len, 20) # size of extra records
self.assertEqual(ex_id, 1) # Zip64 extra record
self.assertEqual(ex_len, 16) # 16 bytes of data
self.assertEqual(ex_usize, file_size) # uncompressed size
self.assertEqual(ex_csize, file_size) # compressed size
self.assertEqual(cd_sig, b"PK\x01\x02") # ensure the central directory header is next

# check unseekable file information
unseekable_data = make_zip(Unseekable(io.BytesIO())).fp.getvalue()
(
header, vers, os, flags, comp, csize, usize, fn_len,
ex_total_len, filename, ex_id, ex_len, ex_usize, ex_csize,
dd_header, dd_usize, dd_csize, cd_sig
) = struct.unpack("<4sBBHH8xIIHH8shhQQ{}x4s4xQQ4s".format(file_size), unseekable_data[:86 + file_size])

self.assertEqual(header, b"PK\x03\x04") # local file header
self.assertGreaterEqual(vers, zipfile.ZIP64_VERSION) # requires zip64 to extract
self.assertEqual(os, 0) # compatible with MS-DOS
self.assertEqual("{:b}".format(flags), "1000") # streaming flag set
self.assertEqual(comp, 0) # compression method = stored
self.assertEqual(csize, 0xFFFFFFFF) # sizes are in zip64 extra
self.assertEqual(usize, 0xFFFFFFFF)
self.assertEqual(fn_len, 8) # filename len
self.assertEqual(ex_total_len, 20) # size of extra records
self.assertEqual(ex_id, 1) # Zip64 extra record
self.assertEqual(ex_len, 16) # 16 bytes of data
self.assertEqual(ex_usize, 0) # uncompressed size - 0 to defer to data descriptor
self.assertEqual(ex_csize, 0) # compressed size - 0 to defer to data descriptor
self.assertEqual(dd_header, b"PK\07\x08") # data descriptor
self.assertEqual(dd_usize, file_size) # file size (8 bytes because zip64)
self.assertEqual(dd_csize, file_size) # compressed size (8 bytes because zip64)
self.assertEqual(cd_sig, b"PK\x01\x02") # ensure the central directory header is next


@requires_zlib()
class DeflateTestZip64InSmallFiles(AbstractTestZip64InSmallFiles,
Expand Down
32 changes: 17 additions & 15 deletions Lib/zipfile.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,12 @@ def __repr__(self):
return ''.join(result)

def FileHeader(self, zip64=None):
"""Return the per-file header as a bytes object."""
"""Return the per-file header as a bytes object.

When the optional zip64 arg is None rather than a bool, we will
decide based upon the file_size and compress_size, if known,
False otherwise.
"""
dt = self.date_time
dosdate = (dt[0] - 1980) << 9 | dt[1] << 5 | dt[2]
dostime = dt[3] << 11 | dt[4] << 5 | (dt[5] // 2)
Expand All @@ -451,16 +456,13 @@ def FileHeader(self, zip64=None):

min_version = 0
if zip64 is None:
# We always explicitly pass zip64 within this module.... This
# remains for anyone using ZipInfo.FileHeader as a public API.
zip64 = file_size > ZIP64_LIMIT or compress_size > ZIP64_LIMIT
if zip64:
fmt = '<HHQQ'
extra = extra + struct.pack(fmt,
1, struct.calcsize(fmt)-4, file_size, compress_size)
if file_size > ZIP64_LIMIT or compress_size > ZIP64_LIMIT:
if not zip64:
raise LargeZipFile("Filesize would require ZIP64 extensions")
# File is larger than what fits into a 4 byte integer,
# fall back to the ZIP64 extension
file_size = 0xffffffff
compress_size = 0xffffffff
min_version = ZIP64_VERSION
Expand Down Expand Up @@ -1183,6 +1185,12 @@ def close(self):
self._zinfo.CRC = self._crc
self._zinfo.file_size = self._file_size

if not self._zip64:
if self._file_size > ZIP64_LIMIT:
raise RuntimeError("File size too large, try using force_zip64")
if self._compress_size > ZIP64_LIMIT:
raise RuntimeError("Compressed size too large, try using force_zip64")

# Write updated header info
if self._zinfo.flag_bits & _MASK_USE_DATA_DESCRIPTOR:
# Write CRC and file sizes after the file data
Expand All @@ -1191,13 +1199,6 @@ def close(self):
self._zinfo.compress_size, self._zinfo.file_size))
self._zipfile.start_dir = self._fileobj.tell()
else:
if not self._zip64:
if self._file_size > ZIP64_LIMIT:
raise RuntimeError(
'File size unexpectedly exceeded ZIP64 limit')
if self._compress_size > ZIP64_LIMIT:
raise RuntimeError(
'Compressed size unexpectedly exceeded ZIP64 limit')
# Seek backwards and write file header (which will now include
# correct CRC and file sizes)

Expand Down Expand Up @@ -1633,8 +1634,9 @@ def _open_to_write(self, zinfo, force_zip64=False):
zinfo.external_attr = 0o600 << 16 # permissions: ?rw-------

# Compressed size can be larger than uncompressed size
zip64 = self._allowZip64 and \
(force_zip64 or zinfo.file_size * 1.05 > ZIP64_LIMIT)
zip64 = force_zip64 or (zinfo.file_size * 1.05 > ZIP64_LIMIT)
if not self._allowZip64 and zip64:
raise LargeZipFile("Filesize would require ZIP64 extensions")

if self._seekable:
self.fp.seek(self.start_dir)
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2023-04-25-19-58-13.gh-issue-103861.JeozgD.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix ``zipfile.Zipfile`` creating invalid zip files when ``force_zip64`` was
used to add files to them. Patch by Carey Metcalfe.
Back | FazBrowse Home | New Git URL