Skip to content
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: pass Buffer data type as ArrayBuffer instead of string #365

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yannbriancon
Copy link

From what I experienced with web sockets, Buffer messages are transmitted as ArrayBuffer in the message event.

This modification may break some people implementations but should be closer to real behaviour.

Happy to have your feedback.

@Atrue
Copy link
Collaborator

Atrue commented Aug 29, 2022

Buffer is a Node module, but this library is supposed to mock the client web socket and is not supposed to create a copy of the Node WebSocket implementation (Anyway the server can be written in any language).
The normalizeSendData function is called both on the client and server, but the only thing it should do right is to mimic the client's behavior. So if you want to pass the data to the WebSocket client in the test I think you can just pass the string or blob/arraybuffer data as the client expects (not the Buffer instance), as you test the client, not the server.

@bytemain
Copy link

Meet the same problem, we use some external npm package which will generate Buffer and we transfer them by the websocket.

@Atrue
Copy link
Collaborator

Atrue commented Dec 20, 2023

@bytemain Can you share an example of the issue?
According to the spec the client WebSocket can accept/send only these types: string, blob and ArrayBuffer, (see https://websockets.spec.whatwg.org/#feedback-from-the-protocol p3 and p4). All these types should be handled properly in the mock-socket

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.

None yet

4 participants