Make it easier to extend zipfile code

Feature or enhancement

Proposal:

This is follow up from #53416

Even if stdlib will never support WinZIP AES encrpyption, it would be useful to allow for downstream project to implement it.

There is pyzipper based on a fork but the code is now very different to what we have in stdlib.

it would help if the stdlib zipfile code is written to allow dependency injection

A very simple proof of concept downstream code is here that can only read AES ZIP files is here

https://github.com/chevah/zipfile-aes

It is using a patch for stdblib that looks like this

--- lib/python3.12/zipfile/__init__.py        2025-07-17 11:19:59.376677338 +0100
+++ src/dev_tools/zipfile_init.py       2025-07-17 11:21:50.797894299 +0100
@@ -1286,6 +1286,9 @@
 
     """
 
+    _ZipInfo = ZipInfo
+    _ZipExtFile = ZipExtFile
+
     fp = None                   # Set here since __del__ checks it
     _windows_illegal_name_trans_table = None
 
@@ -1456,7 +1459,7 @@
                 # Historical ZIP filename encoding
                 filename = filename.decode(self.metadata_encoding or 'cp437')
             # Create ZipInfo instance to store file information
-            x = ZipInfo(filename)
+            x = self._ZipInfo(filename)
             x.extra = fp.read(centdir[_CD_EXTRA_FIELD_LENGTH])
             x.comment = fp.read(centdir[_CD_COMMENT_LENGTH])
             x.header_offset = centdir[_CD_LOCAL_HEADER_OFFSET]
@@ -1592,11 +1595,11 @@
                 "Attempt to use ZIP archive that was already closed")
 
         # Make sure we have an info object
-        if isinstance(name, ZipInfo):
+        if isinstance(name, self._ZipInfo):
             # 'name' is already an info object
             zinfo = name
         elif mode == 'w':
-            zinfo = ZipInfo(name)
+            zinfo = self._ZipInfo(name)
             zinfo.compress_type = self.compression
             zinfo._compresslevel = self.compresslevel
         else:
@@ -1664,7 +1667,7 @@
             else:
                 pwd = None
 
-            return ZipExtFile(zef_file, mode, zinfo, pwd, True)
+            return self._ZipExtFile(zef_file, mode, zinfo, pwd, True)
         except:
             zef_file.close()
             raise
@@ -1762,7 +1765,7 @@
         """Extract the ZipInfo object 'member' to a physical
            file on the path targetpath.
         """
-        if not isinstance(member, ZipInfo):
+        if not isinstance(member, self._ZipInfo):
             member = self.getinfo(member)
 
         # build the destination pathname, replacing
@@ -1838,7 +1841,7 @@
                 "Can't write to ZIP archive while an open writing handle exists"
             )
 
-        zinfo = ZipInfo.from_file(filename, arcname,
+        zinfo = self._ZipInfo.from_file(filename, arcname,
                                   strict_timestamps=self._strict_timestamps)
 
         if zinfo.is_dir():
@@ -1868,8 +1871,8 @@
         the name of the file in the archive."""
         if isinstance(data, str):
             data = data.encode("utf-8")
