fix(core): add custom key encoder and deprecate insecureHash #8379
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
dosubot
bot
added
size:L
| * This function will be removed in a future version. | ||
| */ | ||
| export const insecureHash = (message) => { | ||
| console.warn( |
There was a problem hiding this comment.
Let's just log this once on initial call to avoid flooding the console and add some example code
There was a problem hiding this comment.
add some example code
Can you elaborate on which example code you'd like to be added?
There was a problem hiding this comment.
@jacoblee93 can we maybe link to some docs?
There was a problem hiding this comment.
Ah right... this is used in multiple places
@hntrl maybe let's make a small docs page with agnostic info here?
dosubot
bot
added
size:M
dosubot
bot
added
size:XL
There was a problem hiding this comment.
Hope you don't mind, I picked up on some of the details for this PR to see if we cant get it out a bit quicker. Couple of things to note:
-
I think it's important that we provide what we're declaring as the 'future default' and provide an export for it. Just seems like extra thrash if we're telling people to bring their own hash algo then swapping out the default again entirely.
- On that thread I did some perf testing of a bunch of the different hashing algos implemented in JS to see which one was the most performant (https://github.com/hntrl/js-hash-perf/blob/main/results/analysis.ipynb). I landed on sha256 over sha3, so I'm happy to bundle that into core, but I'm curious to know if that's suitable for your needs/ if there are any other considerations there/ what the security world's opinion is on sha256 (i.e. will another org get on our case for having sha256?)
-
Like Jacob mentioned I added a link to a more detailed warning page with examples in the warning instead of just inlining those details. Would love your opinion on if i'm on message, or if you guys would rework the example code at all.
|
Thank you @hntrl and @jacoblee93 for providing improvements to this PR. I think sha256 is agreat start. It's defintely way stronger hash fn than sha1. The PR LGTM. |
|
Thank you @jacoblee93 and @hntrl for taking the time to help reviewing this work. We (Microsoft) appreciate it. |
3 participants
This PR deprecates the
insecureHashand introduces a support for Custom Key Encoders for Caching and Indexing.Note: The
insecureHashfunction is currently based on SHA-1 algorithm, which may lead to security issues as discussed with @sinedied and @jacoblee93