Skip to content
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.

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

[PATCH] zipfile.ZipFile does not extract directories properly #48960

Closed
faw mannequin opened this issue Dec 21, 2008 · 7 comments
Closed

[PATCH] zipfile.ZipFile does not extract directories properly #48960

faw mannequin opened this issue Dec 21, 2008 · 7 comments
Labels
release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@faw
Copy link
Mannequin

faw mannequin commented Dec 21, 2008

BPO 4710
Nosy @loewis
Files
  • zipfile_extract_dirs.patch: patch
  • zipfile_dirs_support.patch: patch r2
  • dir.diff: r3
  • zipdir.zip: Extraction test data (goes into Lib/test)
  • 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']

    @faw
    Copy link
    Mannequin Author

    faw mannequin commented 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).

    @faw faw mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Dec 21, 2008
    @koen
    Copy link
    Mannequin

    koen mannequin commented 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
    Copy link
    Mannequin Author

    faw mannequin commented 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
    Copy link
    Mannequin Author

    faw mannequin commented 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
    Copy link
    Mannequin

    ggenellina mannequin commented 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 loewis mannequin added the release-blocker label Dec 30, 2008
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented 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
    Copy link
    Mannequin

    loewis mannequin commented Jan 24, 2009

    Committed as r68885, r68886, r68887, and r68888

    @loewis loewis mannequin closed this as completed Jan 24, 2009
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants