Skip to content

zlib: add dictionary support to zstdCompress and zstdDecompress #59240

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

Merged
merged 1 commit into from
Aug 4, 2025

Conversation

lluisemper
Copy link
Contributor

Adds optional dictionary support to zlib’s zstdCompress and zstdDecompress APIs. This enables better compression ratios when the dictionary matches expected input structure or content patterns.

The implementation allows passing a dictionary buffer through the options object. Support was added to both streaming and convenience methods. Tests and documentation were also updated to reflect this new capability.

Fixes: #59105

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Jul 27, 2025
@lluisemper
Copy link
Contributor Author

@addaleax Thanks for the review! I've updated the PR based on your feedback ❤️ I learned a lot thanks to your suggestions.

Please take another look when you get a chance.

@lluisemper lluisemper requested a review from addaleax July 29, 2025 18:57
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Looks great (% one small memory management comment 🙂)

@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 30, 2025
@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 30, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 30, 2025
@nodejs-github-bot
Copy link
Collaborator

@addaleax
Copy link
Member

Looks like the only genuine failure here is the C++ linter failure, those can be fixed as described e.g. here:

node/Makefile

Lines 1515 to 1520 in 91dadf2

# To format staged changes:
# $ make format-cpp
# To format HEAD~1...HEAD (latest commit):
# $ CLANG_FORMAT_START=`git rev-parse HEAD~1` make format-cpp
# To format diff between main and current branch head (main...HEAD):
# $ CLANG_FORMAT_START=main make format-cpp

Copy link

codecov bot commented Jul 30, 2025

Codecov Report

❌ Patch coverage is 65.51724% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.98%. Comparing base (91dadf2) to head (7028088).
⚠️ Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
src/node_zlib.cc 61.53% 5 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59240      +/-   ##
==========================================
- Coverage   90.00%   89.98%   -0.03%     
==========================================
  Files         649      649              
  Lines      192180   192205      +25     
  Branches    37659    37662       +3     
==========================================
- Hits       172970   172949      -21     
- Misses      11830    11878      +48     
+ Partials     7380     7378       -2     
Files with missing lines Coverage Δ
lib/zlib.js 98.27% <100.00%> (+<0.01%) ⬆️
src/node_zlib.cc 77.99% <61.53%> (-0.41%) ⬇️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Adds optional dictionary support to zlib’s zstdCompress and
zstdDecompress APIs. This enables better compression ratios when the
dictionary matches expected input structure or content patterns.

The implementation allows passing a `dictionary` buffer through the
options object. Support was added to both streaming and convenience
methods. Tests and documentation were also updated to reflect this new
capability.

Fixes: nodejs#59105
@lluisemper
Copy link
Contributor Author

Thanks for your patience with me! It looks like I forgot to run the formatter on my previous commit. I've now run it on the latest commit to fix the issue.

