diff options
author | Dominic Elm <elmdominic@gmx.net> | 2021-09-02 16:17:42 +0300 |
---|---|---|
committer | Bradley Farias <bradley.meck@gmail.com> | 2021-09-10 18:54:39 +0300 |
commit | a9dd03b1ec89a75186f05967fc76ec0704050c36 (patch) | |
tree | db9cd062535160d91bd45f178b9761259ca9e2dd | |
parent | a42bd7e944ed422adb78f767c7a9b3187989234a (diff) |
src: add option to disable loading native addons
PR-URL: https://github.com/nodejs/node/pull/39977
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
-rw-r--r-- | doc/api/cli.md | 10 | ||||
-rw-r--r-- | doc/api/errors.md | 9 | ||||
-rw-r--r-- | doc/api/packages.md | 20 | ||||
-rw-r--r-- | doc/node.1 | 5 | ||||
-rw-r--r-- | lib/internal/modules/cjs/helpers.js | 10 | ||||
-rw-r--r-- | lib/internal/modules/esm/resolve.js | 11 | ||||
-rw-r--r-- | src/env-inl.h | 5 | ||||
-rw-r--r-- | src/env.h | 1 | ||||
-rw-r--r-- | src/node.h | 8 | ||||
-rw-r--r-- | src/node_binding.cc | 6 | ||||
-rw-r--r-- | src/node_errors.h | 1 | ||||
-rw-r--r-- | src/node_options.cc | 5 | ||||
-rw-r--r-- | src/node_options.h | 1 | ||||
-rw-r--r-- | src/node_worker.cc | 2 | ||||
-rw-r--r-- | test/addons/no-addons/binding.gyp | 9 | ||||
-rw-r--r-- | test/addons/no-addons/test-worker.js | 59 | ||||
-rw-r--r-- | test/addons/no-addons/test.js | 43 | ||||
-rw-r--r-- | test/es-module/test-esm-no-addons.mjs | 27 | ||||
-rw-r--r-- | test/fixtures/es-module-loaders/loader-with-custom-condition.mjs | 10 | ||||
-rw-r--r-- | test/fixtures/node_modules/pkgexports/addons-entry.js | 3 | ||||
-rw-r--r-- | test/fixtures/node_modules/pkgexports/no-addons-entry.js | 3 | ||||
-rw-r--r-- | test/fixtures/node_modules/pkgexports/package.json | 4 | ||||
-rw-r--r-- | test/parallel/test-no-addons-resolution-condition.js | 29 |
23 files changed, 272 insertions, 9 deletions
diff --git a/doc/api/cli.md b/doc/api/cli.md index f87b2476e0f..3ab4750d8d6 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -598,6 +598,15 @@ added: v7.10.0 This option is a no-op. It is kept for compatibility. +### `--no-addons` +<!-- YAML +added: REPLACEME +--> + +Disable the `node-addons` exports condition as well as disable loading +native addons. When `--no-addons` is specified, calling `process.dlopen` or +requiring a native C++ addon will fail and throw an exception. + ### `--no-deprecation` <!-- YAML added: v0.8.0 @@ -1428,6 +1437,7 @@ Node.js options that are allowed are: * `--inspect` * `--max-http-header-size` * `--napi-modules` +* `--no-addons` * `--no-deprecation` * `--no-experimental-repl-await` * `--no-extra-info-on-fatal-exception` diff --git a/doc/api/errors.md b/doc/api/errors.md index adef81caa6d..a03b1f6fc25 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1027,6 +1027,14 @@ added: The [debugger][] timed out waiting for the required host/port to be free. +<a id="ERR_DLOPEN_DISABLED"></a> +### `ERR_DLOPEN_DISABLED` +<!-- YAML +added: REPLACEME +--> + +Loading native addons has been disabled using [`--no-addons`][]. + <a id="ERR_DLOPEN_FAILED"></a> ### `ERR_DLOPEN_FAILED` <!-- YAML @@ -2879,6 +2887,7 @@ The native call from `process.cpuUsage` could not be processed. [`'uncaughtException'`]: process.md#event-uncaughtexception [`--disable-proto=throw`]: cli.md#--disable-protomode [`--force-fips`]: cli.md#--force-fips +[`--no-addons`]: cli.md#--no-addons [`Class: assert.AssertionError`]: assert.md#class-assertassertionerror [`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type [`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list diff --git a/doc/api/packages.md b/doc/api/packages.md index 5268772de15..4aee6b77ecb 100644 --- a/doc/api/packages.md +++ b/doc/api/packages.md @@ -537,6 +537,11 @@ Node.js implements the following conditions: * `"node"` - matches for any Node.js environment. Can be a CommonJS or ES module file. _This condition should always come after `"import"` or `"require"`._ +* `"node-addons"` - similar to `"node"` and matches for any Node.js environment. + This condition can be used to provide an entry point which uses native C++ + addons as opposed to an entry point which is more universal and doesn't rely + on native addons. This condition can be disabled via the + [`--no-addons` flag][]. * `"default"` - the generic fallback that always matches. Can be a CommonJS or ES module file. _This condition should always come last._ @@ -615,17 +620,23 @@ node --conditions=development main.js ``` which would then resolve the `"development"` condition in package imports and -exports, while resolving the existing `"node"`, `"default"`, `"import"`, and -`"require"` conditions as appropriate. +exports, while resolving the existing `"node"`, `"node-addons"`, `"default"`, +`"import"`, and `"require"` conditions as appropriate. Any number of custom conditions can be set with repeat flags. ### Conditions Definitions -The `"import"`, `"require"`, `"node"` and `"default"` conditions are defined -and implemented in Node.js core, +The `"import"`, `"require"`, `"node"`, `"node-addons"` and `"default"` +conditions are defined and implemented in Node.js core, [as specified above](#conditional-exports). +The `"node-addons"` condition can be used to provide an entry point which +uses native C++ addons. However, this condition can be disabled via the +[`--no-addons` flag][]. When using `"node-addons"`, it's recommended to treat +`"default"` as an enhancement that provides a more universal entry point, e.g. +using WebAssembly instead of a native addon. + Other condition strings are unknown to Node.js and thus ignored by default. Runtimes or tools other than Node.js can use them at their discretion. @@ -1249,6 +1260,7 @@ This field defines [subpath imports][] for the current package. [`"packageManager"`]: #packagemanager [`"type"`]: #type [`--conditions` flag]: #resolving-user-conditions +[`--no-addons` flag]: cli.md#--no-addons [`ERR_PACKAGE_PATH_NOT_EXPORTED`]: errors.md#err_package_path_not_exported [`esm`]: https://github.com/standard-things/esm#readme [`package.json`]: #nodejs-packagejson-field-definitions diff --git a/doc/node.1 b/doc/node.1 index 310de73b89d..a03d322f98e 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -280,6 +280,11 @@ Hide extra information on fatal exception that causes exit. Disable runtime checks for `async_hooks`. These will still be enabled dynamically when `async_hooks` is enabled. . +.It Fl -no-addons +Disable the `node-addons` exports condition as well as disable loading native +addons. When `--no-addons` is specified, calling `process.dlopen` or requiring +a native C++ addon will fail and throw an exception. +. .It Fl -no-warnings Silence all process warnings (including deprecations). . diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index bba572b32cb..3d27a19a250 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -30,8 +30,16 @@ let debug = require('internal/util/debuglog').debuglog('module', (fn) => { debug = fn; }); +const noAddons = getOptionValue('--no-addons'); +const addonConditions = noAddons ? [] : ['node-addons']; + // TODO: Use this set when resolving pkg#exports conditions in loader.js. -const cjsConditions = new SafeSet(['require', 'node', ...userConditions]); +const cjsConditions = new SafeSet([ + 'require', + 'node', + ...addonConditions, + ...userConditions, +]); function loadNativeModule(filename, request) { const mod = NativeModule.map.get(filename); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index ca0e1b316fc..ae4ecc90e4d 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -58,7 +58,16 @@ const { Module: CJSModule } = require('internal/modules/cjs/loader'); const packageJsonReader = require('internal/modules/package_json_reader'); const userConditions = getOptionValue('--conditions'); -const DEFAULT_CONDITIONS = ObjectFreeze(['node', 'import', ...userConditions]); +const noAddons = getOptionValue('--no-addons'); +const addonConditions = noAddons ? [] : ['node-addons']; + +const DEFAULT_CONDITIONS = ObjectFreeze([ + 'node', + 'import', + ...addonConditions, + ...userConditions, +]); + const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS); /** diff --git a/src/env-inl.h b/src/env-inl.h index 061897d95d8..156534bdeee 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -861,6 +861,11 @@ inline bool Environment::is_main_thread() const { return worker_context() == nullptr; } +inline bool Environment::no_native_addons() const { + return (flags_ & EnvironmentFlags::kNoNativeAddons) || + !options_->allow_native_addons; +} + inline bool Environment::should_not_register_esm_loader() const { return flags_ & EnvironmentFlags::kNoRegisterESMLoader; } diff --git a/src/env.h b/src/env.h index 4fd5be8e150..f7d1dace414 100644 --- a/src/env.h +++ b/src/env.h @@ -1197,6 +1197,7 @@ class Environment : public MemoryRetainer { inline void set_has_serialized_options(bool has_serialized_options); inline bool is_main_thread() const; + inline bool no_native_addons() const; inline bool should_not_register_esm_loader() const; inline bool owns_process_state() const; inline bool owns_inspector() const; diff --git a/src/node.h b/src/node.h index 8622dff8425..e4a7de217e9 100644 --- a/src/node.h +++ b/src/node.h @@ -407,7 +407,13 @@ enum Flags : uint64_t { // Set this flag to force hiding console windows when spawning child // processes. This is usually used when embedding Node.js in GUI programs on // Windows. - kHideConsoleWindows = 1 << 5 + kHideConsoleWindows = 1 << 5, + // Set this flag to disable loading native addons via `process.dlopen`. + // This environment flag is especially important for worker threads + // so that a worker thread can't load a native addon even if `execArgv` + // is overwritten and `--no-addons` is not specified but was specified + // for this Environment instance. + kNoNativeAddons = 1 << 6 }; } // namespace EnvironmentFlags diff --git a/src/node_binding.cc b/src/node_binding.cc index 3c9776e655d..050c5dff0ad 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -415,6 +415,12 @@ inline napi_addon_register_func GetNapiInitializerCallback(DLib* dlib) { // cache that's a plain C list or hash table that's shared across contexts? void DLOpen(const FunctionCallbackInfo<Value>& args) { Environment* env = Environment::GetCurrent(args); + + if (env->no_native_addons()) { + return THROW_ERR_DLOPEN_DISABLED( + env, "Cannot load native addon because loading addons is disabled."); + } + auto context = env->context(); CHECK_NULL(thread_local_modpending); diff --git a/src/node_errors.h b/src/node_errors.h index 0f70fe81b9a..f540b3e2a37 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -57,6 +57,7 @@ void OnFatalError(const char* location, const char* message); V(ERR_CRYPTO_UNKNOWN_DH_GROUP, Error) \ V(ERR_CRYPTO_UNSUPPORTED_OPERATION, Error) \ V(ERR_CRYPTO_JOB_INIT_FAILED, Error) \ + V(ERR_DLOPEN_DISABLED, Error) \ V(ERR_DLOPEN_FAILED, Error) \ V(ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE, Error) \ V(ERR_INVALID_ADDRESS, Error) \ diff --git a/src/node_options.cc b/src/node_options.cc index f66fbc8ed45..a85cb92f36d 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -402,6 +402,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::force_async_hooks_checks, kAllowedInEnvironment, true); + AddOption("--addons", + "disable loading native addons", + &EnvironmentOptions::allow_native_addons, + kAllowedInEnvironment, + true); AddOption("--warnings", "silence all process warnings", &EnvironmentOptions::warnings, diff --git a/src/node_options.h b/src/node_options.h index 36b5ed40197..1ef3c5216e0 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -121,6 +121,7 @@ class EnvironmentOptions : public Options { uint64_t max_http_header_size = 16 * 1024; bool deprecation = true; bool force_async_hooks_checks = true; + bool allow_native_addons = true; bool warnings = true; bool force_context_aware = false; bool pending_deprecation = false; diff --git a/src/node_worker.cc b/src/node_worker.cc index 3e3cb67d9e8..3195ddbaf01 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -560,6 +560,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) { worker->environment_flags_ |= EnvironmentFlags::kTrackUnmanagedFds; if (env->hide_console_windows()) worker->environment_flags_ |= EnvironmentFlags::kHideConsoleWindows; + if (env->no_native_addons()) + worker->environment_flags_ |= EnvironmentFlags::kNoNativeAddons; } void Worker::StartThread(const FunctionCallbackInfo<Value>& args) { diff --git a/test/addons/no-addons/binding.gyp b/test/addons/no-addons/binding.gyp new file mode 100644 index 00000000000..77f2fced5a3 --- /dev/null +++ b/test/addons/no-addons/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ '../hello-world/binding.cc' ], + 'includes': ['../common.gypi'], + } + ] +} diff --git a/test/addons/no-addons/test-worker.js b/test/addons/no-addons/test-worker.js new file mode 100644 index 00000000000..2a89cef3e27 --- /dev/null +++ b/test/addons/no-addons/test-worker.js @@ -0,0 +1,59 @@ +// Flags: --no-addons + +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const path = require('path'); +const { Worker } = require('worker_threads'); + +const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`); + +const assertError = (error) => { + assert.strictEqual(error.code, 'ERR_DLOPEN_DISABLED'); + assert.strictEqual( + error.message, + 'Cannot load native addon because loading addons is disabled.' + ); +}; + +{ + // Flags should be inherited + const worker = new Worker(`require(${JSON.stringify(binding)})`, { + eval: true, + }); + + worker.on('error', common.mustCall(assertError)); +} + +{ + // Should throw when using `process.dlopen` directly + const worker = new Worker( + `process.dlopen({ exports: {} }, ${JSON.stringify(binding)});`, + { + eval: true, + } + ); + + worker.on('error', common.mustCall(assertError)); +} + +{ + // Explicitly pass `--no-addons` + const worker = new Worker(`require(${JSON.stringify(binding)})`, { + eval: true, + execArgv: ['--no-addons'], + }); + + worker.on('error', common.mustCall(assertError)); +} + +{ + // If `execArgv` is overwritten it should still fail to load addons + const worker = new Worker(`require(${JSON.stringify(binding)})`, { + eval: true, + execArgv: [], + }); + + worker.on('error', common.mustCall(assertError)); +} diff --git a/test/addons/no-addons/test.js b/test/addons/no-addons/test.js new file mode 100644 index 00000000000..063dd3491e3 --- /dev/null +++ b/test/addons/no-addons/test.js @@ -0,0 +1,43 @@ +// Flags: --no-addons + +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); + +const bindingPath = require.resolve(`./build/${common.buildType}/binding`); + +const assertError = (error) => { + assert(error instanceof Error); + assert.strictEqual(error.code, 'ERR_DLOPEN_DISABLED'); + assert.strictEqual( + error.message, + 'Cannot load native addon because loading addons is disabled.' + ); +}; + +{ + let threw = false; + + try { + require(bindingPath); + } catch (error) { + assertError(error); + threw = true; + } + + assert(threw); +} + +{ + let threw = false; + + try { + process.dlopen({ exports: {} }, bindingPath); + } catch (error) { + assertError(error); + threw = true; + } + + assert(threw); +} diff --git a/test/es-module/test-esm-no-addons.mjs b/test/es-module/test-esm-no-addons.mjs new file mode 100644 index 00000000000..a61c1f3b7cb --- /dev/null +++ b/test/es-module/test-esm-no-addons.mjs @@ -0,0 +1,27 @@ +import { mustCall } from '../common/index.mjs'; +import { Worker, isMainThread } from 'worker_threads'; +import assert from 'assert'; +import { fileURLToPath } from 'url'; +import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs'; + +if (isMainThread) { + const tests = [[], ['--no-addons']]; + + for (const execArgv of tests) { + new Worker(fileURLToPath(import.meta.url), { execArgv }); + } +} else { + [requireFixture, importFixture].forEach((loadFixture) => { + loadFixture('pkgexports/no-addons').then( + mustCall((module) => { + const message = module.default; + + if (process.execArgv.length === 0) { + assert.strictEqual(message, 'using native addons'); + } else { + assert.strictEqual(message, 'not using native addons'); + } + }) + ); + }); +} diff --git a/test/fixtures/es-module-loaders/loader-with-custom-condition.mjs b/test/fixtures/es-module-loaders/loader-with-custom-condition.mjs index 78ffd75e6be..3aefed51d57 100644 --- a/test/fixtures/es-module-loaders/loader-with-custom-condition.mjs +++ b/test/fixtures/es-module-loaders/loader-with-custom-condition.mjs @@ -1,8 +1,14 @@ -import {ok, deepStrictEqual} from 'assert'; +import { ok, deepStrictEqual } from 'assert'; export async function resolve(specifier, context, defaultResolve) { ok(Array.isArray(context.conditions), 'loader receives conditions array'); - deepStrictEqual([...context.conditions].sort(), ['import', 'node']); + + deepStrictEqual([...context.conditions].sort(), [ + 'import', + 'node', + 'node-addons', + ]); + return defaultResolve(specifier, { ...context, conditions: ['custom-condition', ...context.conditions], diff --git a/test/fixtures/node_modules/pkgexports/addons-entry.js b/test/fixtures/node_modules/pkgexports/addons-entry.js new file mode 100644 index 00000000000..6487c639f43 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/addons-entry.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'using native addons'; diff --git a/test/fixtures/node_modules/pkgexports/no-addons-entry.js b/test/fixtures/node_modules/pkgexports/no-addons-entry.js new file mode 100644 index 00000000000..85096c5aab8 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/no-addons-entry.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'not using native addons'; diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index 6c80d5cb15a..7f8f994ac39 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -20,6 +20,10 @@ "./nofallback1": [], "./nofallback2": [null, {}, "builtin:x"], "./nodemodules": "./node_modules/internalpkg/x.js", + "./no-addons": { + "node-addons": "./addons-entry.js", + "default": "./no-addons-entry.js" + }, "./condition": [{ "custom-condition": { "import": "./custom-condition.mjs", diff --git a/test/parallel/test-no-addons-resolution-condition.js b/test/parallel/test-no-addons-resolution-condition.js new file mode 100644 index 00000000000..9707f08d999 --- /dev/null +++ b/test/parallel/test-no-addons-resolution-condition.js @@ -0,0 +1,29 @@ +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const { Worker, isMainThread, parentPort } = require('worker_threads'); +const assert = require('assert'); +const { createRequire } = require('module'); + +const loadFixture = createRequire(fixtures.path('node_modules')); + +if (isMainThread) { + const tests = [[], ['--no-addons']]; + + for (const execArgv of tests) { + const worker = new Worker(__filename, { execArgv }); + + worker.on('message', common.mustCall((message) => { + if (execArgv.length === 0) { + assert.strictEqual(message, 'using native addons'); + } else { + assert.strictEqual(message, 'not using native addons'); + } + })); + } + +} else { + const message = loadFixture('pkgexports/no-addons'); + parentPort.postMessage(message); +} |