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

Add Zstd support (optional dependency, just like Brotli) #652

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

adrianodennanni
Copy link
Contributor

@adrianodennanni adrianodennanni commented Aug 20, 2024

Just like #650 (adds brotli compression), this PR aims to give Mechanize support for Zstandard (zstd), in a similar way (will only work if you have the zstd-ruby gem installed).

@flavorjones
Copy link
Member

@adrianodennanni Thanks for the pull request. Can you help me understand the specific problem that you're having that led you to create this pull request? Is there a server somewhere that is returning content unexpectedly encrypted with zstd, or that doesn't support any other forms of compression?

@adrianodennanni
Copy link
Contributor Author

Hello @flavorjones ! We had a case where a request was not accepted because the headers were not exactly equal to a request made from a real browser. So, we informed the Accept-Encoding header with gzip, deflate, br, zstd as value.
Although usually the server chooses the first encoding algorithm to use, this site choose zstd instead, and returned a body that Mechanize could not understand.

@adrianodennanni
Copy link
Contributor Author

Also, some benchmarks point out that Zstd decompresses 40% faster than Brotli. Informing Accept-Encoding as zstd could also provide performance gains for the client.

@flavorjones
Copy link
Member

@adrianodennanni Thanks for the quick response. This looks great! Going to merge it and cut a release shortly.

@flavorjones flavorjones merged commit 3bace72 into sparklemotion:main Aug 21, 2024
9 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.

2 participants