Cherry-pick 91678cbbce and 7a2db575e0#5422
Conversation
* 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.
cf1fdcd to
c1bc9fe
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
|
@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. |
|
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. |
I understand, it does not make much sense. |
bc966fa to
81bed8b
Compare
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