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

Sending a binary message ends up as string instead of Blob or ArrayBuffer #37

Open
Vahelnir opened this issue May 6, 2024 · 1 comment · May be fixed by #39
Open

Sending a binary message ends up as string instead of Blob or ArrayBuffer #37

Vahelnir opened this issue May 6, 2024 · 1 comment · May be fixed by #39

Comments

@Vahelnir
Copy link

Vahelnir commented May 6, 2024

Environment

crossws 0.2.4
node 20.12.2 & 18.20.2

Reproduction

https://stackblitz.com/~/github.com/Vahelnir/crossws-binary-reproduction
https://github.com/Vahelnir/crossws-binary-reproduction

In this reproduction there's an alert that's triggered only when the received message is not a string.
Line index.ts#14 a binary message is sent, but as you can see, the alert was never called.
If you add { binary: true } though, everything's working as expected, and so the alert is triggered.###

Describe the bug

When sending some binary data, the data the client receives is a string instead of being a Blob (or an ArrayBuffer with the ws.binaryType = "arraybuffer").
Also, I couldn't reproduce it on Stackblitz, but on my macbook the connection closes with { code: 1002, reason: 'Invalid UTF-8 in text frame' }.

Edit: Forgot to add that this only applies to the adapters using ws.

Additional context

If no option is sent, the resulting options.binary is undefined.

send(message: any, options?: { compress?: boolean; binary?: boolean }) {
this.ctx.node.ws.send(toBufferLike(message), {
compress: options?.compress,
binary: options?.binary,
...options,
});
return 0;
}

Since ws does not check if the value is there or not before merging, we are losing the "automatic" default value.
https://github.com/websockets/ws/blob/b73b11828d166e9692a9bffe9c01a7e93bab04a8/lib/websocket.js#L464-L470

If everything above is expected, or shouldn't be fixed in crossws, then it would be nice if the binary option was added to the Peer#send signature (but I don't think the other adapters would be able to do anything with it ?)

abstract send(message: any, options?: { compress?: boolean }): number;

Let me know if I forgot anything, or if you need more details.

Logs

No response

@alexzhang1030
Copy link

The same issue faced...

@alexzhang1030 alexzhang1030 linked a pull request May 15, 2024 that will close this issue
8 tasks
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 a pull request may close this issue.

2 participants