src: add receiver to fast api callback methods

When creating an fast api the callback might use the receiver. In that case if the internal binding is destructured the method won't have access to the reciver and it will throw. Passing the receiver as second argument ensures the receiver is available.
nodejs-github-bot · Sep 28, 2024 · Aug 16, 2024 · Sep 24, 2024 · Sep 24, 2024 · Sep 24, 2024
commit d57f479840757ecbd4ac32a7fe3dafe1543e0aae
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -220,7 +220,7 @@ The initializer module also needs to be allowed. Consider the following example:
 ```console
 $ node --experimental-permission t.js
 node:internal/modules/cjs/loader:162
-  const result = internalModuleStat(filename);
+  const result = internalModuleStat(receiver, filename);
                  ^
 
 Error: Access to this API has been restricted

diff --git a/doc/api/permissions.md b/doc/api/permissions.md
@@ -39,7 +39,7 @@ will be restricted.
 ```console
 $ node --experimental-permission index.js
 node:internal/modules/cjs/loader:171
-  const result = internalModuleStat(filename);
+  const result = internalModuleStat(receiver, filename);
                  ^
 
 Error: Access to this API has been restricted

diff --git a/doc/contributing/adding-v8-fast-api.md b/doc/contributing/adding-v8-fast-api.md
@@ -24,7 +24,7 @@ for example, they may not trigger garbage collection.
   [`node_external_reference.h`](../../src/node_external_reference.h) file.
   Although, it would not start failing or crashing until the function ends up
   in a snapshot (either the built-in or a user-land one). Please refer to the
-  [binding functions documentation](../../src#binding-functions) for more
+  [binding functions documentation](../../src/README.md#binding-functions) for more
   information.
 * To test fast APIs, make sure to run the tests in a loop with a decent
   iterations count to trigger relevant optimizations that prefer the fast API
@@ -38,6 +38,19 @@ for example, they may not trigger garbage collection.
 * The fast callback must be idempotent up to the point where error and fallback
   conditions are checked, because otherwise executing the slow callback might
   produce visible side effects twice.
+* If the receiver is used in the callback it must be passed as a second argument
+  leaving the first one unused.
+
+  ```cpp
+  static int32_t FastInternalModuleStat(
+      Local<Object> unused,
+      Local<Object> recv,
+      const FastOneByteString& input,
+      FastApiCallbackOptions& options) {
+    Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
+    // More code
+  }
+  ```
 
 ## Fallback to slow path
 

diff --git a/lib/fs.js b/lib/fs.js
@@ -1410,7 +1410,7 @@ function readdirSyncRecursive(basePath, options) {
       for (let i = 0; i < readdirResult.length; i++) {
         const resultPath = pathModule.join(path, readdirResult[i]);
         const relativeResultPath = pathModule.relative(basePath, resultPath);
-        const stat = binding.internalModuleStat(resultPath);
+        const stat = binding.internalModuleStat(binding, resultPath);
         ArrayPrototypePush(readdirResults, relativeResultPath);
         // 1 indicates directory
         if (stat === 1) {

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -915,7 +915,7 @@ async function readdirRecursive(originalPath, options) {
       const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
       for (const ent of readdir) {
         const direntPath = pathModule.join(path, ent);
-        const stat = binding.internalModuleStat(direntPath);
+        const stat = binding.internalModuleStat(binding, direntPath);
         ArrayPrototypePush(
           result,
           pathModule.relative(originalPath, direntPath),

diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -234,9 +234,9 @@ function stat(filename) {
     const result = statCache.get(filename);
     if (result !== undefined) { return result; }
   }
-  const result = internalFsBinding.internalModuleStat(filename);
+  const result = internalFsBinding.internalModuleStat(internalFsBinding, filename);
   if (statCache !== null && result >= 0) {
-    // Only set cache when `internalModuleStat(filename)` succeeds.
+    // Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds.
     statCache.set(filename, result);
   }
   return result;

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -243,8 +243,10 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
     throw err;
   }
 
-  const stats = internalFsBinding.internalModuleStat(StringPrototypeEndsWith(path, '/') ?
-    StringPrototypeSlice(path, -1) : path);
+  const stats = internalFsBinding.internalModuleStat(
+    internalFsBinding,
+    StringPrototypeEndsWith(internalFsBinding, path, '/') ? StringPrototypeSlice(path, -1) : path,
+  );
 
   // Check for stats.isDirectory()
   if (stats === 1) {
@@ -804,6 +806,7 @@ function packageResolve(specifier, base, conditions) {
   let lastPath;
   do {
     const stat = internalFsBinding.internalModuleStat(
+      internalFsBinding,
       StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13),
     );
     // Check for !stat.isDirectory()

diff --git a/src/node_external_reference.h b/src/node_external_reference.h
@@ -16,7 +16,8 @@ using CFunctionCallback = void (*)(v8::Local<v8::Value> receiver);
 using CFunctionCallbackReturnDouble =
     double (*)(v8::Local<v8::Object> receiver);
 using CFunctionCallbackReturnInt32 =
-    int32_t (*)(v8::Local<v8::Object> receiver,
+    int32_t (*)(v8::Local<v8::Object> unused,
+                v8::Local<v8::Object> receiver,
                 const v8::FastOneByteString& input,
                 // NOLINTNEXTLINE(runtime/references) This is V8 api.
                 v8::FastApiCallbackOptions& options);

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1034,8 +1034,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
 static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  CHECK(args[0]->IsString());
-  BufferValue path(env->isolate(), args[0]);
+  CHECK_GE(args.Length(), 2);
+  CHECK(args[1]->IsString());
+  BufferValue path(env->isolate(), args[1]);
   CHECK_NOT_NULL(*path);
   ToNamespacedPath(env, &path);
   THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -1053,6 +1054,7 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
 }
 
 static int32_t FastInternalModuleStat(
+    Local<Object> unused,
     Local<Object> recv,
     const FastOneByteString& input,
     // NOLINTNEXTLINE(runtime/references) This is V8 api.

diff --git a/test/parallel/test-permission-fs-internal-module-stat.js b/test/parallel/test-permission-fs-internal-module-stat.js
@@ -18,7 +18,7 @@ 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);
+    internalFsBinding.internalModuleStat(internalFsBinding, blockedFile);
   }, {
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',

diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts
@@ -113,7 +113,7 @@ declare namespace InternalFSBinding {
   function futimes(fd: number, atime: number, mtime: number): void;
   function futimes(fd: number, atime: number, mtime: number, usePromises: typeof kUsePromises): Promise<void>;
 
-  function internalModuleStat(path: string): number;
+  function internalModuleStat(receiver: unknown, path: string): number;
 
   function lchown(path: string, uid: number, gid: number, req: FSReqCallback): void;
   function lchown(path: string, uid: number, gid: number, req: undefined, ctx: FSSyncContext): void;