fix(ext/node): don't throw error on invalid path input on exists and existsSync (#29971)

Currently `fs.exists` and `fs.existsSync` throws error when invalid path
is given to the input.

The expected behavior is to:
- Call the callback with false on `fs.exists`.
- Return false on `fs.existsSync`.

Towards #29972, #24236
This commit is contained in:
Daniel Osvaldo R 2025-07-08 17:30:45 +07:00 committed by GitHub
parent c03199ea1e
commit 1c16fde60f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 40 additions and 21 deletions

View file

@ -253,9 +253,9 @@ export function maybeCallback(cb: unknown) {
// Ensure that callbacks run in the global context. Only use this function // Ensure that callbacks run in the global context. Only use this function
// for callbacks that are passed to the binding layer, callbacks that are // for callbacks that are passed to the binding layer, callbacks that are
// invoked from JS already run in the proper scope. // invoked from JS already run in the proper scope.
export function makeCallback( export function makeCallback<T>(
this: unknown, this: unknown,
cb?: (err: Error | null, result?: unknown) => void, cb?: (arg: T) => void,
) { ) {
validateFunction(cb, "cb"); validateFunction(cb, "cb");

View file

@ -1,41 +1,56 @@
// Copyright 2018-2025 the Deno authors. MIT license. // Copyright 2018-2025 the Deno authors. MIT license.
// TODO(petamoriken): enable prefer-primordials for node polyfills
// deno-lint-ignore-file prefer-primordials
import { op_node_fs_exists, op_node_fs_exists_sync } from "ext:core/ops"; import { op_node_fs_exists, op_node_fs_exists_sync } from "ext:core/ops";
import { getValidatedPathToString } from "ext:deno_node/internal/fs/utils.mjs";
import { primordials } from "ext:core/mod.js";
import { makeCallback } from "ext:deno_node/_fs/_fs_common.ts";
import type { Buffer } from "node:buffer";
import * as pathModule from "node:path";
import { kCustomPromisifiedSymbol } from "ext:deno_node/internal/util.mjs";
import { pathFromURL } from "ext:deno_web/00_infra.js"; const { ObjectDefineProperty, Promise, PromisePrototypeThen } = primordials;
type ExistsCallback = (exists: boolean) => void; type ExistsCallback = (exists: boolean) => void;
/** /**
* TODO: Also accept 'path' parameter as a Node polyfill Buffer type once these
* are implemented. See https://github.com/denoland/deno/issues/3403
* Deprecated in node api * Deprecated in node api
*/ */
export function exists(path: string | URL, callback: ExistsCallback) { export function exists(path: string | Buffer | URL, callback: ExistsCallback) {
path = path instanceof URL ? pathFromURL(path) : path; callback = makeCallback(callback);
op_node_fs_exists(path).then(callback);
try {
path = getValidatedPathToString(path);
} catch {
callback(false);
return;
}
PromisePrototypeThen(
op_node_fs_exists(pathModule.toNamespacedPath(path)),
callback,
);
} }
// The callback of fs.exists doesn't have standard callback signature. // The callback of fs.exists doesn't have standard callback signature.
// We need to provide special implementation for promisify. // We need to provide special implementation for promisify.
// See https://github.com/nodejs/node/pull/13316 // See https://github.com/nodejs/node/pull/13316
const kCustomPromisifiedSymbol = Symbol.for("nodejs.util.promisify.custom"); ObjectDefineProperty(exists, kCustomPromisifiedSymbol, {
Object.defineProperty(exists, kCustomPromisifiedSymbol, { __proto__: null,
value: (path: string | URL) => { value: (path: string | URL) => {
return new Promise((resolve) => { return new Promise((resolve) => {
exists(path, (exists) => resolve(exists)); exists(path, (exists) => resolve(exists));
}); });
}, },
enumerable: false,
writable: false,
configurable: true,
}); });
/** export function existsSync(path: string | Buffer | URL): boolean {
* TODO: Also accept 'path' parameter as a Node polyfill Buffer or URL type once these try {
* are implemented. See https://github.com/denoland/deno/issues/3403 path = getValidatedPathToString(path);
*/ } catch {
export function existsSync(path: string | URL): boolean { return false;
path = path instanceof URL ? pathFromURL(path) : path; }
return op_node_fs_exists_sync(path); return op_node_fs_exists_sync(pathModule.toNamespacedPath(path));
} }

View file

@ -46,7 +46,9 @@ export function once(callback) {
// In addition to being accessible through util.promisify.custom, // In addition to being accessible through util.promisify.custom,
// this symbol is registered globally and can be accessed in any environment as // this symbol is registered globally and can be accessed in any environment as
// Symbol.for('nodejs.util.promisify.custom'). // Symbol.for('nodejs.util.promisify.custom').
const kCustomPromisifiedSymbol = SymbolFor("nodejs.util.promisify.custom"); export const kCustomPromisifiedSymbol = SymbolFor(
"nodejs.util.promisify.custom",
);
// This is an internal Node symbol used by functions returning multiple // This is an internal Node symbol used by functions returning multiple
// arguments, e.g. ['bytesRead', 'buffer'] for fs.read(). // arguments, e.g. ['bytesRead', 'buffer'] for fs.read().
const kCustomPromisifyArgsSymbol = SymbolFor( const kCustomPromisifyArgsSymbol = SymbolFor(

View file

@ -424,6 +424,8 @@
"parallel/test-fs-close.js" = {} "parallel/test-fs-close.js" = {}
"parallel/test-fs-constants.js" = {} "parallel/test-fs-constants.js" = {}
"parallel/test-fs-empty-readStream.js" = {} "parallel/test-fs-empty-readStream.js" = {}
"parallel/test-fs-exists.js" = {}
"parallel/test-fs-existssync-false.js" = {}
"parallel/test-fs-fchown.js" = {} "parallel/test-fs-fchown.js" = {}
"parallel/test-fs-long-path.js" = {} "parallel/test-fs-long-path.js" = {}
"parallel/test-fs-promises-exists.js" = {} "parallel/test-fs-promises-exists.js" = {}