danakt / uuid-by-string

Generates the RFC-4122 Name-Based UUID
MIT License
66 stars 12 forks source link

Consider using `crypto` to generate md5/sha1 hashes #24

Open wovalle opened 1 year ago

wovalle commented 1 year ago

First of all, thank you for the library!

Have you considered using crypto.createHash to create md5/sha1 hashes instead of the js dependencies?

This would allow us to use your libraries in non-node runtimes like cloudflare workers (currently it fails because js-md5 contains eval which is usually a no-no for security reasons).

I created a temporary fork and replaced the dependencies for the native crypto and all tests passed succesfully, can create a PR if you agree.

Thanks!

doroved commented 1 year ago

First of all, thank you for the library!

Have you considered using crypto.createHash to create md5/sha1 hashes instead of the js dependencies?

This would allow us to use your libraries in non-node runtimes like cloudflare workers (currently it fails because js-md5 contains eval which is usually a no-no for security reasons).

I created a temporary fork and replaced the dependencies for the native crypto and all tests passed succesfully, can create a PR if you agree.

Thanks!

Hi, is it possible to adapt this script to work in a browser?

wovalle commented 1 year ago

I think this library is only exported as a commonjs module. If it is exported as an esmodule + those two dependencies I mentioned are replaced, this would be 100% browser compatible.

titanism commented 1 month ago

@wovalle where is this fork/repo at?

titanism commented 1 month ago

Ah, it's at https://github.com/luchyio/uuid-by-string

titanism commented 1 month ago

We've published @forwardemail/uuid-by-string in the interim to npm. Can you please submit a PR so we can get this in the main repo of uuid-by-string for the wider npm community? @wovalle

titanism commented 1 month ago

@wovalle one small note, you should add this to the top of your src/lib.js file before sending a PR over:

+const crypto = require('node:crypto');
src/lib.js
73:  return new Uint8Array(crypto.createHash("md5").update(buf).digest());
82:  return new Uint8Array(crypto.createHash("sha1").update(buf).digest());

Otherwise current tests do not pass:

Screen Shot 2024-08-01 at 5 15 18 AM

You can see we've done this already at: https://github.com/forwardemail/uuid-by-string/commit/ab07dd6ca1f14e9103ea53887713d41847e80a10

wovalle commented 1 month ago

First of all, so cool that a service I've used in the past (forwardemail) cloned one of my repos :D

Sure, just added node:crypto @titanism

wovalle commented 1 month ago

Also created the PR #37 to merge back to upstream. Rebased my changes on top of current master so the commit is clean 👍🏽

Let me know if you have any questions @danakt