Add ZSTD bindings to the node:zlib package. #6007

alistairjevans · 2026-02-03T11:09:01Z

Based pretty heavily on the brotli support.

Closes #4013

github-actions · 2026-02-03T11:09:14Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

alistairjevans · 2026-02-03T11:09:48Z

I have read the CLA Document and I hereby sign the CLA

jasnell · 2026-02-03T15:12:13Z

src/workerd/api/node/zlib-util.c++

+  input_.pos = 0;
+  output_.dst = output.begin();
+  output_.size = output.size();
+  output_.pos = 0;


This could defer to the setInputBuffer and setOutputBuffer methods below to reduce duplication

src/workerd/api/node/zlib-util.c++

jasnell · 2026-02-03T15:13:51Z

src/workerd/api/node/zlib-util.c++

+ZstdEncoderContext::ZstdEncoderContext(ZlibMode _mode): ZstdContext(_mode) {
+  cctx_ = ZSTD_createCCtx();
+}


??

Suggested change

ZstdEncoderContext::ZstdEncoderContext(ZlibMode _mode): ZstdContext(_mode) {

cctx_ = ZSTD_createCCtx();

}

ZstdEncoderContext::ZstdEncoderContext(ZlibMode _mode)

: ZstdContext(_mode),

cctx_(ZSTD_createCCtx()) {}

Also, should we check for cctx_ == nullptr in here or no?

I wouldn't check for nullptr here, initialize will raise an error if it failed to set. Will update to use the compact style.

jasnell · 2026-02-03T15:14:31Z

src/workerd/api/node/zlib-util.c++

