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

Remove crypto.randomUUID() from client side #90

Merged
merged 3 commits into from
Mar 18, 2023

Conversation

HyunggyuJang
Copy link
Contributor

Unless, if we tries to access the client side page from http protocol, crypto.randomUUID() isn't available.

Unless, if we tries to access the client side page from http protocol, crypto.randomUUID() isn't available.
@wtlyu
Copy link
Contributor

wtlyu commented Mar 18, 2023

I got your idea. but it's not safe to use hardcoded messageId. mesaageId should never be duplicated, in any cases. do we have another random way to generate uuid-like messageId?

@wtlyu
Copy link
Contributor

wtlyu commented Mar 18, 2023

Update. it's not limited to be a uuid. any string that not duplicated is acceptable

@wtlyu
Copy link
Contributor

wtlyu commented Mar 18, 2023

crypto.randomUUID() is now standard on all modern browsers and JS runtimes. However, because new browser APIs are restricted to secure contexts, this method is only available to pages served locally (localhost or 127.0.0.1) or over HTTPS.

For readers interested in other UUID versions, generating UUIDs on legacy platforms or in non-secure contexts, there is the uuid module. It is well-tested and supported

from Google. that would be a better choice

@wtlyu
Copy link
Contributor

wtlyu commented Mar 18, 2023

https://www.npmjs.com/package/uuid

seems to be a replace choice. to strictly avoid duplicated id, we can add timestamp to the uuid string.

@HyunggyuJang
Copy link
Contributor Author

https://www.npmjs.com/package/uuid

seems to be a replace choice. to strictly avoid duplicated id, we can add timestamp to the uuid string.

LGTM! I'll do that in a minute. Thanks for your elaboration!

@HyunggyuJang
Copy link
Contributor Author

HyunggyuJang commented Mar 18, 2023

@wtlyu I've gone with

crypto.getRandomValues(new Uint8Array(1))[0].toString() + Date.now()

Will it be sufficient?

@HyunggyuJang HyunggyuJang changed the title Remove crypto from client side Remove crypto.randomUUID() from client side Mar 18, 2023
@wtlyu
Copy link
Contributor

wtlyu commented Mar 18, 2023

looks good. do you test it?

@HyunggyuJang
Copy link
Contributor Author

Yes it works fine in my env!

@danny-avila danny-avila merged commit c7f8b35 into danny-avila:master Mar 18, 2023
jinzishuai pushed a commit to jinzishuai/LibreChat that referenced this pull request Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants