-
Notifications
You must be signed in to change notification settings - Fork 31.3k
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
crypto: fix Hash and Cipher abort on end #38425
base: main
Are you sure you want to change the base?
Conversation
Does it also fix issue in #38035 (comment)? It contains invalid string ( |
@Linkgoron I think a better approach overall would be validating this on the C++ side instead, where the abort occurs in the first place. |
fix Hash and Cipher aborting when using end with hex and specific lengths of chunks fixes: nodejs#38015
5e277fd
to
b89f9ab
Compare
I checked now, and it fixes the issue in the comment - or at least it causes an error to get emitted instead of an abort.
Currently it appears to me that the validation in both cases (Hash and Cipher) is already done on the JS side in the |
Fix
Hash
andCipher
aborting when usingend
withhex
and specific lengths of chunks. The issue was caused because there was missing validation on the written content from theend
method. Note that this actually affects quite a few things in Crypto. This affects decipher/cipher (both deprecated) but also decipheriv/cipheriv as well as Hash and Hmac.I wasn't sure if this should throw in
_transform
or create an error and provide it to the callback, but according to the stream docs providing an error to the callback is what's expected - so I would love to get some input on my fix, and if it makes sense.Fixes: #38015