Skip to content

Cherry-pick 91678cbbce and 7a2db575e0#5422

Merged
mcollina merged 3 commits into
nodejs:mainfrom
lpinca:cherry-pick/91678cbbce-and-7a2db575e0
Jun 13, 2026
Merged

Cherry-pick 91678cbbce and 7a2db575e0#5422
mcollina merged 3 commits into
nodejs:mainfrom
lpinca:cherry-pick/91678cbbce-and-7a2db575e0

Conversation

@lpinca

@lpinca lpinca commented Jun 13, 2026

Copy link
Copy Markdown
Member

Cherry-pick 91678cbbce and 7a2db575e0 from https://github.com/nodejs/undici-ghsa-vxpw-j846-p89q/pull/1.

These should be backported together with 32dbf0b.

cc: @UlisesGascon

Status

* Empty first fragment + non-empty continuation should deliver the message.
* A flood of empty continuation frames must still trip maxFragments.
* WebSocketStream must honor maxFragments the same as WebSocket.
…ptions

* Drop body.length / data.length short-circuits on writeFragments.
  Per RFC 6455 §5.4, zero-byte fragments are conforming and the parser
  uses #fragments.length > 0 to track an in-progress fragmented message.
* Propagate maxFragments and maxPayloadSize from the dispatcher to
  WebSocketStream's ByteParser, mirroring lib/web/websocket/websocket.js.
@lpinca lpinca force-pushed the cherry-pick/91678cbbce-and-7a2db575e0 branch from cf1fdcd to c1bc9fe Compare June 13, 2026 14:18
@lpinca lpinca changed the title Cherry pick 91678cbbce and 7a2db575e0 Cherry-pick 91678cbbce and 7a2db575e0 Jun 13, 2026
@codecov-commenter

codecov-commenter commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.35%. Comparing base (6df53c5) to head (81bed8b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5422      +/-   ##
==========================================
- Coverage   93.35%   93.35%   -0.01%     
==========================================
  Files         110      110              
  Lines       36966    36993      +27     
==========================================
+ Hits        34511    34536      +25     
- Misses       2455     2457       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

@lpinca

lpinca commented Jun 13, 2026

Copy link
Copy Markdown
Member Author

@KhafraDev I guess the 👎 is because you do not think this is relevant for a client, and I might agree, but the problem is of the same type as the zip bomb, so I think both should be fixed or neither.

@KhafraDev

Copy link
Copy Markdown
Member

I don't think zip bombs were a vulnerability either (anything regarding an "attacker server"). I'm not blocking it, I would have preferred if both were disabled by default though, going back to ljharb's original argument against adding fetch in node: "How can we add this when it doesn’t comply with the browser spec ... ?"

I've never been against people using ws instead of undici's WebSocket/the global WebSocket which provides the same mitigations with a lot more customization.

@lpinca

lpinca commented Jun 13, 2026

Copy link
Copy Markdown
Member Author

anything regarding an "attacker server"

I understand, it does not make much sense.

@lpinca lpinca force-pushed the cherry-pick/91678cbbce-and-7a2db575e0 branch from bc966fa to 81bed8b Compare June 13, 2026 15:42

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 382a038 into nodejs:main Jun 13, 2026
36 of 39 checks passed
@lpinca lpinca deleted the cherry-pick/91678cbbce-and-7a2db575e0 branch June 13, 2026 19:06
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.

5 participants