Skip to content

Commit

Permalink
crypto: do not overwrite _writableState.defaultEncoding
Browse files Browse the repository at this point in the history
This only affects the writable side of LazyTransform and should not
change the behavior of any LazyTransform streams (Cipher, Decipher,
Cipheriv, Decipheriv, Hash, Hmac).

If the user does not set defaultEncoding when creating a transform
stream, WritableState uses 'utf8' by default. Only LazyTransform
overwrites this with the return value of getDefaultEncoding(). This
was necessary when crypto.DEFAULT_ENCODING still existed. Now that
DEFAULT_ENCODING has been removed, getDefaultEncoding() always returns
'buffer'. The writable side of LazyTransform appears to treat 'utf8'
and 'buffer' in exactly the same way. Therefore, there seems to be no
need to overwrite _writableState.defaultEncoding at this point.

Nevertheless, because Node.js has failed to hide implementation details
such as _writableState from the ecosystem, we may want to consider this
a breaking change.

Refs: nodejs#47182
Refs: nodejs#8611
  • Loading branch information
tniessen committed Aug 13, 2023
1 parent 87af913 commit de5cc70
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 9 deletions.
9 changes: 0 additions & 9 deletions lib/internal/streams/lazy_transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ const {

const stream = require('stream');

const {
getDefaultEncoding,
} = require('internal/crypto/util');

module.exports = LazyTransform;

function LazyTransform(options) {
Expand All @@ -27,11 +23,6 @@ function makeGetter(name) {
return function() {
stream.Transform.call(this, this._options);
this._writableState.decodeStrings = false;

if (!this._options || !this._options.defaultEncoding) {
this._writableState.defaultEncoding = getDefaultEncoding();
}

return this[name];
};
}
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,9 @@ function testEncoding(options, assertionHash) {
let hashValue = '';

hash.on('data', (data) => {
// The defaultEncoding has no effect on the hash value. It only affects data
// consumed by the Hash transform stream.
assert(Buffer.isBuffer(data));
hashValue += data.toString('hex');
});

Expand All @@ -307,6 +310,8 @@ function testEncoding(options, assertionHash) {

hash.write('öäü');
hash.end();

assert.strictEqual(hash._writableState.defaultEncoding, options?.defaultEncoding ?? 'utf8');
}

// Hash of "öäü" in utf8 format
Expand Down

0 comments on commit de5cc70

Please sign in to comment.