-        if not isinstance(zinfo_or_arcname, ZipInfo):
-            zinfo = ZipInfo(filename=zinfo_or_arcname,
+        if not isinstance(zinfo_or_arcname, self._ZipInfo):
+            zinfo = self._ZipInfo(filename=zinfo_or_arcname,
                             date_time=time.localtime(time.time())[:6])
             zinfo.compress_type = self.compression
             zinfo._compresslevel = self.compresslevel
@@ -1910,7 +1913,7 @@
             directory_name = zinfo_or_directory_name
             if not directory_name.endswith("/"):
                 directory_name += "/"
-            zinfo = ZipInfo(directory_name)
+            zinfo = self._ZipInfo(directory_name)
             zinfo.compress_size = 0
             zinfo.CRC = 0
             zinfo.external_attr = ((0o40000 | mode) & 0xFFFF) << 16

Would you consider such a patch ?

Thanks

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

#53416

#81719

PR at #14957

AES encryption support sounds useful. I'd like to see this.
I wanted to chime in that the tarfile module has something similar. tarfile.TarInfo.__init__ takes a tarinfo=TarInfo default argument, so there is a precedent for something like this.
But it is not documented well enough what API the class given to tarinfo has to provide. For example, TarFile calls self.tarinfo.create_pax_global_header even though create_pax_global_header is not documented.

These changes would be useful for me maintaining pyzipper. In pyzipper, I also override the _ZipWriteFile class and I think it would make sense to allow subclasses to override the _ZipWriteFile class to modify the way zipfile writes too.

I understand that the underscore in _ZipWriteFile means it is a private class but I would be happy supporting it in pyzipper acknowledging that the underlying API may change. It would still greatly simplify maintenance. I'm also happy to focus just on the read aspects in this issue and have the discussion about allowing subclasses to override _ZipWriteFile in a different issue if that would require more discussion.

Thanks Daniel for the feedback.

This looks like a duplicate for #81719

I haven't looked into the WinZip AES write part yet ... but I think that it makes sense to include the write part in the same PR.

It's critical to get this into small PRs to increase the probabily of getting the code reviewed and approded/merged

My proof of concept WinZip AES code only implemens WinZip AE2 ... without CRC checks.

I will try to get my proof of concept updated to support WinZip AE1 and also write operations and then we can see how big of a PR I get.

With that, we see if we can get one PR or break it into multiple PRs.

@serhiy-storchaka

This sounds reasonable conceptually, but I think the proposed solution isn't the right way to do it. Instead of class variables, why not add a zipfile_class kwarg to methods using it? Alternatively, we could store the class as an instance attribute and pass it from the __init__. I don't know if we have a precedenting design for this anywhere in the standard library.

cc @serhiy-storchaka as a zipfile expert and nosy on #81719

@jaraco

cc @jaraco

Thanks for the feedback.

I don't have any preference for class members vs instance members set at init.

The current diff is a proof of concept to see if Python stdlib developers want to support this type of downstream usage

I am happy to send a PR with any option.

With this patch I was able to implement basic support for reading and writing AES encrypted ZIP files.

The read part is not that bad, as the stdlib code already has the handling of encrypted files.

The write part requires more code duplication, as stdlib does not support writing encrypted zip files.

proof of concept code at https://github.com/chevah/zipfile-aes/blob/main/src/zipfile_aes/__init__.py

My suggestion for an implementation plan is:

Get this minimun patch in stdlib. Close this issue as duplicate of Refactor zipfile to ease subclassing and enhancement #81719
In a separate ticket, add support in stdlib for zip encryption (standard zip, not AES). As part if this work, see how the code can be refactored to make it easier to implement WinZip AES downstream.

I am happy to send a PR.

I am waiting for feedback to see wheter the fix should be done via class members or init arguments.

I am ok with any option.

I am +1 on doing it through __init__ args. That makes it possible to have different target classes for each instance.

I have created a PR at #137101

the PR was created for the older issue #81719

My plan is to get the minimum patch accepted

The other refactoring to make it easier to re-use code are nice to have... and maybe can be done in a separate PR that will be associated with this issue

Thanks

I would like to add a suggestion, I once came across a problem that zipfile would raise an error where the actual filename and in the header differed (if fname_str != zinfo.orig_filename:) which makes opening arbitrary zip files impossible, even though such error does not mean data corruption or anything, and there was no other possible solution than monkey-patching. I believe making the module more lenient would allow much wider usage, similar to how we can specify how to handle unicode errors when decoding bytes.

@IamMusavaRibica

@IamMusavaRibica ... I think that this needs to be handled in a separate issue and PR.

I saw something similar when writing automated tests...but I didn't had much time to investigate this error.

Do you have an example code to reproduce this?

Which software was used to create the source ZIP file ?

This looks reasonable.

Is is fine to have class variable or methods purposed for overriding in a subclass. Constructor argument is not needed -- you cannot make ZipFile supporting AES encryption by just passing a custom ZipInfo subclass. This is not what users want to do. A ZipFile subclass which implements all necessary hooks would be more convenient. Of course, you cannot combine several ZipFile subclasses which provide different extended functions, but I don't think this is needed. And it will be easier to include support of AES encryption in the base class.

But I have several general notes:

The isinstance() checks should check against ZipInfo. Basic ZipInfo should still be accepted in these functions. Creation of new ZipInfo objects can be customized.
Instead of self._ZipInfo.from_file(), call a special ZipFile method. This is more flexible.
Instead of _ZipInfo, _ZipExtFile and _ZipWriteFile use methods with more meaningful name and maybe with better designed signature. For example, ZipExtFile() is always called with mode='rb' and close_fileobj=True. They should not be parameters of the method that creates ZipExtFile.

@adiroiban

@adiroiban

Do you have an example code to reproduce this?

Here. (archived, just in case)

import os, zipfile

with zipfile.ZipFile(os.path.expanduser(r'~\Downloads\PROIZVODI-2025-06-29.zip')) as zf:
    for i in zf.namelist():
        with zf.open(i) as f:
            pass

Which software was used to create the source ZIP file ?

Not sure, unfortunately. But it isn't hard to create a normal one and then edit it by hand to create such zip file.

@serhiy-storchaka

I like @serhiy-storchaka's idea of using methods to return the instances of ZipExtFile, _ZipWriteFile and ZipInfo or their subclasses as it gives us a location to do custom setup in ZipFile before creating the instance.

For the method names, would any of the following work:

init_extract_obj(zinfo, zip64)
init_write_obj(fileobj, zinfo, pwd)
init_zipinfo(filename) or create_zipinfo(filename) because the zipinfo is fully created vs the other two where those have been initialised and are ready to read / write?

I'm not attached to those names so happy for you to choose anything :p

For the AES implementation, we need to be able to pass the password through to the subclass but other people may need to pass other values through to support different features they want. Should we also tackle how we pass extra kwargs to the custom subclasses from the various ZipFile methods that may return them in this PR (eg. open(), write(), writestr() may all return _ZipWriteFile and ZipExtFile is similar)? The obvious approach I can think of would be to add a **kwargs term to these method signatures. The new methods that create the instances should not have a **kwargs term so we catch typos and incorrect parameters for the particular implementation, instead of gobbling them with no error. Is that an acceptable approach for the stdlib API?

As for names, "init" implies that there is already uninitialized object. I prefer prefixes "make", "create" or "build". For now, all these methods should be private, to not clutter the interface for common users. I would avoid too general "obj" and used "file" or "stream". Or "reader" and "writer".

Currently, ZipInfo(filename) creates partially initialized ZipInfo object. And then its attributes are changed. It would be better to pass all or most necessary data to the factory method.

For now, all these methods should be private, to not clutter the interface for common users.

This sounds like a documentation problem. I really don't think we should be encouraging people to use private APIs.

I have been thinking a lot about this for a few different use cases/feature requests across the bug tracker, and I wanted to take a step back and look at a few feature requests such as this one and others such as #113767

I also think it is important we design a composable/interoperable system. For example, we want to allow AES encrypted deflate64 entries even if the each of those are provided by independent 3rd party packages.

I have a design in mind that I want to sketch out more and make a write up on discuss to get feedback, I'll link back here.

Thanks for looking into this.

Other than the AES encryption and Zstandard compressions , are there any other use cases for 3rd party extenstions ?

I think that it would help to have a list of use cases so that we can validate the new design.

Cheers

@gpshead

Other than the AES encryption and Zstandard compressions , are there any other use cases for 3rd party extenstions ?

Well, zstandard compression is now in the stdlib and supported for zipfile: https://docs.python.org/3/library/compression.zstd.html. I think deflate64 is perhaps a better example of a compression format people want added since it is generated by Windows (see #134334). There's also been requests for older methods to be supported which was closed: #103412, I expect those users would be happy to have a third party library provide the older compression formats.

As for encryption, I expect AES to be the most popular, but given people want support for older compression methods, I'd expect being able to read older encryption methods would probably be useful too.

The other consideration is that PKWare can always add new features to ZIP, so ensuring flexibility in compression and decryption schemes would make it easier to add new features.

I think that it would help to have a list of use cases so that we can validate the new design.

The other features that I am aware users want would be to customize arbitrary compression settings (currently they can customize the compression level for zlib and bz2, but not lzma or zstd. I expect users will eventually want to use e.g. zstd or zlib dictionaries, or other advanced settings). To accomplish this, I think the way (de)compression is handled needs to be a bit more generic than today.

Finally, the current zipfile code locks on every read to the underlying file object (in _SharedFile). pip found some performance improvements in a test removing these locks (among other improvements): pypa/pip#12826. Users should be able to use a subclass of ZipExtFile to lock for the entire lifetime of the ZipExtFile to reduce this overhead if that better fits their I/O patterns (i.e. read large chunks from large entries directly).

I think I'll go ahead and describe my design at a higher level (I don't have an implementation quite yet), but here's a sketch:

