Remove dependency on `base64-js`

[jonkoops]

The base64-js package has not been maintained for more than 4 years, so it should probably be removed as a dependency.

[dcousens]

@jonkoops do we actually need it for browsers in 2024? Could we simply use atob and btoa now?

[jonkoops]

You are correct @dcousens, that is in fact what we did for Keycloak.

[dcousens]

https://nodejs.org/api/globals.html#atobdata was added in Node 16, so I think we're good if we're assuming LTS for the library (we are)

[jonkoops]

Yeah, this is also an opportunity to align the implementations between Node.js and the browser, might be a good code-cleanup. Did some quick testing, but I was unable to get things to work in a 15 min time-span (had some trouble running the tests on a browser as well).

[dcousens]

@jonkoops if you want to help review #349 🧡

[chjj]

I suspect atob and btoa will have more performance overhead than a custom base64 implementation due to the need for string preprocessing (to match node.js behavior) and the extra conversion back and forth between a binary string a buffer.

I don't have proof because I haven't benchmarked it, but the implementation in #349 is probably the fastest/best we can get.

edit: On second thought, if we optimistically parse base64 with atob and fallback to preprocessing on error, it may be faster? Something like:

try {
  return Buffer.from(atob(str), 'binary');
} catch (e) {
  return Buffer.from(atob(removeInvalidCharacters(str)), 'binary')
}

I'll have a crack at it at some point.

[dcousens]

Using atob/btoa is strongly preferred, if at least for the first attempt for easier code review.
We can optimize afterwards.

[chjj]

Here are the numbers for atob (in node.js):

$ node perf/write-base64.js
BrowserBuffer#write(8192, "base64") x 1,638 ops/sec ±0.50% (93 runs sampled)

Compared to the implementation in #349:

$ node perf/write-base64.js
BrowserBuffer#write(8192, "base64") x 24,542 ops/sec ±0.02% (97 runs sampled)

This was the code used (with the optimized version of asciiWrite):

function base64Write(buf, string, offset, length) {
  return asciiWrite(buf, atob(string), offset, length);
}

If you're scratching your head like I was, look no further than the insanely broken implementation of atob in node.js itself.

This line in particular is the problem:

    const index = ArrayPrototypeIndexOf(
      kForgivingBase64AllowedChars,
      StringPrototypeCharCodeAt(input, n));

In other words, the node.js atob function runs with O(n^2) time complexity! How code this irresponsible made it into the node.js codebase is beyond me.

I'll have to benchmark this in an actual web browser to get some real numbers.

[jonkoops]

In other words, the node.js atob function runs with O(n^2) time complexity! How code this irresponsible made it into the node.js codebase is beyond me.

Oh damn, nice find! I wonder how other run-times such as Bun and Deno compare in this regard. Did you log an issue with the Node.js team?

[dcousens]

What is the browser performance like? We could probably still rely on atob and btoa and then log the nodejs issue upstream?

[vweevers]

In other words, the node.js atob function runs with O(n^2) time complexity! How code this irresponsible made it into the node.js codebase is beyond me.

Because Buffer exists as the better alternative (from Node.js' perspective that is). The atob function is marked as Legacy:

Stability: 3 - Legacy. Although this feature is unlikely to be removed and is still covered by semantic versioning guarantees, it is no longer actively maintained, and other alternatives are available.

Which explains this comment:

// The implementation here has not been performance optimized in any way and
// should not be.

That said, you could make the argument that this hurts web compatibility, and still file an issue.

[jonkoops]

So perhaps take the Buffer.from() approach for Node.js and see if the perf for atob in browsers is acceptable? Would prevent the need to roll a bunch of custom code.