-
Notifications
You must be signed in to change notification settings - Fork 30.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
Why crypto.createHash() is not writable until setEncoding() is called? #12269
Comments
cc @nodejs/streams - this doesn't come out of lib/crypto.js as far as I can tell. |
@bnoordhuis I will have a look. I think it's |
it's coming out of lazyTransform, does removing the console log change anything ? |
I think the problem is that https://github.com/nodejs/node/blob/master/lib/internal/streams/legacy.js#L11-L91 does not cause the lazyness of LazyTransform triggered. |
@mcollina but in the example, the console.log will actually trigger LazyTransform |
@calvinmetcalf why? the lazyness is on |
ignore me, I thought he was calling console.log on _writableState |
I found out the root of the problem. Actually this code can reproduce it:
So, the main reason is the A fix could be another backward compatibility hack - setting |
Makes LazyTransform writable by Streams1 by assigning .writable = true before the actual classes are loaded. Fixes: nodejs#12269
Makes LazyTransform writable by Streams1 by assigning .writable = true before the actual classes are loaded. Fixes: nodejs/node#12269 PR-URL: nodejs/node#12380 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Please look at the code below:
So, if one use
hasher
with the legacystream.pipe()
implementation there will be an error, because the hash will be always calculated from the empty value.It's easy to reproduce:
The text was updated successfully, but these errors were encountered: