[v2] atob inconsistency with CF/browser implementation

[dan-lee]

We just ran into a problem with v2, there seems to be an inconsistency with CF atob and node atob:

// CF:
atob(`SGFsbG8\ngV2VsdA==`) // 'Hallo Welt'


// Node
atob(`SGFsbG8\ngV2VsdA==`) //  DOMException [InvalidCharacterError]: Invalid character
//           ↑↑

Buffer.from(`SGFsbG8\ngV2VsdA==`, 'base64').toString() // 'Hallo Welt'

I think the handling of \n is different. Maybe it makes sense to use atobBuffer not only for jest here:

miniflare/packages/core/src/plugins/core.ts

Lines 285 to 288 in c8c2d85

	// Fix for Jest :(, jest-environment-node doesn't include atob/btoa in the
	// global scope
	atob: globalThis.atob ?? atobBuffer,
	btoa: globalThis.btoa ?? btoaBuffer,

[mrbbot]

Hey! 👋

Interestingly, running atob("SGFsbG8\ngV2VsdA==") in my browser (Firefox) actually gives me 'Hallo Welt', not a DOMException. Looking into this further, the spec defines atob as the result of running forgiving-base64 decode on the data, the first step of which is to remove all whitespace characters (\t, \n, \r, ) from the input. So I think this is actually a bug in Node.js's implementation. Indeed, if you look at Node's code, it just checks if characters are valid Base 64:

https://github.com/nodejs/node/blob/1086468aa3d328d2eac00bf66058906553ecd209/lib/buffer.js#L1229-L1239

Therefore, I agree with you that it makes sense to use atobBuffer not only for Jest. I'll do a bit more digging first, but I'll try get this fixed for the next version.

[dan-lee]

Yes, browser and CF implementation seem to be aligned. atob in node.js on the other hand is buggy as you mentioned.

In the types, atob is also mentioned as deprecated:

This function is only provided for compatibility with legacy web platform APIs and should never be used in new code, because they use strings to represent binary data and predate the introduction of typed arrays in JavaScript. For code running using Node.js APIs, converting between base64-encoded strings and binary data should be performed using Buffer.from(str, 'base64') and buf.toString('base64'). Use Buffer.from(data, 'base64') instead.

[mrbbot]

Hey! 👋 miniflare@2.0.0-rc.3 has just been released, including the fix for this. You can find the changelog here.