crypto: simplify state failure handling

tniessen · targos · commit bef02abb7da4 · 2018-08-11T18:10:10.000+02:00
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>
diff --git 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
@@ -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);
 }
 
 
@@ -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
 }
 
 
@@ -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
 }
 
 
