max-mapper / extract-zip

Zip extraction written in pure JavaScript. Extracts a zip into a directory.
BSD 2-Clause "Simplified" License
388 stars 126 forks source link

Error when extracting GitHub repo archive #13

Closed lukehorvat closed 8 years ago

lukehorvat commented 8 years ago

Trying to extract a repo archive downloaded from GitHub:

const fs = require("fs");
const got = require("got");
const extract = require("extract-zip");
const pify = require("pify");

got("https://codeload.github.com/maxogden/extract-zip/zip/master", { encoding: null })
.then(res => {
  return pify(fs.writeFile)("master.zip", res.body);
}).then(() => {
  return pify(extract)("master.zip", { dir: process.cwd() });
}).then(() => {
  console.log("Done!");
}).catch(err => {
  console.error(err);
});

Produces the following error:

Error: Callback was already called.
    at /Users/lukehorvat/code/test1/node_modules/async/lib/async.js:30:31
    at WriteStream.<anonymous> (/Users/lukehorvat/code/test1/node_modules/extract-zip/index.js:122:24)
    at emitOne (events.js:82:20)
    at WriteStream.emit (events.js:169:7)
    at WriteStream.<anonymous> (fs.js:1846:12)
    at FSReqWrap.oncomplete (fs.js:82:15)

I decided to print out the error above line 122 and it is as follows:

Error: ENOENT: no such file or directory, open '/Users/lukehorvat/code/test1/extract-zip-master/'

Any idea what's going on here?

max-mapper commented 8 years ago

Not sure whats happening. Could you run it with DEBUG=extract-zip node yourcode.js?

lukehorvat commented 8 years ago
extract-zip creating target directory +0ms /Users/lukehorvat/code/test1
extract-zip opening +3ms master.zip with opts { dir: '/Users/lukehorvat/code/test1' }
extract-zip zipfile entry +5ms extract-zip-master/
extract-zip extracting entry +1ms { filename: 'extract-zip-master/',
isDir: false,
isSymlink: false }
extract-zip mkdirp +1ms { dir: '/Users/lukehorvat/code/test1' }
extract-zip zipfile entry +1ms extract-zip-master/.gitignore
extract-zip opening read stream +0ms /Users/lukehorvat/code/test1/extract-zip-master/
extract-zip finished processing +2ms extract-zip-master/ { err: undefined }
extract-zip extracting entry +0ms { filename: 'extract-zip-master/.gitignore',
isDir: false,
isSymlink: false }
extract-zip mkdirp +0ms { dir: '/Users/lukehorvat/code/test1/extract-zip-master' }
extract-zip zipfile entry +1ms extract-zip-master/.npmignore
extract-zip write error +0ms { error: 
 { [Error: ENOENT: no such file or directory, open '/Users/lukehorvat/code/test1/extract-zip-master/']
   errno: -2,
   code: 'ENOENT',
   syscall: 'open',
   path: '/Users/lukehorvat/code/test1/extract-zip-master/' } }
/Users/lukehorvat/code/test1/node_modules/async/lib/async.js:30
          if (called) throw new Error("Callback was already called.");
                      ^

Error: Callback was already called.
  at /Users/lukehorvat/code/test1/node_modules/async/lib/async.js:30:31
  at WriteStream.<anonymous> (/Users/lukehorvat/code/test1/node_modules/extract-zip/index.js:121:24)
  at emitOne (events.js:82:20)
  at WriteStream.emit (events.js:169:7)
  at WriteStream.<anonymous> (fs.js:1846:12)
  at FSReqWrap.oncomplete (fs.js:82:15)
dedelp commented 8 years ago

I can confirm this on windows 7. It appears the directory is not being recognized as a directory. For reference, the externalFileAttributes=16. Doing some further investigation led me here http://stackoverflow.com/a/6297838 - specifically:

The mapping of the external attributes is
      host-system dependent (see 'version made by').  For
      MS-DOS, the low order byte is the MS-DOS directory
      attribute byte.  If input came from standard input, this
      field is set to zero.

Per https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT - Upper byte of version made by = 0 for MS-DOS generated. For a fix I am using the following

        var madeBy = entry.versionMadeBy >> 8
        var isDir = (mode & IFMT) === IFDIR || (madeBy === 0 && entry.externalFileAttributes===16)

Works fine with a few tests I did with windows generated and linux generated zip files, but I did not test it very extensively, nor check to see what else may need to be similarly changed (permissions?).

max-mapper commented 8 years ago

@dedelp excellent sleuthing!

max-mapper commented 8 years ago

fixed in 1.1.2, and I added a test case with the github zip from above