From 9212875406ab2e15c9a07a2c111e1a8065d7ced8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sat, 4 Aug 2018 23:52:37 +0200 Subject: [PATCH] crypto: simplify state failure handling It is more intuitive to return true/false instead of undefined/false and also simplifies handling in the JS layer. PR-URL: https://github.com/nodejs/node/pull/22131 Reviewed-By: Jon Moss Reviewed-By: Luigi Pinca Reviewed-By: Ujjwal Sharma --- lib/internal/crypto/cipher.js | 9 +++------ src/node_crypto.cc | 12 +++++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index fc8ee88742d135..d876010cf99159 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -181,7 +181,7 @@ Cipher.prototype.final = function final(outputEncoding) { Cipher.prototype.setAutoPadding = function setAutoPadding(ap) { - if (this._handle.setAutoPadding(ap) === false) + if (!this._handle.setAutoPadding(ap)) throw new ERR_CRYPTO_INVALID_STATE('setAutoPadding'); return this; }; @@ -200,10 +200,7 @@ Cipher.prototype.setAuthTag = function setAuthTag(tagbuf) { ['Buffer', 'TypedArray', 'DataView'], tagbuf); } - // Do not do a normal falsy check because the method returns - // undefined if it succeeds. Returns false specifically if it - // errored - if (this._handle.setAuthTag(tagbuf) === false) + if (!this._handle.setAuthTag(tagbuf)) throw new ERR_CRYPTO_INVALID_STATE('setAuthTag'); return this; }; @@ -216,7 +213,7 @@ Cipher.prototype.setAAD = function setAAD(aadbuf, options) { } const plaintextLength = getUIntOption(options, 'plaintextLength'); - if (this._handle.setAAD(aadbuf, plaintextLength) === false) + if (!this._handle.setAAD(aadbuf, plaintextLength)) throw new ERR_CRYPTO_INVALID_STATE('setAAD'); return this; }; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 735a470d2c5499..c7b8c218b88af3 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2926,6 +2926,8 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { memset(cipher->auth_tag_, 0, sizeof(cipher->auth_tag_)); memcpy(cipher->auth_tag_, Buffer::Data(args[0]), cipher->auth_tag_len_); + + args.GetReturnValue().Set(true); } @@ -2980,9 +2982,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo& args) { CHECK(args[1]->IsInt32()); int plaintext_len = args[1].As()->Value(); - if (!cipher->SetAAD(Buffer::Data(args[0]), Buffer::Length(args[0]), - plaintext_len)) - args.GetReturnValue().Set(false); // Report invalid state failure + bool b = cipher->SetAAD(Buffer::Data(args[0]), Buffer::Length(args[0]), + plaintext_len); + args.GetReturnValue().Set(b); // Possibly report invalid state failure } @@ -3094,8 +3096,8 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - if (!cipher->SetAutoPadding(args.Length() < 1 || args[0]->BooleanValue())) - args.GetReturnValue().Set(false); // Report invalid state failure + bool b = cipher->SetAutoPadding(args.Length() < 1 || args[0]->BooleanValue()); + args.GetReturnValue().Set(b); // Possibly report invalid state failure }