Make a DeflateFile class implementing BufferedIOBase similar to LZMAFile, BZ2File, etc. (this is probably useful anyway)
Add registries for compression and decompression implementations similar to gh-113767: Adds a zipfile.register_compressor() API. #113768 (the registries will be independent, as someone may want to only implement one or the other)
Add a registry for decryption implementations
ZipExtFile also now drives decryption from the decryption registry
Eliminate _SharedFile and move the locking to ZipExtFile. ZipExtFile now drives a decompressor I/O object (as well as decryptor I/O object) with proper locking per method (similar to _SharedFile, but probably fewer lock acquires overall). Users can override the ZipExtFile class to modify how locking is done (e.g. per instance locking)
ZipExtFile now handles parsing the local file header (currently done in open()). This is needed to eliminate _SharedFile, and will allow further customization when unpacking zip file entries
Allow users to specify their own ZipExtFile subclasses to be used in ZipFile.open() for further customization

Operating on file I/O solves an issue with #113767 which is that the compressors/decompressors don't all share the same API surface. Using a file I/O wrapper for (de)compression is a natural, well understood API boundary, and would allow overriding default settings for (de)compression. (e.g. you could override the ZIP_LZMA entry with a LZMAFile with custom presets). It would also reduce a lot of the logic needed in ZipExtFile, which re-implements the *File implementations.