+ZstdEncoderContext::~ZstdEncoderContext() {
+  if (cctx_ != nullptr) {
+    ZSTD_freeCCtx(cctx_);
+    cctx_ = nullptr;


It might be worth wrapping this in a custom smart pointer to ensure the cleanup is correct. Non-blocking tho.

Added (brotli implementation does already do this)

jasnell · 2026-02-03T15:15:41Z

src/workerd/api/node/zlib-util.c++

+kj::Maybe<CompressionError> ZstdEncoderContext::initialize(uint64_t pledgedSrcSize) {
+  if (cctx_ == nullptr) {
+    cctx_ = ZSTD_createCCtx();
+  }


When would this be the case since we're calling ZSTD_createCCtx() in the constructor? If there's a possibility this is nullptr here a doc comment explaining the scenario would be helpful.

I think this was a faulty duplication of some of the behaviour in the brotli implementation; resetStream for brotli calls initialize to reset the context, but zstd has it's own ZSTD_CCtx_reset, so creating the context in initialize is unnecessary.

jasnell · 2026-02-03T15:18:51Z

src/workerd/api/node/zlib-util.c++

+    if (ZSTD_isError(result)) {
+      error_ = ZSTD_getErrorCode(result);
+      return CompressionError(kj::str(ZSTD_getErrorName(result)),
+          kj::str("ERR_ZSTD_COMPRESSION_FAILED"), -1);
+    }


Given that this ZSTD_isError(...) block is repeated across multiple functions, it might be worth having a separate utility function, e.g.

KJ_IF_SOME(err, checkIsError(result)) { return err; }

Added a zstdCheckError function, although to be honest I'm not sure I like it more than just making the repeated checks 🙈

jasnell · 2026-02-03T15:19:28Z

src/workerd/api/node/zlib-util.c++

+}
+
+kj::Maybe<CompressionError> ZstdEncoderContext::setParams(int key, int value) {
+  size_t result = ZSTD_CCtx_setParameter(cctx_, static_cast<ZSTD_cParameter>(key), value);


A KJ_DASSERT verifying that key is within the range of ZSTD_cParameter would be helpful

jasnell · 2026-02-03T15:25:30Z

@anonrig ... given your familiarity with the zlib impl in workerd, it would be good to have your review on this as well.

src/node/internal/internal_zlib.ts

anonrig · 2026-02-03T15:56:32Z

src/node/internal/internal_zlib_base.ts

+  constructor(options: ZstdOptions | undefined | null, mode: number) {
+    ok(mode === CONST_ZSTD_DECODE || mode === CONST_ZSTD_ENCODE);
+    zstdInitParamsArray.fill(-1);


This diverges from the Node.js implementation and makes it a lot less readable. Ideally the constructor should receive a parameter for initParamsArray just like Node.js

class Zstd extends ZlibBase { constructor(opts, mode, initParamsArray, maxParam) {

If we diverge from Node.js, finding bugs & keep the code in sync would be a lot harder then it really is.

Updated 👍

anonrig · 2026-02-03T15:57:26Z

src/workerd/api/node/tests/BUILD.bazel

+    size = "large",
+    src = "zstd-nodejs-test.wd-test",
+    args = ["--experimental"],
+    data = ["zstd-nodejs-test.js"],


Suggested change

data = ["zstd-nodejs-test.js"],

data = ["zlib-zstd-nodejs-test.js"],

anonrig · 2026-02-03T15:57:32Z

src/workerd/api/node/tests/BUILD.bazel


+wd_test(
+    size = "large",
+    src = "zstd-nodejs-test.wd-test",


Suggested change

src = "zstd-nodejs-test.wd-test",

src = "zlib-zstd-nodejs-test.wd-test",

src/workerd/api/node/tests/zstd-nodejs-test.js

anonrig · 2026-02-03T16:00:04Z

src/workerd/api/node/tests/zstd-nodejs-test.js

+export const zstdConstantsTest = {
+  test() {
+    // Flush directives
+    assert.strictEqual(typeof zlib.constants.ZSTD_e_continue, 'number');


You've already tested this in zlib-nodejs-test.

Removed duplicate

src/node/internal/internal_zlib.ts

alistairjevans · 2026-02-03T16:54:07Z

Thank you for the feedback @anonrig and @jasnell! 🙏 I believe I've addressed all your comments now.

alistairjevans · 2026-02-11T20:56:56Z

Merged latest; @anonrig / @jasnell , any chance of a re-review? 🙏

anonrig · 2026-02-11T21:26:28Z

LGTM. @alistairjevans formatter seems to be failing: https://github.com/cloudflare/workerd/actions/runs/21922721360/job/63307147452?pr=6007

@jasnell can you take a look?

alistairjevans · 2026-02-11T21:49:35Z

Thanks @anonrig, formatting fix pushed, linter passes locally now.

jasnell · 2026-02-11T22:25:49Z

Overall I think this looks good. What I'm considering... (something I wish I had thought to do on the previous node:zlib additions)... is adding this initially behind an experimental compat flag and having the security team do some quick fuzz testing on it. Anything new that is dealing with bytes being passed around makes me just a bit nervous and a little extra testing doesn't hurt. That's not to say I see anything in here concretely questionable, just thinking about taking the extra precaution. @danlapid what do you think?

Add ZSTD bindings to the node:zlib package.

ab0e269

alistairjevans requested review from a team as code owners February 3, 2026 11:09

github-actions bot added a commit that referenced this pull request Feb 3, 2026

@alistairjevans has signed the CLA in #6007

87f4359

alistairjevans mentioned this pull request Feb 3, 2026

node:zlib implement zstd #4013

Open

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Show resolved Hide resolved

jasnell reviewed Feb 3, 2026

View reviewed changes

jasnell requested a review from anonrig February 3, 2026 15:25

anonrig reviewed Feb 3, 2026

View reviewed changes

alistairjevans added 6 commits February 3, 2026 16:20

zlib-util minor formatting and asserts

e1da60c

Bring zstd arguments in line with nodejs layout.

3e2357e

Test updates

3f9e786

Add zstdCheckError helper function.

08acff0

Remove duplicating test

915c17a

Use smart pointer for zstd context.

2295317

Merge branch 'main' into alistairjevans/zstd-support

8d66963

anonrig approved these changes Feb 11, 2026

View reviewed changes

Fix formatting.

e77c44a

	data = ["zstd-nodejs-test.js"],
	data = ["zlib-zstd-nodejs-test.js"],

	src = "zstd-nodejs-test.wd-test",
	src = "zlib-zstd-nodejs-test.wd-test",

Add ZSTD bindings to the node:zlib package. #6007

Are you sure you want to change the base?

Add ZSTD bindings to the node:zlib package. #6007

Conversation

alistairjevans commented Feb 3, 2026

Uh oh!

github-actions bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alistairjevans commented Feb 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell commented Feb 3, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alistairjevans commented Feb 3, 2026

Uh oh!

alistairjevans commented Feb 11, 2026

Uh oh!

anonrig commented Feb 11, 2026

Uh oh!

alistairjevans commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

github-actions bot commented Feb 3, 2026 •

edited

Loading

alistairjevans commented Feb 11, 2026 •

edited

Loading

jasnell commented Feb 11, 2026 •

edited

Loading