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

Changed the decompression library from gzip-js to pako #121

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

naguam
Copy link
Contributor

@naguam naguam commented Jan 27, 2024

Overview

Replacement from gzip-js to pako for the gunzip decompression in the Update route.

Description

Pako is a fast and maintained gzip compression/decompression library.

Gzip-js is not so maintained and crashes on files > 1MB because of node limits on array expansions.

Tested with files of a few KB, 1MB, 10MB and 30 MB.

This does not change the fact we set a limit in the configuration.

But with this commit if the limit is high, it does not crash anymore.

Copy link
Member

@PierreSchwang PierreSchwang left a comment

Choose a reason for hiding this comment

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

Please pin the dependencies to specific versions.

Copy link
Member

@PierreSchwang PierreSchwang left a comment

Choose a reason for hiding this comment

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

Seems to work for me

@NotMyFault
Copy link
Member

Thanks for the PR, please rebase your branch to address the conflicts @naguam

Pako is fast maintained and modern contrary to gzip-js

Pako does not hang on gzip files > 1MB by using too much memory in an array for node as does gzip-js

The size limit must always be defined by configuration, not library capabilities.
@naguam
Copy link
Contributor Author

naguam commented Feb 16, 2024

Rebased my commits on top of the main branch

@NotMyFault NotMyFault merged commit 08a04fd into IntellectualSites:main Feb 16, 2024
4 checks passed
@naguam
Copy link
Contributor Author

naguam commented Feb 16, 2024

Thanks

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

Successfully merging this pull request may close these issues.

4 participants