Finally, this would allow unifying ZipExtFile with _ZipWriteFile, since they would both operate on entries from the (de)compression registries.

I realize this is a pretty major refactor to zipfile, but it would make the API have a number of escape hatches that would allow composable customization of zipfile for 3rd party libraries, and I think would also make the code simpler. I want to make sure that the *File APIs are performant enough, and can be used instead of the (de)compressors directly. If not, we can always define a protocol that must be implemented and use (de)compressors directly.

I've started working on an implementation but curious what folks think.

cc @gpshead since you originally were looking at the compression registry for zipfile. Curious if you have thoughts on the proposal

cc gpshead since you originally were looking at the compression registry for zipfile. Curious if you have thoughts on the proposal

You've thought well beyond what I ever had. 😁 random things to add:

additional/alternate example use case to add: replacing existing supported compression method implementations with third party enhanced versions of those. third parties providing solutions to Support linking against zlib-rs and libbzip2-rs #139314 and Use zlib-ng (fast!) rather than mainline stale zlib in binary releases #91349 for example (even if we do eventually get to those examples ourselves). (this is probably equivalent implementation wise to your idea of allowing ways to parameterize options on existing ones like zlib or zstd though)
From past experience in a build farm distributed action scenario: Being able to accept pre-compressed, pre-crc'd inputs for zip file entries is worthwhile. In highly parallel build scenarios it is feasible to have independent build actions go ahead and do the appropriate compression of many of the data members feeding into an ultimate .zip creation action so that the final assembly of the zip file, perhaps via the Python zipfile module, is significantly just feeding some already compressed+crc'd blobs into the right place within the ultimate output zip structure. Instead of trying to do serial member compression on its own. (even if we locally parallelized compression of individual members - that belongs in its own feature request, that'd never be as powerful as a distributed build farm and would always be a scheduling challenge to give the an arbitrary zip creation an appropriate number cores and temporary space to do that parallelization itself)

You've thought well beyond what I ever had. 😁

Well I am glad the proposal seems good :)

