Skip to content

Handle partial unzip#773

Merged
sindresorhus merged 4 commits intosindresorhus:mainfrom
avoylenko:partial-zip
Nov 10, 2025
Merged

Handle partial unzip#773
sindresorhus merged 4 commits intosindresorhus:mainfrom
avoylenko:partial-zip

Conversation

@avoylenko
Copy link
Contributor

Do not throw the EndOfStreamError() exception in case you pass only partial zip data.

Code to reproduce the error:

file.stream.once('data', async (firstChunk) => {
    const type = await fileTypeFromBuffer(firstChunk)
})

Stack trace:

EndOfStreamError: End-Of-Stream
at BufferTokenizer.peekBuffer (file:///opt/novatalks.engine/node_modules/strtok3/lib/BufferTokenizer.js:38:19)
at BufferTokenizer.readBuffer (file:///opt/novatalks.engine/node_modules/strtok3/lib/BufferTokenizer.js:24:38)
at BufferTokenizer.readToken (file:///opt/novatalks.engine/node_modules/strtok3/lib/AbstractTokenizer.js:32:32)
at ZipHandler.unzip (file:///opt/novatalks.engine/node_modules/@tokenizer/inflate/lib/index.js:127:61)
at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
at async Object.detectConfident [as detect] (file:///opt/novatalks.engine/node_modules/file-type/core.js:521:4)
at async FileTypeParser.fromTokenizer (file:///opt/novatalks.engine/node_modules/file-type/core.js:211:21)

@sindresorhus
Copy link
Owner

Needs a test. And I'm not sure it's correct to just completely silence it.

@avoylenko
Copy link
Contributor Author

@sindresorhus do you want me to push a test that covers this scenario?

@sindresorhus
Copy link
Owner

Yes

@avoylenko
Copy link
Contributor Author

avoylenko commented Aug 6, 2025

Just pushed the test, hope it suits the test structure.

core.js Outdated
return {};
}
});
}).catch(() => {});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to check if the error caught is an EndOfStreamError error, if not throw the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented as required

Copy link
Collaborator

@Borewit Borewit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Julien-Marcou
Copy link

Hello @Borewit I'm encountering the same issue, but for a different reason.

I'm checking the full buffer, and not a partial buffer:

I have a File instance (which is a zip file in my case), and I'm just doing await fileTypeFromBlob(file), and I'm getting the following error:

EndOfStreamError: End-Of-Stream
    at WebStreamByobReader.read (file:///home/jmarcou/dev/my-project/node_modules/strtok3/lib/stream/AbstractStreamReader.js:26:19)
    at ReadStreamTokenizer.readBuffer (file:///home/jmarcou/dev/my-project/node_modules/strtok3/lib/ReadStreamTokenizer.js:34:51)
    at ReadStreamTokenizer.readToken (file:///home/jmarcou/dev/my-project/node_modules/strtok3/lib/AbstractTokenizer.js:32:32)
    at ZipHandler.unzip (file:///home/jmarcou/dev/my-project/node_modules/@tokenizer/inflate/lib/index.js:127:61)
    at async Object.detectConfident [as detect] (file:///home/jmarcou/dev/my-project/node_modules/file-type/core.js:521:4)
    at async FileTypeParser.fromTokenizer (file:///home/jmarcou/dev/my-project/node_modules/file-type/core.js:211:21)
    at async FileTypeParser.fromStream (file:///home/jmarcou/dev/my-project/node_modules/file-type/core.js:243:11)

I think I'm getting this error because when it reads the last chunk of the file, the last chunk is "shorter" than the expected chunk length, and therefore it throws the EndOfStreamError.

If I understand this correctly, without the error being thrown, it would have correctly returned the type as a zip. So this pull request would not only fix reading partial zip buffer, but also fix full zip buffer ^^

Could we expect to see this PR merged & release in the near future?

@Borewit
Copy link
Collaborator

Borewit commented Nov 9, 2025

@sindresorhus, can we merge this PR?

@sindresorhus sindresorhus merged commit 7ad3a90 into sindresorhus:main Nov 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants