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

Support Uint8Array buffers in stdio #4

Merged
merged 1 commit into from
Sep 8, 2024

Conversation

arjunbajaj
Copy link
Contributor

Problem

Currently, uWASI converts buffers to strings for stdout/stderr and converts strings to buffers for stdin. This is not deseriable in certain conditions and causes a real bug in WASI modules that are passing binary data (such as network protocol packets), due to how TextDecoder.decode works.

The Encoding/Decoding Bug

I discovered that converting binaries to strings using (new TextDecoder).decode(buf) is causing issues. Essentially, when non UTF-8 compliant byte sequences are found, it adds another character to the string. Running (new TextEncoder).encode(str) then produces a buffer which is different from the original. This is causing issue in my WASI module as it's passing binary packets from WASI to a socket opened in JS. The example below reproduces the problem:

> const x = new Uint8Array([0, 0, 1, 200])
undefined

> x
Uint8Array(4) [ 0, 0, 1, 200 ]

> const decoded = new TextDecoder('utf-8').decode(x)
undefined

> decoded
'\x00\x00\x01�'

> new TextEncoder().encode(decoded)
Uint8Array(6) [ 0, 0, 1, 239, 191, 189 ]

Fixes

This PR fixes two related issues:

  1. It now accepts an outputBuffers boolean in the useStdio options. By default its assumed as false keeping the current behaviour unchanged. If true is passed, the stdout and stderr handlers will be passed Uint8Arrays instead of strings.

  2. It also accepts Uint8Arrays on stdin, along with strings. If a Uint8Array is passed, no encoding will be done, unlike for strings. Again, the current behaviour remains unchanged.

  3. I have also updated the readme documenting both the new features.

Let me know if I should improve the PR in any way.

Thanks!

1. `stdin` now accepts Uint8Arrays directly.
2. Uint8Arrays are sent to `stdout/stderr` handlers, when `outputBuffers: true` is passed to `useStdio({})` options.
Copy link
Member

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kateinoigakukun kateinoigakukun merged commit ce54440 into swiftwasm:main Sep 8, 2024
1 check passed
@arjunbajaj
Copy link
Contributor Author

Thanks for merging! Really appreciate it.

Any plans for the next npm release?

@kateinoigakukun
Copy link
Member

Just released 1.3.0 :)

@arjunbajaj
Copy link
Contributor Author

Thank you!

@arjunbajaj arjunbajaj deleted the stdio-buffers-support branch September 8, 2024 22:08
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.

2 participants