fix: limit EventSource event size#5552
Conversation
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5552 +/- ##
==========================================
+ Coverage 93.44% 93.48% +0.04%
==========================================
Files 110 110
Lines 37443 37545 +102
==========================================
+ Hits 34987 35098 +111
+ Misses 2456 2447 -9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
KhafraDev
left a comment
KhafraDev
left a comment
There was a problem hiding this comment.
I'm not as familiar with EventSource so some of these might be wrong, feel free to dismiss as needed
| eventSourceInitDict = webidl.converters.EventSourceInitDict(eventSourceInitDict, prefix, 'eventSourceInitDict') | ||
|
|
||
| this.#dispatcher = eventSourceInitDict.node.dispatcher || eventSourceInitDict.dispatcher | ||
| this.#dispatcher = eventSourceInitDict.node.dispatcher || eventSourceInitDict.dispatcher || getGlobalDispatcher() |
There was a problem hiding this comment.
this would make more sense as part of the webidl conversion as a default value
| module.exports = { | ||
| EventSourceStream | ||
| EventSourceStream, | ||
| defaultMaxEventSize |
There was a problem hiding this comment.
I don't see this being imported anywhere
| } | ||
|
|
||
| getEventDataSize (event) { | ||
| if (event === this.event) { |
| } | ||
|
|
||
| setEventDataSize (event, eventDataSize) { | ||
| if (event === this.event) { |
There was a problem hiding this comment.
same here, we can only ever have 1 event at a time
metcoder95
left a comment
metcoder95
left a comment
There was a problem hiding this comment.
lgtm
leverage to @KhafraDev comments
Signed-off-by: Matteo Collina <hello@matteocollina.com>
6 participants
This relates to...
N/A
Rationale
EventSource messages should not accumulate data beyond Node.js's maximum string length, while allowing applications to configure a lower limit when needed.
Changes
Features
eventSource.maxEventSizedispatcher option.Bug Fixes
buffer.kStringMaxLengthby default.Breaking Changes and Deprecations
N/A
Status