antelle / node-stream-zip

node.js library for fast reading of large ZIPs
Other
447 stars 63 forks source link

Throws error on zip.close() if at least one entity is converted to stream #74

Closed Allypost closed 3 years ago

Allypost commented 3 years ago

When at least one entity in a zip file is converted to a stream, the library throws a bad file descriptor error:

node:events:355
      throw er; // Unhandled 'error' event
      ^

Error: EBADF: bad file descriptor, read
Emitted 'error' event on EntryDataReaderStream instance at:
    at EntryDataReaderStream.readCallback (/tmp/tmp.b4Y5QdWysT/node_modules/node-stream-zip/node_stream_zip.js:1087:18)
    at FSReqCallback.wrapper [as oncomplete] (node:fs:557:5) {
  errno: -9,
  code: 'EBADF',
  syscall: 'read'
}

This is the code that I tested:

const { async: StreamZipAsync } = require("node-stream-zip");

(async () => {
  const zip = new StreamZipAsync({
    file: "./test.zip",
  });

  for (const entry of Object.values(await zip.entries())) {
    await zip.stream(entry).then((stream) => stream.end());
  }

  await zip.close();
})();

It works if the await zip.close() line is omitted. It also works fine if the entity is not converted to a stream (the zip.stream method isn't used).

The files I tested with were generated by zipping 1MB of random bytes:

head -c 1M </dev/urandom >test && zip test.zip test

This was tested/observed on Manjaro Linux and inside an Alpine docker image.

I only saw an example for streaming one file but the description for the .close method implies multiple streams can be opened. I may be wrong in that assumption but then a disclaimer should be put in place to warn of incorrect usages.

antelle commented 3 years ago

Hi! I think it's because you're not waiting for streams to finish:

await zip.stream(entry) // waits for the stream to become available
    .then((stream) =>   // the stream is ready now
        stream.end()    // does nothing: ReadableStream is closed when `end` event is emitted
    );
antelle commented 3 years ago

Just tried your example, it works if you change it like this:

    for (const entry of Object.values(await zip.entries())) {
        const stream = await zip.stream(entry);
        await new Promise((resolve) => stream.on('end', resolve));
    }