Skip to content

Commit

Permalink
crypto: remove default encoding from sign/verify
Browse files Browse the repository at this point in the history
getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires
some careful justification but the default encoding can be eliminated
from sig.js entirely.

In Sign.prototype.update, we can safely remove the conditional
assignment of getDefaultEncoding() to encoding. This is because
SignUpdate() in crypto_sig.cc internally calls node::crypto::Decode,
which returns UTF8 for falsy encoding values. In other words, with the
conditional assignment, StringBytes::Write() ultimately receives the
encoding BUFFER, and without the conditional assignment, it receives the
encoding UTF8. However, StringBytes::Write() treats both encodings
identically, so there is no need to deviate from the internal default
encoding UTF8.

In Sign.prototype.sign, we can also safely remove the conditional
assignment of getDefaultEncoding() to encoding. Whether encoding is
falsy or 'buffer' makes no difference.

In Verify.prototype.verify, we can also safely remove the conditional
assignment of getDefaultEncoding() to sigEncoding. This is because the
function passes the sigEncoding to getArrayBufferOrView(), which passes
it to Buffer.from(). If sigEncoding is 'buffer', getArrayBufferOrView()
instead passes 'utf8' to Buffer.from(). Because the default encoding of
Buffer.from() is 'utf8', passing a falsy encoding to
getArrayBufferOrView() instead of 'buffer' results in the same behavior.

Refs: #47182
PR-URL: #49145
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
tniessen authored and UlisesGascon committed Sep 10, 2023
1 parent 173aed4 commit 08197aa
Showing 1 changed file with 0 additions and 6 deletions.
6 changes: 0 additions & 6 deletions lib/internal/crypto/sig.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const {

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

Expand Down Expand Up @@ -70,8 +69,6 @@ Sign.prototype._write = function _write(chunk, encoding, callback) {
};

Sign.prototype.update = function update(data, encoding) {
encoding = encoding || getDefaultEncoding();

if (typeof data === 'string') {
validateEncoding(data, encoding);
} else if (!isArrayBufferView(data)) {
Expand Down Expand Up @@ -131,7 +128,6 @@ Sign.prototype.sign = function sign(options, encoding) {
const ret = this[kHandle].sign(data, format, type, passphrase, rsaPadding,
pssSaltLength, dsaSigEnc);

encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
return ret.toString(encoding);

Expand Down Expand Up @@ -216,8 +212,6 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
passphrase,
} = preparePublicOrPrivateKey(options, true);

sigEncoding = sigEncoding || getDefaultEncoding();

// Options specific to RSA
const rsaPadding = getPadding(options);
const pssSaltLength = getSaltLength(options);
Expand Down

0 comments on commit 08197aa

Please sign in to comment.