diff --git a/src/crypto/crypto_cipher.h b/src/crypto/crypto_cipher.h index e725a2f30dc6b0..d74fa4447b7b31 100644 --- a/src/crypto/crypto_cipher.h +++ b/src/crypto/crypto_cipher.h @@ -249,9 +249,8 @@ class CipherJob final : public CryptoJob { } } - v8::Maybe ToResult( - v8::Local* err, - v8::Local* result) override { + v8::Maybe ToResult(v8::Local* err, + v8::Local* result) override { Environment* env = AsyncWrap::env(); CryptoErrorStore* errors = CryptoJob::errors(); @@ -262,11 +261,18 @@ class CipherJob final : public CryptoJob { CHECK(errors->Empty()); *err = v8::Undefined(env->isolate()); *result = out_.ToArrayBuffer(env); - return v8::Just(!result->IsEmpty()); + if (result->IsEmpty()) { + return v8::Nothing(); + } + } else { + *result = v8::Undefined(env->isolate()); + if (!errors->ToException(env).ToLocal(err)) { + return v8::Nothing(); + } } - - *result = v8::Undefined(env->isolate()); - return v8::Just(errors->ToException(env).ToLocal(err)); + CHECK(!result->IsEmpty()); + CHECK(!err->IsEmpty()); + return v8::JustVoid(); } SET_SELF_SIZE(CipherJob) diff --git a/src/crypto/crypto_keygen.cc b/src/crypto/crypto_keygen.cc index aa0c29e8f7e62d..a8ddc0df4611bc 100644 --- a/src/crypto/crypto_keygen.cc +++ b/src/crypto/crypto_keygen.cc @@ -14,7 +14,6 @@ namespace node { using v8::FunctionCallbackInfo; using v8::Int32; -using v8::Just; using v8::JustVoid; using v8::Local; using v8::Maybe; @@ -81,7 +80,7 @@ KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(Environment* env, } MaybeLocal SecretKeyGenTraits::EncodeKey(Environment* env, - SecretKeyGenConfig* params) { + SecretKeyGenConfig* params) { std::shared_ptr data = KeyObjectData::CreateSecret(std::move(params->out)); Local ret; diff --git a/src/crypto/crypto_keygen.h b/src/crypto/crypto_keygen.h index 9ec5fc7bf5ba84..aa4a66df804028 100644 --- a/src/crypto/crypto_keygen.h +++ b/src/crypto/crypto_keygen.h @@ -91,9 +91,8 @@ class KeyGenJob final : public CryptoJob { } } - v8::Maybe ToResult( - v8::Local* err, - v8::Local* result) override { + v8::Maybe ToResult(v8::Local* err, + v8::Local* result) override { Environment* env = AsyncWrap::env(); CryptoErrorStore* errors = CryptoJob::errors(); AdditionalParams* params = CryptoJob::params(); @@ -108,14 +107,17 @@ class KeyGenJob final : public CryptoJob { *result = Undefined(env->isolate()); *err = try_catch.Exception(); } - return v8::Just(true); + } else { + if (errors->Empty()) errors->Capture(); + CHECK(!errors->Empty()); + *result = Undefined(env->isolate()); + if (!errors->ToException(env).ToLocal(err)) { + return v8::Nothing(); + } } - - if (errors->Empty()) - errors->Capture(); - CHECK(!errors->Empty()); - *result = Undefined(env->isolate()); - return v8::Just(errors->ToException(env).ToLocal(err)); + CHECK(!result->IsEmpty()); + CHECK(!err->IsEmpty()); + return v8::JustVoid(); } SET_SELF_SIZE(KeyGenJob) @@ -183,9 +185,8 @@ struct KeyPairGenTraits final { return KeyGenJobStatus::OK; } - static v8::MaybeLocal EncodeKey( - Environment* env, - AdditionalParameters* params) { + static v8::MaybeLocal EncodeKey(Environment* env, + AdditionalParameters* params) { v8::Local keys[2]; if (params->key .ToEncodedPublicKey(env, params->public_key_encoding, &keys[0]) @@ -224,9 +225,8 @@ struct SecretKeyGenTraits final { Environment* env, SecretKeyGenConfig* params); - static v8::MaybeLocal EncodeKey( - Environment* env, - SecretKeyGenConfig* params); + static v8::MaybeLocal EncodeKey(Environment* env, + SecretKeyGenConfig* params); }; template diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h index 11d4e5cdf71e02..ceefff2bc0fdaf 100644 --- a/src/crypto/crypto_keys.h +++ b/src/crypto/crypto_keys.h @@ -369,23 +369,28 @@ class KeyExportJob final : public CryptoJob { } } - v8::Maybe ToResult( - v8::Local* err, - v8::Local* result) override { + v8::Maybe ToResult(v8::Local* err, + v8::Local* result) override { Environment* env = AsyncWrap::env(); CryptoErrorStore* errors = CryptoJob::errors(); if (out_.size() > 0) { CHECK(errors->Empty()); *err = v8::Undefined(env->isolate()); *result = out_.ToArrayBuffer(env); - return v8::Just(!result->IsEmpty()); + if (result->IsEmpty()) { + return v8::Nothing(); + } + } else { + if (errors->Empty()) errors->Capture(); + CHECK(!errors->Empty()); + *result = v8::Undefined(env->isolate()); + if (!errors->ToException(env).ToLocal(err)) { + return v8::Nothing(); + } } - - if (errors->Empty()) - errors->Capture(); - CHECK(!errors->Empty()); - *result = v8::Undefined(env->isolate()); - return v8::Just(errors->ToException(env).ToLocal(err)); + CHECK(!result->IsEmpty()); + CHECK(!err->IsEmpty()); + return v8::JustVoid(); } SET_SELF_SIZE(KeyExportJob) diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 63df2fd937ca9f..922e77091d7217 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -361,31 +361,31 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork { v8::HandleScope handle_scope(env->isolate()); v8::Context::Scope context_scope(env->context()); - // TODO(tniessen): Remove the exception handling logic here as soon as we - // can verify that no code path in ToResult will ever throw an exception. v8::Local exception; v8::Local args[2]; { node::errors::TryCatchScope try_catch(env); - v8::Maybe ret = ptr->ToResult(&args[0], &args[1]); - if (!ret.IsJust()) { + // If ToResult returns Nothing, then an exception should have been + // thrown and we should have caught it. Otherwise, args[0] and args[1] + // both should have been set to a value, even if the value is undefined. + if (ptr->ToResult(&args[0], &args[1]).IsNothing()) { CHECK(try_catch.HasCaught()); + CHECK(try_catch.CanContinue()); exception = try_catch.Exception(); - } else if (!ret.FromJust()) { - return; } } - if (exception.IsEmpty()) { - ptr->MakeCallback(env->ondone_string(), arraysize(args), args); - } else { + if (!exception.IsEmpty()) { ptr->MakeCallback(env->ondone_string(), 1, &exception); + } else { + CHECK(!args[0].IsEmpty()); + CHECK(!args[1].IsEmpty()); + ptr->MakeCallback(env->ondone_string(), arraysize(args), args); } } - virtual v8::Maybe ToResult( - v8::Local* err, - v8::Local* result) = 0; + virtual v8::Maybe ToResult(v8::Local* err, + v8::Local* result) = 0; CryptoJobMode mode() const { return mode_; } @@ -413,8 +413,9 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork { v8::Local ret[2]; env->PrintSyncTrace(); job->DoThreadPoolWork(); - v8::Maybe result = job->ToResult(&ret[0], &ret[1]); - if (result.IsJust() && result.FromJust()) { + if (job->ToResult(&ret[0], &ret[1]).IsJust()) { + CHECK(!ret[0].IsEmpty()); + CHECK(!ret[1].IsEmpty()); args.GetReturnValue().Set( v8::Array::New(env->isolate(), ret, arraysize(ret))); } @@ -504,9 +505,8 @@ class DeriveBitsJob final : public CryptoJob { success_ = true; } - v8::Maybe ToResult( - v8::Local* err, - v8::Local* result) override { + v8::Maybe ToResult(v8::Local* err, + v8::Local* result) override { Environment* env = AsyncWrap::env(); CryptoErrorStore* errors = CryptoJob::errors(); if (success_) { @@ -515,16 +515,19 @@ class DeriveBitsJob final : public CryptoJob { if (!DeriveBitsTraits::EncodeOutput( env, *CryptoJob::params(), &out_) .ToLocal(result)) { - return v8::Nothing(); + return v8::Nothing(); + } + } else { + if (errors->Empty()) errors->Capture(); + CHECK(!errors->Empty()); + *result = v8::Undefined(env->isolate()); + if (!errors->ToException(env).ToLocal(err)) { + return v8::Nothing(); } - return v8::Just(true); } - - if (errors->Empty()) - errors->Capture(); - CHECK(!errors->Empty()); - *result = v8::Undefined(env->isolate()); - return v8::Just(errors->ToException(env).ToLocal(err)); + CHECK(!result->IsEmpty()); + CHECK(!err->IsEmpty()); + return v8::JustVoid(); } SET_SELF_SIZE(DeriveBitsJob)