From 0f7da2c03cd40e69725d0d44d3fce1ee11c6106c Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 2 Feb 2021 23:07:34 -0800 Subject: [PATCH 1/5] node-api: allow retrieval of add-on file name Unlike JS-only modules, native add-ons are always associated with a dynamic shared object from which they are loaded. Being able to retrieve its absolute path is important to native-only add-ons, i.e. add-ons that are not themselves being loaded from a JS-only module located in the same package as the native add-on itself. Currently, the file name is obtained at environment construction time from the JS `module.filename`. Nevertheless, the presence of `module` is not required, because the file name could also be passed in via a private property added onto `exports` from the `process.dlopen` binding. Fixes: https://github.com/nodejs/node-addon-api/issues/449 PR-URL: https://github.com/nodejs/node/pull/37195 --- doc/api/n-api.md | 24 ++++++++++++++ src/env.h | 1 + src/node_api.cc | 40 +++++++++++++++++++---- src/node_api.h | 3 ++ test/node-api/test_general/test.js | 5 ++- test/node-api/test_general/test_general.c | 13 ++++++++ 6 files changed, 79 insertions(+), 7 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 1e2240d26ca409..e73c96a58d7c9a 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -5958,6 +5958,30 @@ idempotent. This API may only be called from the main thread. +## Miscellaneous utilities + +## node_api_get_module_file_name + + + +> Stability: 1 - Experimental + +```c +NAPI_EXTERN napi_status +node_api_get_module_file_name(napi_env env, const char** result); + +``` + +* `[in] env`: The environment that the API is invoked under. +* `[out] result`: A null-terminated string containing the absolute path of the + file from which the add-on was loaded. The string is owned by `env` and must + not be modified or freed. + +`result` may be an empty string if the add-on loading process fails to establish +the add-on's file name during loading. + [ABI Stability]: https://nodejs.org/en/docs/guides/abi-stability/ [AppVeyor]: https://www.appveyor.com [C++ Addons]: addons.md diff --git a/src/env.h b/src/env.h index ce0bcbfd25fc5a..1ec3351b8cffe9 100644 --- a/src/env.h +++ b/src/env.h @@ -253,6 +253,7 @@ constexpr size_t kFsStatsBufferLength = V(fd_string, "fd") \ V(fields_string, "fields") \ V(file_string, "file") \ + V(filename_string, "filename") \ V(fingerprint256_string, "fingerprint256") \ V(fingerprint_string, "fingerprint") \ V(flags_string, "flags") \ diff --git a/src/node_api.cc b/src/node_api.cc index f1a5265b6a7234..1f1a5b4f0831c1 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -15,8 +15,9 @@ #include struct node_napi_env__ : public napi_env__ { - explicit node_napi_env__(v8::Local context): - napi_env__(context) { + explicit node_napi_env__(v8::Local context, + const std::string& module_filename): + napi_env__(context), filename(module_filename) { CHECK_NOT_NULL(node_env()); } @@ -46,6 +47,10 @@ struct node_napi_env__ : public napi_env__ { }); }); } + + const char* GetFilename() const { return filename.c_str(); } + + std::string filename; }; typedef node_napi_env__* node_napi_env; @@ -87,10 +92,11 @@ class BufferFinalizer : private Finalizer { }; }; -static inline napi_env NewEnv(v8::Local context) { +static inline napi_env +NewEnv(v8::Local context, const std::string& module_filename) { node_napi_env result; - result = new node_napi_env__(context); + result = new node_napi_env__(context, module_filename); // TODO(addaleax): There was previously code that tried to delete the // napi_env when its v8::Context was garbage collected; // However, as long as N-API addons using this napi_env are in place, @@ -552,16 +558,30 @@ void napi_module_register_by_symbol(v8::Local exports, v8::Local module, v8::Local context, napi_addon_register_func init) { + node::Environment* node_env = node::Environment::GetCurrent(context); + std::string module_filename = ""; if (init == nullptr) { - node::Environment* node_env = node::Environment::GetCurrent(context); CHECK_NOT_NULL(node_env); node_env->ThrowError( "Module has no declared entry point."); return; } + // We set `env->filename` from `module.filename` here, but we could just as + // easily add a private property to `exports` in `process.dlopen`, which + // receives the file name from JS, and retrieve *that* here. Thus, we are not + // endorsing commonjs here by making use of `module.filename`. + v8::Local filename_js; + v8::Local modobj; + if (module->ToObject(context).ToLocal(&modobj) && + modobj->Get(context, node_env->filename_string()).ToLocal(&filename_js) && + filename_js->IsString()) { + node::Utf8Value filename(node_env->isolate(), filename_js); // Cast + module_filename = *filename; + } + // Create a new napi_env for this specific module. - napi_env env = v8impl::NewEnv(context); + napi_env env = v8impl::NewEnv(context, module_filename); napi_value _exports; env->CallIntoModule([&](napi_env env) { @@ -1257,3 +1277,11 @@ napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func) { CHECK_NOT_NULL(func); return reinterpret_cast(func)->Ref(); } + +napi_status node_api_get_module_file_name(napi_env env, const char** result) { + CHECK_ENV(env); + CHECK_ARG(env, result); + + *result = static_cast(env)->GetFilename(); + return napi_clear_last_error(env); +} diff --git a/src/node_api.h b/src/node_api.h index 786988e296b8b2..e7be240cdccd38 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -261,6 +261,9 @@ NAPI_EXTERN napi_status napi_add_async_cleanup_hook( NAPI_EXTERN napi_status napi_remove_async_cleanup_hook( napi_async_cleanup_hook_handle remove_handle); +NAPI_EXTERN napi_status +node_api_get_module_file_name(napi_env env, const char** result); + #endif // NAPI_EXPERIMENTAL EXTERN_C_END diff --git a/test/node-api/test_general/test.js b/test/node-api/test_general/test.js index 108d51a1e11ac9..90b89e7af6eddf 100644 --- a/test/node-api/test_general/test.js +++ b/test/node-api/test_general/test.js @@ -1,9 +1,12 @@ 'use strict'; const common = require('../../common'); -const test_general = require(`./build/${common.buildType}/test_general`); +const filename = require.resolve(`./build/${common.buildType}/test_general`); +const test_general = require(filename); const assert = require('assert'); +assert.strictEqual(test_general.filename, filename); + const [ major, minor, patch, release ] = test_general.testGetNodeVersion(); assert.strictEqual(process.version.split('-')[0], `v${major}.${minor}.${patch}`); diff --git a/test/node-api/test_general/test_general.c b/test/node-api/test_general/test_general.c index a7b7c34047c717..d430e2df4f3520 100644 --- a/test/node-api/test_general/test_general.c +++ b/test/node-api/test_general/test_general.c @@ -1,3 +1,4 @@ +#define NAPI_EXPERIMENTAL #include #include #include "../../js-native-api/common.h" @@ -20,9 +21,21 @@ static napi_value testGetNodeVersion(napi_env env, napi_callback_info info) { return result; } +static napi_value GetFilename(napi_env env, napi_callback_info info) { + const char* filename; + napi_value result; + + NODE_API_CALL(env, node_api_get_module_file_name(env, &filename)); + NODE_API_CALL(env, + napi_create_string_utf8(env, filename, NAPI_AUTO_LENGTH, &result)); + + return result; +} + static napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor descriptors[] = { DECLARE_NODE_API_PROPERTY("testGetNodeVersion", testGetNodeVersion), + DECLARE_NODE_API_GETTER("filename", GetFilename), }; NODE_API_CALL(env, napi_define_properties( From e2a762656122ee484e3ee6c67fdcf2050d939b90 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 3 Feb 2021 16:19:27 -0800 Subject: [PATCH 2/5] heed review comment --- doc/api/n-api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index e73c96a58d7c9a..7c6097eaa379ce 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -5976,8 +5976,8 @@ node_api_get_module_file_name(napi_env env, const char** result); * `[in] env`: The environment that the API is invoked under. * `[out] result`: A null-terminated string containing the absolute path of the - file from which the add-on was loaded. The string is owned by `env` and must - not be modified or freed. + location from which the add-on was loaded. The string is owned by `env` and + must not be modified or freed. `result` may be an empty string if the add-on loading process fails to establish the add-on's file name during loading. From ecddc20bd642740a569336ccaa7cf58840541b24 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 6 Feb 2021 21:06:36 -0800 Subject: [PATCH 3/5] prefix with file:// --- src/node_api.cc | 7 ++++++- test/node-api/test_general/test.js | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 1f1a5b4f0831c1..8dbf48d466dfe1 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -577,7 +577,12 @@ void napi_module_register_by_symbol(v8::Local exports, modobj->Get(context, node_env->filename_string()).ToLocal(&filename_js) && filename_js->IsString()) { node::Utf8Value filename(node_env->isolate(), filename_js); // Cast - module_filename = *filename; + + // Turn the absolute path into a URL. Currently the absolute path is always + // a file system path. + // TODO(gabrielschulhof): Pass the `filename` through unchanged if/when we + // receive it as a URL already. + module_filename = std::string("file://") + (*filename); } // Create a new napi_env for this specific module. diff --git a/test/node-api/test_general/test.js b/test/node-api/test_general/test.js index 90b89e7af6eddf..dd409f010a3ada 100644 --- a/test/node-api/test_general/test.js +++ b/test/node-api/test_general/test.js @@ -5,7 +5,9 @@ const filename = require.resolve(`./build/${common.buildType}/test_general`); const test_general = require(filename); const assert = require('assert'); -assert.strictEqual(test_general.filename, filename); +// TODO(gabrielschulhof): This test may need updating if/when the filename +// becomes a full-fledged URL. +assert.strictEqual(test_general.filename, `file://${filename}`); const [ major, minor, patch, release ] = test_general.testGetNodeVersion(); assert.strictEqual(process.version.split('-')[0], From 77fff595ee5387accb1ab7b91668bc577459aa08 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 8 Feb 2021 12:45:55 -0800 Subject: [PATCH 4/5] Update doc/api/n-api.md Co-authored-by: Michael Dawson --- doc/api/n-api.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 7c6097eaa379ce..4900a22a4bcc81 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -5975,8 +5975,9 @@ node_api_get_module_file_name(napi_env env, const char** result); ``` * `[in] env`: The environment that the API is invoked under. -* `[out] result`: A null-terminated string containing the absolute path of the - location from which the add-on was loaded. The string is owned by `env` and +* `[out] result`: A URL containing the absolute path of the + location from which the add-on was loaded. For a file on the local + file system it will start with `file://`. The string is owned by `env` and must not be modified or freed. `result` may be an empty string if the add-on loading process fails to establish From 2d017d4d56b0412ba35d307890280bc610cc90f7 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 8 Feb 2021 12:50:30 -0800 Subject: [PATCH 5/5] retain message about null-termination --- doc/api/n-api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 4900a22a4bcc81..53980a45ed3c3e 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -5977,8 +5977,8 @@ node_api_get_module_file_name(napi_env env, const char** result); * `[in] env`: The environment that the API is invoked under. * `[out] result`: A URL containing the absolute path of the location from which the add-on was loaded. For a file on the local - file system it will start with `file://`. The string is owned by `env` and - must not be modified or freed. + file system it will start with `file://`. The string is null-terminated and + owned by `env` and must thus not be modified or freed. `result` may be an empty string if the add-on loading process fails to establish the add-on's file name during loading.