diff --git a/src/env.h b/src/env.h index 47723baa7859ff..ba1558a13d5ac7 100644 --- a/src/env.h +++ b/src/env.h @@ -133,6 +133,7 @@ class ModuleWrap; V(dns_txt_string, "TXT") \ V(domain_string, "domain") \ V(emit_string, "emit") \ + V(emit_warning_string, "emitWarning") \ V(exchange_string, "exchange") \ V(enumerable_string, "enumerable") \ V(idle_string, "idle") \ diff --git a/src/node.cc b/src/node.cc index ed2f0c14ff3c59..fe564ae4898e5e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -147,12 +147,15 @@ using v8::HandleScope; using v8::HeapStatistics; using v8::Integer; using v8::Isolate; +using v8::Just; using v8::Local; using v8::Locker; +using v8::Maybe; using v8::MaybeLocal; using v8::Message; using v8::Name; using v8::NamedPropertyHandlerConfiguration; +using v8::Nothing; using v8::Null; using v8::Number; using v8::Object; @@ -2277,8 +2280,11 @@ static void DLOpen(const FunctionCallbackInfo& args) { } if (mp->nm_version == -1) { if (env->EmitNapiWarning()) { - ProcessEmitWarning(env, "N-API is an experimental feature and could " - "change at any time."); + if (ProcessEmitWarning(env, "N-API is an experimental feature and could " + "change at any time.").IsNothing()) { + dlib.Close(); + return; + } } } else if (mp->nm_version != NODE_MODULE_VERSION) { char errmsg[1024]; @@ -2425,8 +2431,62 @@ static void OnMessage(Local message, Local error) { FatalException(Isolate::GetCurrent(), error, message); } +static Maybe ProcessEmitWarningGeneric(Environment* env, + const char* warning, + const char* type = nullptr, + const char* code = nullptr) { + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); + + Local process = env->process_object(); + Local emit_warning; + if (!process->Get(env->context(), + env->emit_warning_string()).ToLocal(&emit_warning)) { + return Nothing(); + } + + if (!emit_warning->IsFunction()) return Just(false); + + int argc = 0; + Local args[3]; // warning, type, code + + // The caller has to be able to handle a failure anyway, so we might as well + // do proper error checking for string creation. + if (!String::NewFromUtf8(env->isolate(), + warning, + v8::NewStringType::kNormal).ToLocal(&args[argc++])) { + return Nothing(); + } + if (type != nullptr) { + if (!String::NewFromOneByte(env->isolate(), + reinterpret_cast(type), + v8::NewStringType::kNormal) + .ToLocal(&args[argc++])) { + return Nothing(); + } + if (code != nullptr && + !String::NewFromOneByte(env->isolate(), + reinterpret_cast(code), + v8::NewStringType::kNormal) + .ToLocal(&args[argc++])) { + return Nothing(); + } + } + + // MakeCallback() unneeded because emitWarning is internal code, it calls + // process.emit('warning', ...), but does so on the nextTick. + if (emit_warning.As()->Call(env->context(), + process, + argc, + args).IsEmpty()) { + return Nothing(); + } + return Just(true); +} + + // Call process.emitWarning(str), fmt is a snprintf() format string -void ProcessEmitWarning(Environment* env, const char* fmt, ...) { +Maybe ProcessEmitWarning(Environment* env, const char* fmt, ...) { char warning[1024]; va_list ap; @@ -2434,24 +2494,20 @@ void ProcessEmitWarning(Environment* env, const char* fmt, ...) { vsnprintf(warning, sizeof(warning), fmt, ap); va_end(ap); - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - - Local process = env->process_object(); - MaybeLocal emit_warning = process->Get(env->context(), - FIXED_ONE_BYTE_STRING(env->isolate(), "emitWarning")); - Local arg = node::OneByteString(env->isolate(), warning); - - Local f; + return ProcessEmitWarningGeneric(env, warning); +} - if (!emit_warning.ToLocal(&f)) return; - if (!f->IsFunction()) return; - // MakeCallback() unneeded, because emitWarning is internal code, it calls - // process.emit('warning', ..), but does so on the nextTick. - f.As()->Call(process, 1, &arg); +Maybe ProcessEmitDeprecationWarning(Environment* env, + const char* warning, + const char* deprecation_code) { + return ProcessEmitWarningGeneric(env, + warning, + "DeprecationWarning", + deprecation_code); } + static bool PullFromCache(Environment* env, const FunctionCallbackInfo& args, Local module, diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 6e9d2255cdd430..8d5efe4d6b426b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1062,8 +1062,12 @@ void SecureContext::AddRootCerts(const FunctionCallbackInfo& args) { root_cert_store, extra_root_certs_file.c_str()); if (err) { + // We do not call back into JS after this line anyway, so ignoring + // the return value of ProcessEmitWarning does not affect how a + // possible exception would be propagated. ProcessEmitWarning(sc->env(), - "Ignoring extra certs from `%s`, load failed: %s\n", + "Ignoring extra certs from `%s`, " + "load failed: %s\n", extra_root_certs_file.c_str(), ERR_error_string(err, nullptr)); } @@ -3618,7 +3622,10 @@ void CipherBase::Init(const char* cipher_type, int mode = EVP_CIPHER_CTX_mode(ctx_); if (encrypt && (mode == EVP_CIPH_CTR_MODE || mode == EVP_CIPH_GCM_MODE || mode == EVP_CIPH_CCM_MODE)) { - ProcessEmitWarning(env(), "Use Cipheriv for counter mode of %s", + // Ignore the return value (i.e. possible exception) because we are + // not calling back into JS anyway. + ProcessEmitWarning(env(), + "Use Cipheriv for counter mode of %s", cipher_type); } diff --git a/src/node_internals.h b/src/node_internals.h index 5466736200d5b8..09bcbba6e0b39f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -287,7 +287,10 @@ class FatalTryCatch : public v8::TryCatch { Environment* env_; }; -void ProcessEmitWarning(Environment* env, const char* fmt, ...); +v8::Maybe ProcessEmitWarning(Environment* env, const char* fmt, ...); +v8::Maybe ProcessEmitDeprecationWarning(Environment* env, + const char* warning, + const char* deprecation_code); void FillStatsArray(double* fields, const uv_stat_t* s); diff --git a/test/parallel/test-process-emit-warning-from-native.js b/test/parallel/test-process-emit-warning-from-native.js new file mode 100644 index 00000000000000..d50ea3962bdedc --- /dev/null +++ b/test/parallel/test-process-emit-warning-from-native.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +if (!common.hasCrypto) + common.skip('missing crypto'); +if (common.hasFipsCrypto) + common.skip('crypto.createCipher() is not supported in FIPS mode'); + +const crypto = require('crypto'); +const key = '0123456789'; + +{ + common.expectWarning('Warning', + 'Use Cipheriv for counter mode of aes-256-gcm'); + + // Emits regular warning expected by expectWarning() + crypto.createCipher('aes-256-gcm', key); +} + +const realEmitWarning = process.emitWarning; + +{ + // It's a good idea to make this overridable from userland. + process.emitWarning = () => { throw new Error('foo'); }; + assert.throws(() => { + crypto.createCipher('aes-256-gcm', key); + }, /^Error: foo$/); +} + +{ + Object.defineProperty(process, 'emitWarning', { + get() { throw new Error('bar'); }, + configurable: true + }); + assert.throws(() => { + crypto.createCipher('aes-256-gcm', key); + }, /^Error: bar$/); +} + +// Reset back to default after the test. +Object.defineProperty(process, 'emitWarning', { + value: realEmitWarning, + configurable: true, + writable: true +}); diff --git a/test/parallel/test-tls-env-bad-extra-ca.js b/test/parallel/test-tls-env-bad-extra-ca.js index f7e9341ad00757..e141de34a9fe1b 100644 --- a/test/parallel/test-tls-env-bad-extra-ca.js +++ b/test/parallel/test-tls-env-bad-extra-ca.js @@ -18,7 +18,7 @@ if (process.env.CHILD) { const env = Object.assign({}, process.env, { CHILD: 'yes', - NODE_EXTRA_CA_CERTS: `${fixtures.fixturesDir}/no-such-file-exists`, + NODE_EXTRA_CA_CERTS: `${fixtures.fixturesDir}/no-such-file-exists-🐢`, }); const opts = { @@ -32,8 +32,12 @@ fork(__filename, opts) assert.strictEqual(status, 0, 'client did not succeed in connecting'); })) .on('close', common.mustCall(function() { - const re = /Warning: Ignoring extra certs from.*no-such-file-exists.* load failed:.*No such file or directory/; - assert(re.test(stderr), stderr); + // TODO(addaleax): Make `SafeGetenv` work like `process.env` + // encoding-wise + if (!common.isWindows) { + const re = /Warning: Ignoring extra certs from.*no-such-file-exists-🐢.* load failed:.*No such file or directory/; + assert(re.test(stderr), stderr); + } })) .stderr.setEncoding('utf8').on('data', function(str) { stderr += str;