@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 30, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 30, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 31, 2025
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 31, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/59240
✔  Done loading data for nodejs/node/pull/59240
----------------------------------- PR info ------------------------------------
Title      zlib: add dictionary support to zstdCompress and zstdDecompress (#59240)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     lluisemper:59105 -> nodejs:main
Labels     c++, zlib, semver-minor, author ready, needs-ci
Commits    1
 - zlib: add dictionary support to zstdCompress and zstdDecompress
Committers 1
 - lluisemper <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/59240
Fixes: https://github.com/nodejs/node/issues/59105
Reviewed-By: Anna Henningsen <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/59240
Fixes: https://github.com/nodejs/node/issues/59105
Reviewed-By: Anna Henningsen <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 27 Jul 2025 10:56:37 GMT
   ✔  Approvals: 1
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/59240#pullrequestreview-3072972806
   ✘  This PR needs to wait 68 more hours to land (or 0 hours if there is one more approval)
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2025-07-31T12:06:22Z: https://ci.nodejs.org/job/node-test-pull-request/68335/
- Querying data for job/node-test-pull-request/68335/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/16651874016

@addaleax addaleax removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jul 31, 2025
@addaleax addaleax added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 4, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 4, 2025
@nodejs-github-bot nodejs-github-bot merged commit b8e6432 into nodejs:main Aug 4, 2025
68 of 73 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in b8e6432

@addaleax
Copy link
Member

addaleax commented Aug 4, 2025

@lluisemper Thank you for the PR! 🚀

panva pushed a commit to panva/node that referenced this pull request Aug 7, 2025
Adds optional dictionary support to zlib’s zstdCompress and
zstdDecompress APIs. This enables better compression ratios when the
dictionary matches expected input structure or content patterns.

The implementation allows passing a `dictionary` buffer through the
options object. Support was added to both streaming and convenience
methods. Tests and documentation were also updated to reflect this new
capability.

Fixes: nodejs#59105
PR-URL: nodejs#59240
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Aug 8, 2025
Adds optional dictionary support to zlib’s zstdCompress and
zstdDecompress APIs. This enables better compression ratios when the
dictionary matches expected input structure or content patterns.

The implementation allows passing a `dictionary` buffer through the
options object. Support was added to both streaming and convenience
methods. Tests and documentation were also updated to reflect this new
capability.

Fixes: #59105
PR-URL: #59240
Reviewed-By: Anna Henningsen <[email protected]>
mete0rfish pushed a commit to mete0rfish/node-contribute that referenced this pull request Aug 9, 2025
Adds optional dictionary support to zlib’s zstdCompress and
zstdDecompress APIs. This enables better compression ratios when the
dictionary matches expected input structure or content patterns.

The implementation allows passing a `dictionary` buffer through the
options object. Support was added to both streaming and convenience
methods. Tests and documentation were also updated to reflect this new
capability.

Fixes: nodejs#59105
PR-URL: nodejs#59240
Reviewed-By: Anna Henningsen <[email protected]>
panva pushed a commit to panva/node that referenced this pull request Aug 9, 2025
Adds optional dictionary support to zlib’s zstdCompress and
zstdDecompress APIs. This enables better compression ratios when the
dictionary matches expected input structure or content patterns.

The implementation allows passing a `dictionary` buffer through the
options object. Support was added to both streaming and convenience
methods. Tests and documentation were also updated to reflect this new
capability.

Fixes: nodejs#59105
PR-URL: nodejs#59240
Reviewed-By: Anna Henningsen <[email protected]>
nodejs-github-bot added a commit that referenced this pull request Aug 11, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Adds optional dictionary support to zlib’s zstdCompress and
zstdDecompress APIs. This enables better compression ratios when the
dictionary matches expected input structure or content patterns.

The implementation allows passing a `dictionary` buffer through the
options object. Support was added to both streaming and convenience
methods. Tests and documentation were also updated to reflect this new
capability.

Fixes: #59105
PR-URL: #59240
Reviewed-By: Anna Henningsen <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
  * (SEMVER-MINOR) add --use-env-proxy (Joyee Cheung) #59151
  * (SEMVER-MINOR) support `${pid}` placeholder in --cpu-prof-name (Haram Jeong) #59072
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
  * (SEMVER-MINOR) add tls.setDefaultCACertificates() (Joyee Cheung) #58822
deps:
  * update archs files for openssl-3.5.1 (Node.js GitHub Bot) #59234
  * upgrade openssl sources to openssl-3.5.1 (Node.js GitHub Bot) #59234
dns:
  * (SEMVER-MINOR) support max timeout (theanarkh) #58440
doc:
  * update the instruction on how to verify releases (Antoine du Hamel) #59113
esm:
  * (SEMVER-MINOR) unflag --experimental-wasm-modules (Guy Bedford) #57038
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
http,https:
  * (SEMVER-MINOR) add built-in proxy support in http/https.request and Agent (Joyee Cheung) #58980
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
net:
  * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087
test:
  * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980
worker:
  * (SEMVER-MINOR) add web locks api (ishabi) #58666
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
  * (SEMVER-MINOR) add --use-env-proxy (Joyee Cheung) #59151
  * (SEMVER-MINOR) support `${pid}` placeholder in --cpu-prof-name (Haram Jeong) #59072
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
  * (SEMVER-MINOR) add tls.setDefaultCACertificates() (Joyee Cheung) #58822
deps:
  * update archs files for openssl-3.5.1 (Node.js GitHub Bot) #59234
  * upgrade openssl sources to openssl-3.5.1 (Node.js GitHub Bot) #59234
dns:
  * (SEMVER-MINOR) support max timeout (theanarkh) #58440
doc:
  * update the instruction on how to verify releases (Antoine du Hamel) #59113
esm:
  * (SEMVER-MINOR) unflag --experimental-wasm-modules (Guy Bedford) #57038
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
http,https:
  * (SEMVER-MINOR) add built-in proxy support in http/https.request and Agent (Joyee Cheung) #58980
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
net:
  * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087
test:
  * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980
worker:
  * (SEMVER-MINOR) add web locks api (ishabi) #58666
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 14, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 14, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <[email protected]>
meteorqz6 pushed a commit to meteorqz6/node that referenced this pull request Aug 15, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) nodejs#59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) nodejs#59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) nodejs#58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) nodejs#59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) nodejs#59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) nodejs#59240

PR-URL: nodejs#59449
Signed-off-by: RafaelGSS <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dictionary support for zstd
3 participants