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

Polyfill for btoa inconsistent with WebApi / Node #638

Closed
chetjan opened this issue Oct 12, 2023 · 5 comments · Fixed by #643 or #646
Closed

Polyfill for btoa inconsistent with WebApi / Node #638

chetjan opened this issue Oct 12, 2023 · 5 comments · Fixed by #643 or #646

Comments

@chetjan
Copy link

chetjan commented Oct 12, 2023

Bug Report

btoa does not behave like it does in NodeJS / Chromium. For reference, the polyfill is export const btoa = (str) => Buffer.from(str, 'binary').toString('base64')

let a = new Uint8Array([1,2,3]) in the following examples:

Current
Behavior when running the on an edge server:
Buffer.from(a, 'binary').toString('base64') -> 'AQID'

Expected
In Chrome / in Node:
btoa(a) -> 'MSwyLDM='

Additional context

Technically (based on reading MDN) the spec for btoa is to run on a string. Buffer.from('1,2,3', 'binary').toString('base64') -> 'MSwyLDM=' (the expected result). However, I suspect, since Buffer.from is overloaded to accept arrays it is encoding the binary values in the array, rather than converting Uint8Array to a string first. Buffer.from(a.toString(), 'binary').toString('base64') yields the "correct" result as well.

The reason this came up is because when running locally everything worked and I spent a while trying to figure out why it didn't work when deployed. The pseudo code I was running:

Server

const f = streamFetchRequest()
for await (const chunk of convertFToAsyncIterable(f))
  yield {
    type: 'audio',
    content: btoa(chunk)
  }

Client

const m = atob(message.content)
queueSoundToPlay(m)
@chetjan
Copy link
Author

chetjan commented Oct 12, 2023

I think I may have confused the environment of the error. The inconsistency remains, but where it is wrong I got flipped. It seems to be wrong when running the development server locally.

Going with the examples above 'AQID' is produced locally, 'MSwyLDM=' is produced on the server. I don't think it is an issue solely with my development environment, but if others are not able to replicate feel free to close this.

Does the polyfill run locally? I am a bit confused as to why this would happen otherwise.

@Kikobeats
Copy link
Member

@chetjan btoa expects a string as input; can you confirm to me it works fine if you do this instead?

btoa(new Uint8Array([1, 2, 3]).toString())

@chetjan
Copy link
Author

chetjan commented Oct 16, 2023

@Kikobeats Yes, that works fine, but the main issue I'm trying to bring attention to is that the polyfill doesn't match the function it is polyfilling. This can cause unexpected behavior when deploying.

In terms of a test spec, what I would expect to pass would be:

import { btoa as btoaPolyfill } from '@edge-runtime/ponyfill'

test('btoa', async () => {
    const value = new Uint8Array([1, 2, 3])
    expect(btoaPolyfill(value)).toBe(btoa(value))
})

Which I suspect would not pass currently.

It may not be worth fixing, the core-js polyfill looks rather complicated. Alternatively, maybe just throw an error if the input is not a string (then it is clear to the developer that the function expects a string).

@Kikobeats
Copy link
Member

@chetjan oh, I can see now your point 👍

How does this look to you? #646

@chetjan
Copy link
Author

chetjan commented Oct 16, 2023

Looks good! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants