src: fix build with `--shared-brotli` by lazerg · Pull Request #66895 · nodejs/node

lazerg · 2026-10-21T11:49:34Z

BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH isn't in any brotli release yet (1.2.0 is the latest), so building with --shared-brotli fails since 7ba22c2. It's an enum value and the bundled brotli still reports 1.2.0, so there's nothing to #ifdef on. This only exposes the constant when brotli is bundled and skips the matching test otherwise.

I built with --shared-brotli against Homebrew brotli 1.2.0 and ran parallel/test-zlib* (90 tests, all pass). The GN change is untested, I had no gn locally.

Fixes: #66894

Assisted-by: Claude Code Signed-off-by: lazerg <lazerg2@gmail.com>

nodejs-github-bot · 2026-10-21T11:49:40Z

Review requested:

@nodejs/gyp

github-actions · 2026-10-21T11:50:06Z

Caution

AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context.

aduh95 · 2026-10-21T11:53:24Z

  NODE_DEFINE_CONSTANT(target, BROTLI_DECODER_ERROR_FORMAT_PADDING_1);
  NODE_DEFINE_CONSTANT(target, BROTLI_DECODER_ERROR_FORMAT_PADDING_2);
  NODE_DEFINE_CONSTANT(target, BROTLI_DECODER_ERROR_FORMAT_DISTANCE);
+#ifndef NODE_SHARED_BROTLI


Suggested change

#ifndef NODE_SHARED_BROTLI

#ifdef BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH

I wonder if we need a different check here, since BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH is an enum constant.

aduh95 · 2026-10-21T11:54:04Z

+    }, {
+      'defines': [ 'NODE_SHARED_BROTLI' ],


Suggested change

}, {

'defines': [ 'NODE_SHARED_BROTLI' ],

aduh95 · 2026-10-21T11:54:19Z

+    if (node_shared_brotli) {
+      defines += [ "NODE_SHARED_BROTLI" ]
+    }


Suggested change

if (node_shared_brotli) {

defines += [ "NODE_SHARED_BROTLI" ]

}

panva · 2026-10-21T11:56:31Z

 }

