Skip to content

Commit

Permalink
crypto: simplify state failure handling
Browse files Browse the repository at this point in the history
It is more intuitive to return true/false instead of undefined/false
and also simplifies handling in the JS layer.

PR-URL: #22131
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
  • Loading branch information
tniessen authored and rvagg committed Aug 15, 2018
1 parent 916a1d5 commit 9212875
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 11 deletions.
9 changes: 3 additions & 6 deletions lib/internal/crypto/cipher.js
Expand Up @@ -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;
};
Expand All @@ -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;
};
Expand All @@ -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;
};
Expand Down
12 changes: 7 additions & 5 deletions src/node_crypto.cc
Expand Up @@ -2926,6 +2926,8 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& 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);
}


Expand Down Expand Up @@ -2980,9 +2982,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo<Value>& args) {
CHECK(args[1]->IsInt32());
int plaintext_len = args[1].As<Int32>()->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
}


Expand Down Expand Up @@ -3094,8 +3096,8 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo<Value>& 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
}


Expand Down

0 comments on commit 9212875

Please sign in to comment.