Skip to content

Commit

Permalink
fs: add fast api for InternalModuleStat
Browse files Browse the repository at this point in the history
PR-URL: nodejs#51344
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
anonrig authored and sophoniie committed Jun 20, 2024
1 parent 0928eb2 commit ce8388d
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 7 deletions.
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/node_external_reference.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ using CFunctionCallbackWithOneByteString =
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,
const v8::FastOneByteString& input,
// NOLINTNEXTLINE(runtime/references) This is V8 api.
v8::FastApiCallbackOptions& options);
using CFunctionCallbackValueReturnDouble =
double (*)(v8::Local<v8::Value> receiver);
using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> receiver,
Expand Down Expand Up @@ -55,6 +60,7 @@ class ExternalReferenceRegistry {
V(CFunctionCallback) \
V(CFunctionCallbackWithOneByteString) \
V(CFunctionCallbackReturnDouble) \
V(CFunctionCallbackReturnInt32) \
V(CFunctionCallbackValueReturnDouble) \
V(CFunctionCallbackWithInt64) \
V(CFunctionCallbackWithBool) \
Expand Down
40 changes: 39 additions & 1 deletion src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
#include "stream_base-inl.h"
#include "string_bytes.h"
#include "uv.h"
#include "v8-fast-api-calls.h"

#include <filesystem>

#if defined(__MINGW32__) || defined(_MSC_VER)
# include <io.h>
Expand All @@ -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;
Expand Down Expand Up @@ -1038,6 +1043,33 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(rc);
}

static int32_t FastInternalModuleStat(
Local<Object> 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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-permission-fs-internal-module-stat.js
Original file line number Diff line number Diff line change
@@ -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',
});
}

0 comments on commit ce8388d

Please sign in to comment.