Repository navigation
zipfile to handle duplicate files in archive #47073
Description
| BPO | 2824 |
|---|---|
| Nosy | @birkenfeld, @serhiy-storchaka, @iritkatriel, @obfusk, @jheiselman |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2008-05-11.20:17:49.633>
labels = ['extension-modules', 'type-feature', '3.9', '3.10', '3.11']
title = 'zipfile to handle duplicate files in archive'
updated_at = <Date 2021-08-02.22:01:33.012>
user = 'https://bugs.python.org/techtonik'bugs.python.org fields:
activity = <Date 2021-08-02.22:01:33.012>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2008-05-11.20:17:49.633>
creator = 'techtonik'
dependencies = []
files = ['16776', '16843']
hgrepos = []
issue_num = 2824
keywords = ['patch']
message_count = 13.0
messages = ['66661', '66682', '66728', '66781', '89468', '102439', '102578', '102600', '102721', '181042', '383102', '385801', '385802']
nosy_count = 11.0
nosy_names = ['georg.brandl', 'alanmcintyre', 'techtonik', 'BitTorment', 'brtzsnr', 'Yaniv.Aknin', 'serhiy.storchaka', 'michael.driscoll', 'iritkatriel', 'obfusk', 'jheiselman']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue2824'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
Activity
techtonik commentedon May 11, 2008
ZipFile allows to add the same file to archive twice. I bet it is not
intended behavior for many users who would like to either replace file
inside of archive or get runtime warning about duplicate file to be
added. http://code.google.com/p/gvn/issues/detail?id=63
birkenfeld commentedon May 11, 2008
I think a warning would be sensible here. The behavior is certainly not
what I would expect.
techtonik commentedon May 12, 2008
How about adding optional "replace=True" attribute to the write method?
So that people who are not aware enough to adjust the code and handle
new warning could still get valid archives.
BitTorment commentedon May 13, 2008
The mechanism for throwing an error has been written, however for the
case of a duplicated filename, it appears to have been deliberatly not
been used by the original author.:
Putting a 'replace=True' switch seems a little clumsy, it would be much
better to raise an error, or allow the user a way to globally control
what happens in this case, i.e. overwrite the existing file or drop it.
Adding a global behaviour switch seems to be the best way to preserve
backwards compatibility.
What do people think is the best way?
-- Martin
brtzsnr commentedon Jun 17, 2009
a similar problem appears when the zip file contains two files as in:
['_test/tree.pl', '_test/'].
for ZipFile.extractall() when _test/tree.pl is extracted _test/ is
created and the the extraction of _test/ fails because OSError: [Errno
17] File exists: '_test/'
techtonik commentedon Apr 6, 2010
Still an issue for Python 2.7
YanivAknin commentedon Apr 8, 2010
This affect 3.x as well.
Regardless of the exact version this will come out in, I think the only proper solution is one with which we eventually, maybe in two versions' time, end up with a behaviour that raises an exception upon double insertion. Also, we should leave some switch to allow double insertions (as someone may /want/ to create a peculiar file). If we agree that's a worthy end goal, let's see what deprecation path we take to get there.
I feel PEP4/PEP5 cover more significant changes to the language/stdlib than this, but in their spirit I propose the following concrete solution: For the nearest possible version let's issue a DeprecationWarning when adding dual files and provide a global switch that will change the DeprecationWarning into raising a relevant exception instead. For the version after that, let's remove the DeprecationWarning warning altogether and choose one of these behaviours:
(a) a global switch that will raise by default
(b) a switch in ZipFile's __init__() that will raise by default
(c) a switch in ZipFile's write() that will raise by default
I vote for (c), but I think the delta between the options is small.
If this is agreeable with the audience, I'll be happy to produce a patch against 2.7 and 3.2 that will provide the chosen behaviour, as well as change tests and documentation accordingly. I realize that time is of the essence if we want this included in 2.7, and I'll be happy to do the work in a timely manner, so if you're interested kindly let me know what you feel while the iron's hot. Either way, I'll be happy to see it in any future version.
Regarding the behaviour brtzsnr observed, I think it's a separate bug. I'll try to reproduce this behaviour, and open a separate ticket if I manage to do so.
blah. Small bug, many words!
techtonik commentedon Apr 8, 2010
On the second thought having a switch for a low-level zip format hacking is a good addition. It may also be used for in-place removals from .zip file by erasing directory entry. Otherwise remove operation will require repacking archive into temporary file, which is too high level and vulnerable to disk space/permission matters.
So, I vote for:
(b) a switch in ZipFile's __init__() that will raise by default
and also because you'd need to modify write() and writestr().
class zipfile.ZipFile(file[, mode[, compression[, allowZip64, [low_level]]]])
"""Added in version 2.7: If
low_levelcompatibility switch is set (False by default), then ZipFile allows low level operations, such as adding two files with the same name without raising an exception. This is for compatibility with previous versions."""BTW, allowZip64 would be nice to be aliased/deprecated in favor of allow_zip64 for consistency with other double word params in module.
YanivAknin commentedon Apr 9, 2010
Attached is the addition of the 'low_level' parameter to ZipFile. Included are the parameter, a global switch controlling whether the parameter will raise an Exception or trigger a DeprecationWarning (the latter, for now), updated tests and updated documents. I didn't run test_zipfile64, I can't say for sure it works but I have no reason to believe it doesn't.
Changes are against py3k; let me know what you think, I'll backport to trunk.
michaeldriscoll commentedon Jan 31, 2013
Whatever became of this? We just stumbled across this bug in our code at work where we accidentally put multiple files of the same name into the zip file and not only does it not overwrite the others, but when you go to access that file name, it grabs the first one that was put in.
iritkatriel commentedon Dec 15, 2020
This was changed to a warning in bpo-20262. Is that enough or should it be an exception?
jheiselman commentedon Jan 27, 2021
This just bit us too. I don't feel like a warning is enough. In our case, we want to prevent it from happening to begin with so that a developer who isn't expecting this doesn't have to know ahead of time to check for a file.
Once a duplicate is in the zip file, it is maintained in the listing of files, but deleting one of the entries using Windows explorer results in both entries being deleted which can (and has) led to a loss of data. So preventing it from happening in the first place seems more prudent. Of course, allow an override with a parameter to .write() perhaps.
jheiselman commentedon Jan 27, 2021
Further, some tools like zipgrep, seem to iterate over the toc in the zipfile and end up running the grep part once per entry leading to some duplication of data returned without it being obvious that there wasn't actually duplicate data in the zip archive.
Interestingly, the zipfile module does not expose the duplicate toc entries using 'python -m zipfile -l <file.zip>', so it's hiding the issue itself.