Skip to content

Commit 39893c2

Browse files
divybotlittledivy
andauthored
perf(node): skip require permission checks when read is fully granted (#34722)
## Summary Every `require()` (and `node:worker_threads` filename resolution) runs through the `ensure_read_permission` helpers in `ext/node/ops/require.rs` and `ext/node/ops/worker_threads.rs`. Those helpers fetch the `NodeRequireLoaderRc` and dispatch into the loader, whose CLI implementation (`CliNodeRequireLoader::ensure_read_permission`) does a `url_from_file_path` + module-graph lookup — an `RwLock` read, an `Arc` clone, a `Url` allocation and a hashmap lookup — on **every** call, before the registry checker's existing `query_read_all()` fast path is ever reached. As noted in the issue, this means we effectively always do node permission work even when permissions are fully allowed. This hoists the read-all check to the top of the two ext/node op helpers, so the entire loader dispatch and its per-call work are skipped when read is fully granted. ## Correctness The loader's graph / `node_modules` allowances only exist to permit reads *without* a full read grant; when read is fully granted everything is allowed anyway, so returning the path unchanged is equivalent. `--deny-read` keeps `query_read_all()` returning `false`, so restricted reads still fall through to the full per-path check. Smoke-tested locally: - `deno run -A` / `deno run --allow-read` resolve `require()` as before. - `deno run --allow-read=main.cjs` still denies reading a sibling dependency with `NotCapable: Requires read access ...`. Closes #23919 Closes denoland/divybot#424 Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent adba38f commit 39893c2

2 files changed

Lines changed: 12 additions & 0 deletions

File tree

ext/node/ops/require.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ fn ensure_read_permission<'a>(
3939
state: &mut OpState,
4040
file_path: Cow<'a, Path>,
4141
) -> Result<Cow<'a, Path>, JsErrorBox> {
42+
// Fast path: when read is fully granted there's nothing to check, so skip
43+
// fetching the loader and the per-call work it does (e.g. module graph
44+
// lookups) entirely.
45+
if state.borrow::<PermissionsContainer>().query_read_all() {
46+
return Ok(file_path);
47+
}
4248
let loader = state.borrow::<NodeRequireLoaderRc>().clone();
4349
let permissions = state.borrow_mut::<PermissionsContainer>();
4450
loader.ensure_read_permission(permissions, file_path)

ext/node/ops/worker_threads.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ fn ensure_read_permission<'a>(
3232
state: &mut OpState,
3333
file_path: Cow<'a, Path>,
3434
) -> Result<Cow<'a, Path>, JsErrorBox> {
35+
// Fast path: when read is fully granted there's nothing to check, so skip
36+
// fetching the loader and the per-call work it does (e.g. module graph
37+
// lookups) entirely.
38+
if state.borrow::<PermissionsContainer>().query_read_all() {
39+
return Ok(file_path);
40+
}
3541
let loader = state.borrow::<NodeRequireLoaderRc>().clone();
3642
let permissions = state.borrow_mut::<PermissionsContainer>();
3743
loader.ensure_read_permission(permissions, file_path)

0 commit comments

Comments
 (0)