dhensby / node-http-message-signatures

A node package for signing HTTP messages as per the http-message-signing draft specification
ISC License
13 stars 8 forks source link

Replace Buffer with Uint8Array #160

Open 0x0ece opened 8 months ago

0x0ece commented 8 months ago

I don't know why base64 is so hard in js. My recommendation would be to use https://github.com/paulmillr/scure-base, but lmk your thoughts. (btw, if you ever want to add algorithms also for client, there's https://github.com/paulmillr/noble-curves from the same author).

I have node18 on my system, when I added the new dependency it seems it made additional changes to the package.json and lock file. If you can share what version of node you're using, I can probably fix that.

All tests pass on my machine. Interestingly enough, using scure I found some extra spaces in some base64 encodings (scure is more strict).

dhensby commented 8 months ago

Please reword your commit message so that it conforms to the commit linting spec (feat: replace Buffer with Uint8Array)

base64 shouldn't be too hard in browser envs (see https://developer.mozilla.org/en-US/docs/Glossary/Base64#javascript_support). Is an external lib necessary when we only need basic base64 encode/decode, especially when the lib provides so much more?

This removes the use of Buffer, which is nice, but it doesn't actually make it work in browser environments because it still depends on node:crypto package, right?

At the moment minimum support is node 12, so if you install packages using node 12 locally, that should keep the package-lock.json compatible

0x0ece commented 8 months ago

This removes the use of Buffer, which is nice, but it doesn't actually make it work in browser environments because it still depends on node:crypto package, right?

Correct, we'd need a way to conditionally include algorithms for server and not for client. Or, eventually, reimplement all algorithms client-side.

I'll make the changes (but need a bit of time).