-{
+if (!process.config.variables.node_shared_brotli) {


Suggested change

if (!process.config.variables.node_shared_brotli) {

if (zlib.constants.BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH) {

aduh95 · 2026-10-21T12:09:34Z

Can you remove the cherry-pick to validate it does fix this build and still pass the tests

node/tools/nix/sharedLibDeps.nix

Lines 25 to 32 in 697ecce

    
           brotli = pkgs.brotli.overrideAttrs (old: { 
        
             patches = (old.patches or [ ]) ++ [ 
        
               (pkgs.fetchpatch2 { 
        
                 url = "https://github.com/google/brotli/commit/0ed4268a4401abcb4472096340477cf4af7d1a2b.patch?full_index=1"; 
        
                 hash = "sha256-dVpnYI2JLDEt2VQkbI6RnbpdcQSMB5bTQ6TJp5CLnyw="; 
        
               }) 
        
             ]; 
        
           });

codecov · 2026-10-21T13:35:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.57%. Comparing base (7ba22c2) to head (ab1673e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66895      +/-   ##
==========================================
- Coverage   90.58%   90.57%   -0.01%     
==========================================
  Files         797      797              
  Lines      278434   278486      +52     
  Branches    53531    53526       -5     
==========================================
+ Hits       252216   252236      +20     
- Misses      16571    16591      +20     
- Partials     9647     9659      +12

Files with missing lines	Coverage Δ
src/node_zlib.cc	`80.59% <ø> (+0.29%)`	⬆️

... and 27 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.

ziyao233 · 2026-10-21T17:56:11Z

Since commit introducing this regression is related to a CVE, I wonder would such workaround breaks the fix?

0x9fff00 · 2026-10-21T21:01:27Z

Brotli 1.2.1 has been released with a backport of the fix for CVE-2026-111497 which added BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH.

Since commit introducing this regression is related to a CVE, I wonder would such workaround breaks the fix?

Yes. If --shared-brotli is used, Brotli 1.2.1 is required in order to fix CVE-2026-111497 because in that case the vendored copy of Brotli in Node.js is not used.

Assisted-by: Claude Code Signed-off-by: lazerg <lazerg2@gmail.com>

github-actions · 2026-10-22T03:11:26Z

Platform	Number of requisites	Proportion of new derivations
`x86_64-linux`	544 -> 541 => +-3	0 / 541 = 0.0%
`aarch64-linux`	545 -> 542 => +-3	0 / 542 = 0.0%
`x86_64-darwin`	457 -> 454 => +-3	0 / 454 = 0.0%
`aarch64-darwin`	470 -> 467 => +-3	0 / 467 = 0.0%
Total	2016 -> 2004 => +-12	0 / 2004 = 0.0%

Changelog

@@ -120 +119,0 @@
-/nix/store/kminssh6dm17s3f6cqszaxwpyyhm6i88-brotli-1.2.0 (aarch64-darwin)
@@ -122,2 +120,0 @@
-/nix/store/6aaijr2v6rlgailsir2l1q9j3k72flxx-brotli-1.2.0 (aarch64-linux)
-/nix/store/895yl6jdl10mkffq8qrgimy2i7snla3q-brotli-1.2.0 (x86_64-darwin)
@@ -125 +121,0 @@
-/nix/store/2nyhgrln2i4rw0rdijyqh0nn333ws3nx-brotli-1.2.0 (x86_64-linux)
@@ -128,2 +123,0 @@
-/nix/store/c4y22jmyh7wjhbdidbnhz661jpg9kaim-brotli-1.2.0-dev (aarch64-darwin)
-/nix/store/44lj6r7v54yfzcd7p6l6n76g7ma7y633-brotli-1.2.0-dev (aarch64-linux)
@@ -132 +125,0 @@
-/nix/store/af449dhklypcj2kild1fv4q9dy2bmjdv-brotli-1.2.0-dev (x86_64-darwin)
@@ -134 +126,0 @@
-/nix/store/ainnzs18blgfd5f3n21wnq0m4j5ryf6j-brotli-1.2.0-dev (x86_64-linux)
@@ -136 +127,0 @@
-/nix/store/kl3m09q74kf4g4mfk9whw65w8dw0jg3a-brotli-1.2.0-lib (aarch64-darwin)
@@ -138 +128,0 @@
-/nix/store/r5vy3djdp4qpddj4bsw8hqcl9asyp1h2-brotli-1.2.0-lib (aarch64-linux)
@@ -140 +129,0 @@
-/nix/store/yvkvvnyvnaw08bl9gvnkbk33m339v4ff-brotli-1.2.0-lib (x86_64-darwin)
@@ -142 +130,0 @@
-/nix/store/f667bn2vz2vv1hf4ayjc49r3w967ivhr-brotli-1.2.0-lib (x86_64-linux)

lazerg · 2026-10-22T03:18:22Z

@aduh95 ✅ Done

I removed the Nix cherry-pick of the brotli patch (c47b864). The shared-libraries CI now builds against unpatched brotli, so it checks the fix by itself.

I did not use your #ifdef suggestion for src/node_zlib.cc. BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH is an enum value, and #ifdef cannot see an enum value. The constant would be missing from every build. The constants now come from BROTLI_DECODER_ERROR_CODES_LIST in the brotli header (67111ab).

Local result: all 90 parallel/test-zlib* tests pass with system brotli 1.2.1. With brotli 1.2.0, the block-switch test is skipped.

src: fix build with --shared-brotli

ab1673e

Assisted-by: Claude Code Signed-off-by: lazerg <lazerg2@gmail.com>

github-actions Bot added the agentscan:community-flagged label Oct 21, 2026

aduh95 reviewed Oct 21, 2026

View reviewed changes

panva reviewed Oct 21, 2026

View reviewed changes

lazerg added 2 commits October 22, 2026 08:05

src: derive brotli decoder constants from header

67111ab

Assisted-by: Claude Code Signed-off-by: lazerg <lazerg2@gmail.com>

tools: drop brotli patch from Nix shared deps

c47b864

Assisted-by: Claude Code Signed-off-by: lazerg <lazerg2@gmail.com>

	#ifndef NODE_SHARED_BROTLI
	#ifdef BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH

	if (node_shared_brotli) {
	defines += [ "NODE_SHARED_BROTLI" ]
	}

	if (!process.config.variables.node_shared_brotli) {
	if (zlib.constants.BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH) {

Conversation

lazerg commented Oct 21, 2026

Uh oh!

nodejs-github-bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lazerg Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

aduh95 commented Oct 21, 2026

Uh oh!

codecov Bot commented Oct 21, 2026

Codecov Report

Uh oh!

ziyao233 commented Oct 21, 2026

Uh oh!

0x9fff00 commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

lazerg commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lazerg Oct 22, 2026 •

edited

Loading

lazerg commented Oct 22, 2026 •

edited

Loading