Repository navigation
[PATCH] zipfile.ZipFile does not extract directories properly #48960
Description
| BPO | 4710 |
|---|---|
| Nosy | @loewis |
| 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 = <Date 2009-01-24.14:18:42.132>
created_at = <Date 2008-12-21.13:10:10.032>
labels = ['type-bug', 'library', 'release-blocker']
title = '[PATCH] zipfile.ZipFile does not extract directories properly'
updated_at = <Date 2009-01-24.14:18:42.131>
user = 'https://bugs.python.org/faw'bugs.python.org fields:
activity = <Date 2009-01-24.14:18:42.131>
actor = 'loewis'
assignee = 'none'
closed = True
closed_date = <Date 2009-01-24.14:18:42.132>
closer = 'loewis'
components = ['Library (Lib)']
creation = <Date 2008-12-21.13:10:10.032>
creator = 'faw'
dependencies = []
files = ['12413', '12435', '12604', '12605']
hgrepos = []
issue_num = 4710
keywords = ['patch', 'needs review']
message_count = 7.0
messages = ['78143', '78158', '78160', '78233', '78341', '79204', '80450']
nosy_count = 4.0
nosy_names = ['loewis', 'ggenellina', 'koen', 'faw']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue4710'
versions = ['Python 2.7']
Activity
faw commentedon Dec 21, 2008
This behaviour has been known of course for quite long time. I suppose
this is not intentional so I've played a bit with this and I hope you'll
consider some little change.
Currently, if a ZIP archive contains some subdirectories then
zipfile.ZipFile.extract()/extractall() will create files instead of
directories in the target location. This of course will lead some
scripts to crash (unless any work-around has been done) because files
from the subdirectories couldn't be created.
Attached is a patch against current 2.7 tree. Applied, will make
extractall() extract properly all the contents of the archive with
proper tree structure. If a directory name is passed to the extract(),
it will only create the directory itself without the contents (I guess
it is obvious).
koen commentedon Dec 21, 2008
I'm no expert, but is it possible for ZIP files to have Windows-style
path seperators ('\') as well?
And is this new behavior desirable for existing code as well? It might
break existing applications, so perhaps a new extractrecursive()
function is more intuitive.
Finally, I've noticed that the patch does not add any tests to test for
the new/old behavior.
faw commentedon Dec 21, 2008
I'm not sure if it would make sense to save current
extract()/extractall() behaviour and implement new with another function
because current one is simply faulty. And if it's about BC breaks then
well... it may be introduced in 3.0 line, I think that leaving faulty
behaviour and introducing another with another function misses the point.
AFAIK there'd be no problem with Windows-style paths but to be sure I
revised my patch.
Yes, I'm sorry for the lack of tests, it is my first patch and I forgot
about this. I'll write them in a couple of hours.
faw commentedon Dec 23, 2008
Here is a revised version of the patch with directories support for
write(). It works like UNIX zip program so if a directory path is passed
to the function, it stores only the directory itself (without the contents).
This time without tests as well because I don't want to mess up with
them until I finish my patch completely.
ggenellina commentedon Dec 27, 2008
Your usage of os.sep is incorrect, both when reading and writing
directories.
Zip files are (more-or-less) platform independent. The specification
*requires* forward slashes in paths [1], and the zipfile module
already writes them that way. Checking for os.sep is wrong - at least
on Windows.
I've never encountered malformed entries of that kind (like "directory
\" instead of "directory/") but if you want to suport such beasts,
check for "/" *and* os.sep explicitely.
[1] See APPNOTE.TXT (there is a link near the top of zipfile.py)
loewis commentedon Jan 5, 2009
Here is a version of the patch that rationalizes usage of os.sep, fixes
a few bugs with r2, and adds a test case.
loewis commentedon Jan 24, 2009
Committed as r68885, r68886, r68887, and r68888