For 1, replacing existing/default compression methods is part of the design I had in mind. The defaults for compression in the stdlib will match what we have today (e.g. lzma with the current presets), but you can always override them with an API like

COMPRESSION_REGISTRY.register(ZIP_LZMA, lzma.LZMAFile, flag_bits=1 << 1, **my_kwargs_for_LZMAFile)

I also want to check with pip folks to see if a) they would want to use this and b) if so, are there security considerations to made. We may need to make it easy to opt-out of overriding default compression methods.

For 2, I think modifying ZipExtFile should allow for this. You could have code like:

class DirectWriteZipExtFile(ZipExtFile):
    # initialization etc excluded for brevity
    # set_metadata is a custom function that just attaches the metadata to class to be used in close()
    def set_metadata(self, metadata): ...
    def write(self, data):
        # data is already compressed
        self.fileobj.write(data)
    def close(self):
        BufferedIOBase.close(self)
        # ... generate zip header byte stream from metadata (crc, compressed size, file size)
        self.fileobj.write(zinfo_bytes)

with ZipFile.open('myzip.zip', 'wb', ) as z:
    # pre_computed_data comes from the build farm
    for file_name, contents, metadata in pre_computed_data():
        with z.open(file_name, 'w', extfile_class=DirectWriteZipExtFile) as d:
            # d is now a DirectWriteZipExtFile
            d.set_metadata(metadata)
            d.write(contents)
        # d is closed and the header is written with the information already computed

Note the above API is just a sketch, I'm not sure if passing the ZipExtFile subclasses to ZipFile.open is the right granularity. Probably better to set it elsewhere but I'm still thinking about what is best.

Wanted to update that I do not expect to have time to work on this anytime soon (i.e. in the next 6-12 months). If someone wants to work on this I recommend looking at the comments in #137101 and above.

Navigation Menu

Repository navigation

Make it easier to extend zipfile code #136741

Feature or enhancement

Proposal:

Has this already been discussed elsewhere?

Links to previous discussion of this feature:

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Make it easier to extend zipfile code #136741

Description

Feature or enhancement

Proposal:

Has this already been discussed elsewhere?

Links to previous discussion of this feature:

Activity

mxmlnkn commented on Jul 17, 2025

danifus commented on Jul 18, 2025

adiroiban commented on Jul 18, 2025

ZeroIntensity commented on Jul 20, 2025

picnixz commented on Jul 21, 2025

adiroiban commented on Jul 21, 2025

adiroiban commented on Jul 21, 2025

ZeroIntensity commented on Jul 21, 2025

adiroiban commented on Jul 25, 2025

IamMusavaRibica commented on Jul 26, 2025

adiroiban commented on Jul 28, 2025

serhiy-storchaka commented on Jul 28, 2025

IamMusavaRibica commented on Jul 28, 2025

danifus commented on Jul 29, 2025

serhiy-storchaka commented on Jul 29, 2025

ZeroIntensity commented on Jul 29, 2025

emmatyping commented on Oct 16, 2025

adiroiban commented on Oct 16, 2025

emmatyping commented on Oct 17, 2025

gpshead commented on Oct 17, 2025

emmatyping commented on Oct 17, 2025

emmatyping commented on Dec 18, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions