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

Exception when using strip:1 #25

Closed
wants to merge 1 commit into from

Conversation

georgzoeller
Copy link

decompress-zip:0.0.6: Extracting a simple zip file with one level of empty directory at the root will throw an TypeError on path.join decompress-zip.js:259 if strip:1 is set

DecompressZip.prototype.extractFile = function (file, options) {
var destination = path.join(options.path, file.path);

The issue is in decompress-zip.js:109..120, which, instead of adding .path to the file entry, is returning the path directly in files.map().

files = files.map(function (file) {
                if (file.type !== 'Directory') {
...
                    return file.path //<-- this is replacing file with file.path in the map.
                    // should be:
                    //   file.path = path.join(dir.join(path.sep), filename);
                    //   return file

                }

Note: The current tests do not detect the issue as they do not validate the feature at all - they just test strip to see if an exception is thrown for the 'stripping too deep' case. Ideally, the stripping too deep test case should validate the exact exception expected and a new test case with strip:1 should be added.

… root will throw an TypeError on path.join decompress-zip.js:259 if strip:1 is set

```
DecompressZip.prototype.extractFile = function (file, options) {
var destination = path.join(options.path, file.path);

```
The issue is in decompress-zip.js:109..120, which, instead of adding .path to the file entry, is returning the path directly in files.map().

```
files = files.map(function (file) {
                if (file.type !== 'Directory') {
...
                    return file.path //<-- this is replacing file with file.path in the map.
                    // should be:
                    //   file.path = path.join(dir.join(path.sep), filename);
                    //   return file

                }
```
@georgzoeller georgzoeller changed the title Exeption when using strip:0 Exception when using strip:0 May 2, 2014
@georgzoeller georgzoeller changed the title Exception when using strip:0 Exception when using strip:1 May 5, 2014
@georgzoeller
Copy link
Author

On further inspection, it is becoming more clear that this code has never run, there's more wrong with it.

The if () filter inside the .map function generates undefined entries in the returned path array for any directory in the zip structure, so even with my fix, only zip files that don't have a sub directory structure will successfully extract. All other's will fail further down the chain when file.path is tested against the first undefined entry.

tomas added a commit to tomas/decompress-zip that referenced this pull request May 8, 2014
@wenbing
Copy link

wenbing commented Jul 28, 2014

I got this problem too

@sindresorhus
Copy link
Contributor

#31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants