Skip to content

Fix extract when fileName is a directory#7599

Closed
f111fei wants to merge 1 commit intomicrosoft:masterfrom
f111fei:patch-6
Closed

Fix extract when fileName is a directory#7599
f111fei wants to merge 1 commit intomicrosoft:masterfrom
f111fei:patch-6

Conversation

@f111fei
Copy link
Copy Markdown
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
Copy Markdown

mention-bot commented Jun 13, 2016

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
Closed
@joaomoreno
Copy link
Copy Markdown
Member

joaomoreno commented Jul 6, 2016

@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
Copy Markdown
Member

joaomoreno commented Jul 6, 2016

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.

Reviewers

No reviews

Assignees

@joaomoreno joaomoreno

Labels

None yet

Projects

None yet

Milestone

July 2016

Development

Successfully merging this pull request may close these issues.

4 participants

@f111fei @mention-bot @joaomoreno @msftclas