From 1aa8a785be7aff22fabbdc92f9e3e16e17069efb Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sun, 16 Jun 2024 12:08:09 -0400 Subject: [PATCH] fs: add fast api for `InternalModuleStat` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/51344 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Geoffrey Booth Reviewed-By: Franziska Hinkelmann Reviewed-By: Stephen Belanger Reviewed-By: Matteo Collina --- lib/internal/modules/cjs/loader.js | 4 +- lib/internal/modules/esm/resolve.js | 9 +++-- src/node_external_reference.h | 6 +++ src/node_file.cc | 40 ++++++++++++++++++- ...test-permission-fs-internal-module-stat.js | 26 ++++++++++++ 5 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-permission-fs-internal-module-stat.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index dbe40cb824cd29..ffc9d94834a17d 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -139,7 +139,7 @@ const { const assert = require('internal/assert'); const fs = require('fs'); const path = require('path'); -const { internalModuleStat } = internalBinding('fs'); +const internalFsBinding = internalBinding('fs'); const { safeGetenv } = internalBinding('credentials'); const { getCjsConditions, @@ -225,7 +225,7 @@ function stat(filename) { const result = statCache.get(filename); if (result !== undefined) { return result; } } - const result = internalModuleStat(filename); + const result = internalFsBinding.internalModuleStat(filename); if (statCache !== null && result >= 0) { // Only set cache when `internalModuleStat(filename)` succeeds. statCache.set(filename, result); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 99d91ed794ea4c..f9673a3ed54571 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -54,7 +54,7 @@ const { const { Module: CJSModule } = require('internal/modules/cjs/loader'); const { getConditionsSet } = require('internal/modules/esm/utils'); const packageJsonReader = require('internal/modules/package_json_reader'); -const { internalModuleStat } = internalBinding('fs'); +const internalFsBinding = internalBinding('fs'); /** * @typedef {import('internal/modules/esm/package_config.js').PackageConfig} PackageConfig @@ -246,7 +246,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) { throw err; } - const stats = internalModuleStat(toNamespacedPath(StringPrototypeEndsWith(path, '/') ? + const stats = internalFsBinding.internalModuleStat(toNamespacedPath(StringPrototypeEndsWith(path, '/') ? StringPrototypeSlice(path, -1) : path)); // Check for stats.isDirectory() @@ -806,8 +806,9 @@ function packageResolve(specifier, base, conditions) { let packageJSONPath = fileURLToPath(packageJSONUrl); let lastPath; do { - const stat = internalModuleStat(toNamespacedPath(StringPrototypeSlice(packageJSONPath, 0, - packageJSONPath.length - 13))); + const stat = internalFsBinding.internalModuleStat( + toNamespacedPath(StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13)), + ); // Check for !stat.isDirectory() if (stat !== 1) { lastPath = packageJSONPath; diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 6bbeffb8941ea3..0b87ee0ceb16d7 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -15,6 +15,11 @@ using CFunctionCallbackWithOneByteString = using CFunctionCallback = void (*)(v8::Local receiver); using CFunctionCallbackReturnDouble = double (*)(v8::Local receiver); +using CFunctionCallbackReturnInt32 = + int32_t (*)(v8::Local receiver, + const v8::FastOneByteString& input, + // NOLINTNEXTLINE(runtime/references) This is V8 api. + v8::FastApiCallbackOptions& options); using CFunctionCallbackValueReturnDouble = double (*)(v8::Local receiver); using CFunctionCallbackWithInt64 = void (*)(v8::Local receiver, @@ -55,6 +60,7 @@ class ExternalReferenceRegistry { V(CFunctionCallback) \ V(CFunctionCallbackWithOneByteString) \ V(CFunctionCallbackReturnDouble) \ + V(CFunctionCallbackReturnInt32) \ V(CFunctionCallbackValueReturnDouble) \ V(CFunctionCallbackWithInt64) \ V(CFunctionCallbackWithBool) \ diff --git a/src/node_file.cc b/src/node_file.cc index ccfd6a740dd9cf..230396b95c34a8 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -39,6 +39,9 @@ #include "stream_base-inl.h" #include "string_bytes.h" #include "uv.h" +#include "v8-fast-api-calls.h" + +#include #if defined(__MINGW32__) || defined(_MSC_VER) # include @@ -52,6 +55,8 @@ using v8::Array; using v8::BigInt; using v8::Context; using v8::EscapableHandleScope; +using v8::FastApiCallbackOptions; +using v8::FastOneByteString; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -1038,6 +1043,33 @@ static void InternalModuleStat(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(rc); } +static int32_t FastInternalModuleStat( + Local recv, + const FastOneByteString& input, + // NOLINTNEXTLINE(runtime/references) This is V8 api. + FastApiCallbackOptions& options) { + Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked()); + + auto path = std::filesystem::path(input.data, input.data + input.length); + if (UNLIKELY(!env->permission()->is_granted( + env, permission::PermissionScope::kFileSystemRead, path.string()))) { + options.fallback = true; + return -1; + } + + switch (std::filesystem::status(path).type()) { + case std::filesystem::file_type::directory: + return 1; + case std::filesystem::file_type::regular: + return 0; + default: + return -1; + } +} + +v8::CFunction fast_internal_module_stat_( + v8::CFunction::Make(FastInternalModuleStat)); + constexpr bool is_uv_error_except_no_entry(int result) { return result < 0 && result != UV_ENOENT; } @@ -3261,7 +3293,11 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, SetMethod(isolate, target, "rmdir", RMDir); SetMethod(isolate, target, "mkdir", MKDir); SetMethod(isolate, target, "readdir", ReadDir); - SetMethod(isolate, target, "internalModuleStat", InternalModuleStat); + SetFastMethod(isolate, + target, + "internalModuleStat", + InternalModuleStat, + &fast_internal_module_stat_); SetMethod(isolate, target, "stat", Stat); SetMethod(isolate, target, "lstat", LStat); SetMethod(isolate, target, "fstat", FStat); @@ -3382,6 +3418,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(MKDir); registry->Register(ReadDir); registry->Register(InternalModuleStat); + registry->Register(FastInternalModuleStat); + registry->Register(fast_internal_module_stat_.GetTypeInfo()); registry->Register(Stat); registry->Register(LStat); registry->Register(FStat); diff --git a/test/parallel/test-permission-fs-internal-module-stat.js b/test/parallel/test-permission-fs-internal-module-stat.js new file mode 100644 index 00000000000000..b39c276f3c1d99 --- /dev/null +++ b/test/parallel/test-permission-fs-internal-module-stat.js @@ -0,0 +1,26 @@ +// Flags: --expose-internals --experimental-permission --allow-fs-read=test/common* --allow-fs-read=tools* --allow-fs-read=test/parallel* --allow-child-process +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); + +if (!common.hasCrypto) { + common.skip('no crypto'); +} + +const { internalBinding } = require('internal/test/binding'); +const assert = require('node:assert'); +const fixtures = require('../common/fixtures'); + +const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); +const internalFsBinding = internalBinding('fs'); + +// Run this inside a for loop to trigger the fast API +for (let i = 0; i < 10_000; i++) { + assert.throws(() => { + internalFsBinding.internalModuleStat(blockedFile); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + }); +}