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

Fix extract when fileName is a directory #7599

Closed
wants to merge 1 commit into from

Conversation

f111fei
Copy link
Contributor

@f111fei f111fei commented Jun 13, 2016

See https://github.com/thejoshwolfe/yauzl/blob/master/README.md

  zipfile.on("entry", function(entry) {
    if (/\/$/.test(entry.fileName)) {
      // directory file names end with '/'
      mkdirp(entry.fileName, function(err) {
        if (err) throw err;
        zipfile.readEntry();
      });
    } else {
      ......
    }
  };

If entry.fileName is a directory name, extracting will be failed.

Here is a test file.
https://github.com/f111fei/test-files/raw/master/test.zip

The case has 4 entries.

/
/1/
/1/2/
/1/2/README.md

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @egamma and @joaomoreno to be potential reviewers

@joaomoreno joaomoreno added this to the June 2016 milestone Jun 13, 2016
@joaomoreno joaomoreno self-assigned this Jun 13, 2016
@joaomoreno joaomoreno modified the milestones: June 2016, July 2016 Jun 24, 2016
@joaomoreno joaomoreno mentioned this pull request Jun 24, 2016
@joaomoreno
Copy link
Member

@f111fei I took your suggestion from #8044 and started writing tests for zip: a11c9b2

The first (and only, so far) test tries to extract the file you appended to this PR. The test seemed to pass successfully... I then realised the file you provided didn't cause the issue. It needs to be a zip file containing an empty directory. I've updated the test and then it started failing.

@joaomoreno
Copy link
Member

Your fix got close, but there was still the problem of opening the stream of an empty directory: the extract would never end. I've pushed another commit on top.

For some idiotic reason I failed to properly preserve history and ended up not merging this properly and did a rebase instead. You still appear as a committer, though: 74b8236

Thanks for your help! 🎆 Great job! 🍻

@joaomoreno joaomoreno closed this Jul 6, 2016
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants