-
-
Notifications
You must be signed in to change notification settings - Fork 622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(websocket): prevent sending entire buffer when streaming Uint8Array chunks #3570
Conversation
Hi @Ariel-Moroshko Thank you for the PR! @nakasyou Can you review this? |
To fix the CI error, please format the code with the command |
Hi @yusukebe, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3570 +/- ##
==========================================
- Coverage 94.71% 94.71% -0.01%
==========================================
Files 157 157
Lines 9539 9536 -3
Branches 2774 2810 +36
==========================================
- Hits 9035 9032 -3
Misses 504 504 ☔ View full report in Codecov by Sentry. |
I understood. TypedArray does not necessarily return an ArrayBuffer with exactly the same meaning. it may be part of. That is a bug. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks! Merge it now. |
…ay chunks (honojs#3570) * fix(websocket): prevent full buffer send on Uint8Array chunks * Fix formatting and line endings for CI compliance
Problem
When sending a
Uint8Array
through the WebSocket'ssend()
method, the current implementation converts it to its underlyingArrayBuffer
usingsource.buffer
. This becomes problematic when theUint8Array
is a view over a larger buffer, which is common in streaming scenarios.For example, when streaming data with Bun, it internally uses a buffer pool with 16KB buffers for efficiency. So if you have a
Uint8Array
chunk of 800 bytes, it's actually a view over a 16KB buffer. The current implementation sends the entire 16KB buffer instead of just the 800 bytes of actual data:This causes:
Solution
We should send the
Uint8Array
directly without converting it to its buffer. This preserves the view information and sends only the actual data:Example
The WebSocket API can handle
Uint8Array
directly, so there's no need for the conversion toArrayBuffer
. This change ensures we only send the actual data bytes while maintaining all existing functionality.