From 593b5bd77ea444ac01e8bb957c7997eeabe45072 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Sat, 10 Oct 2026 08:27:36 +0200 Subject: [PATCH] fs: fix do_not_throw_error argument index in sync fstat The synchronous branch of the fstat binding read its do_not_throw_error flag from args[2], which is always undefined on that branch, so the flag was ignored and the binding always threw. Read the flag from args[3] as documented, and restore tryStatSync() to closing the file descriptor it owns before rethrowing the fstat error, which was its behaviour before the flag was introduced. Refs: https://github.com/nodejs/node/pull/49868 Assisted-by: MiniMax M3.1 Flash Preview + Opus 5.5 Signed-off-by: Daniel Lockyer --- lib/fs.js | 10 +++++-- src/node_file.cc | 2 +- test/parallel/test-fs-fstat-sync-error.js | 35 +++++++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-fs-fstat-sync-error.js diff --git a/lib/fs.js b/lib/fs.js index 4436fa2df6e6..64aae5f2cdc6 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -516,9 +516,13 @@ function readFileAfterOneShot(err, buffer, fd, size, closeErr) { } function tryStatSync(fd, isUserFd) { - const stats = binding.fstat(fd, false, undefined, true /* shouldNotThrow */); - if (stats === undefined && !isUserFd) { - fs.closeSync(fd); + let threw = true; + let stats; + try { + stats = binding.fstat(fd, false, undefined, false); + threw = false; + } finally { + if (threw && !isUserFd) fs.closeSync(fd); } return stats; } diff --git a/src/node_file.cc b/src/node_file.cc index 0bc7511ccb54..9eac793bfa82 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1287,7 +1287,7 @@ static void FStat(const FunctionCallbackInfo& args) { AsyncCall( env, req_wrap_async, args, "fstat", UTF8, AfterStat, uv_fs_fstat, fd); } else { // fstat(fd, use_bigint, undefined, do_not_throw_error) - bool do_not_throw_error = args[2]->IsTrue(); + bool do_not_throw_error = args[3]->IsTrue(); const auto should_throw = [do_not_throw_error](int result) { return is_uv_error(result) && !do_not_throw_error; }; diff --git a/test/parallel/test-fs-fstat-sync-error.js b/test/parallel/test-fs-fstat-sync-error.js new file mode 100644 index 000000000000..3b723f12c0de --- /dev/null +++ b/test/parallel/test-fs-fstat-sync-error.js @@ -0,0 +1,35 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const { internalBinding } = require('internal/test/binding'); + +const binding = internalBinding('fs'); + +const badFd = fs.openSync(__filename, 'r'); +fs.closeSync(badFd); + +// The synchronous fstat binding honours its do_not_throw_error argument. +assert.strictEqual(binding.fstat(badFd, false, undefined, true), undefined); +assert.throws(() => binding.fstat(badFd, false, undefined, false), { + code: 'EBADF', + syscall: 'fstat', +}); + +// readFileSync() surfaces the fstat error and closes a file descriptor it +// opened itself, but leaves a user-supplied one alone. +fs.openSync = () => badFd; +fs.closeSync = common.mustCall((fd) => { + assert.strictEqual(fd, badFd); +}); + +assert.throws(() => fs.readFileSync('dummy', 'latin1'), { + code: 'EBADF', + syscall: 'fstat', +}); +assert.throws(() => fs.readFileSync(badFd, 'latin1'), { + code: 'EBADF', + syscall: 'fstat', +});