From 2827aaaa02edc09a414f94704fac60ef36f1cf7d Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 28 Apr 2025 10:51:26 +0300 Subject: [PATCH 01/15] os: expose `guessFileDescriptorType` Exposes the internal `guessHandleType` function as `guessFileDescriptorType`, which can be used to see if a handle has a specific type, regardless of the OS it is on. This helps out with detecting, for example, if standard input is piped into the process, instead of relying on file system calls. Refs: https://github.com/nodejs/node/issues/57603 --- doc/api/os.md | 42 +++++++++++++++++++++++++++++++++++++ lib/internal/util.js | 2 ++ lib/os.js | 3 ++- src/node_util.cc | 49 ++++++++++++++++++++++---------------------- 4 files changed, 71 insertions(+), 25 deletions(-) diff --git a/doc/api/os.md b/doc/api/os.md index 9f2aa0390cc56a..e920ffee8156e0 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -508,6 +508,48 @@ On POSIX systems, the operating system release is determined by calling available, `GetVersionExW()` will be used. See for more information. +## `os.guessHandleType(handle)` + + + +* `handle` {integer} The handle number to try and guess the type of. + +* Returns: {string} + +Returns the type of the handle passed in, or `'INVALID'` if the provided handle +is invalid. + +Currently, the following types for a handle can be returned: + + + + + + + + + + + + + + + + + + + + + + + + + + +
Constant
TCP
TTY
UDP
FILE
PIPE
UNKNOWN
INVALID
+ ## OS constants The following constants are exported by `os.constants`. diff --git a/lib/internal/util.js b/lib/internal/util.js index 28bb83e558c426..cebbd7f77572c2 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -898,6 +898,8 @@ function getCIDR(address, netmask, family) { } const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN']; +handleTypes[-1] = 'INVALID'; + function guessHandleType(fd) { const type = _guessHandleType(fd); return handleTypes[type]; diff --git a/lib/os.js b/lib/os.js index 5e53879bd6d5aa..b339f6cc2d5b33 100644 --- a/lib/os.js +++ b/lib/os.js @@ -40,7 +40,7 @@ const { }, hideStackFrames, } = require('internal/errors'); -const { getCIDR } = require('internal/util'); +const { getCIDR, guessHandleType: _guessHandleType } = require('internal/util'); const { validateInt32 } = require('internal/validators'); const { @@ -329,6 +329,7 @@ module.exports = { uptime: getUptime, version: getOSVersion, machine: getMachine, + guessHandleType: _guessHandleType, }; ObjectFreeze(constants.signals); diff --git a/src/node_util.cc b/src/node_util.cc index fbfda9c1551e07..230412da7f145b 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -67,18 +67,18 @@ static void GetOwnNonIndexProperties( PropertyFilter filter = FromV8Value(args[1]); - if (!object->GetPropertyNames( - context, KeyCollectionMode::kOwnOnly, - filter, - IndexFilter::kSkipIndices) - .ToLocal(&properties)) { + if (!object + ->GetPropertyNames(context, + KeyCollectionMode::kOwnOnly, + filter, + IndexFilter::kSkipIndices) + .ToLocal(&properties)) { return; } args.GetReturnValue().Set(properties); } -static void GetConstructorName( - const FunctionCallbackInfo& args) { +static void GetConstructorName(const FunctionCallbackInfo& args) { CHECK(args[0]->IsObject()); Local object = args[0].As(); @@ -87,8 +87,7 @@ static void GetConstructorName( args.GetReturnValue().Set(name); } -static void GetExternalValue( - const FunctionCallbackInfo& args) { +static void GetExternalValue(const FunctionCallbackInfo& args) { CHECK(args[0]->IsExternal()); Isolate* isolate = args.GetIsolate(); Local external = args[0].As(); @@ -101,15 +100,14 @@ static void GetExternalValue( static void GetPromiseDetails(const FunctionCallbackInfo& args) { // Return undefined if it's not a Promise. - if (!args[0]->IsPromise()) - return; + if (!args[0]->IsPromise()) return; auto isolate = args.GetIsolate(); Local promise = args[0].As(); int state = promise->State(); - Local values[2] = { Integer::New(isolate, state) }; + Local values[2] = {Integer::New(isolate, state)}; size_t number_of_values = 1; if (state != Promise::PromiseState::kPending) values[number_of_values++] = promise->Result(); @@ -119,8 +117,7 @@ static void GetPromiseDetails(const FunctionCallbackInfo& args) { static void GetProxyDetails(const FunctionCallbackInfo& args) { // Return undefined if it's not a proxy. - if (!args[0]->IsProxy()) - return; + if (!args[0]->IsProxy()) return; Local proxy = args[0].As(); @@ -128,10 +125,7 @@ static void GetProxyDetails(const FunctionCallbackInfo& args) { // the util binding layer. It's accessed in the wild and `esm` would break in // case the check is removed. if (args.Length() == 1 || args[1]->IsTrue()) { - Local ret[] = { - proxy->GetTarget(), - proxy->GetHandler() - }; + Local ret[] = {proxy->GetTarget(), proxy->GetHandler()}; args.GetReturnValue().Set( Array::New(args.GetIsolate(), ret, arraysize(ret))); @@ -167,8 +161,7 @@ static void GetCallerLocation(const FunctionCallbackInfo& args) { } static void PreviewEntries(const FunctionCallbackInfo& args) { - if (!args[0]->IsObject()) - return; + if (!args[0]->IsObject()) return; Isolate* isolate = args.GetIsolate(); bool is_key_value; @@ -176,8 +169,7 @@ static void PreviewEntries(const FunctionCallbackInfo& args) { if (!args[0].As()->PreviewEntries(&is_key_value).ToLocal(&entries)) return; // Fast path for WeakMap and WeakSet. - if (args.Length() == 1) - return args.GetReturnValue().Set(entries); + if (args.Length() == 1) return args.GetReturnValue().Set(entries); Local ret[] = {entries, Boolean::New(isolate, is_key_value)}; return args.GetReturnValue().Set(Array::New(isolate, ret, arraysize(ret))); @@ -215,7 +207,10 @@ static uint32_t GetUVHandleTypeCode(const uv_handle_type type) { case UV_UNKNOWN_HANDLE: return 5; default: - ABORT(); + // For an unhandled handle type, we want to return `UNKNOWN` instead of + // `INVALID` since the type is "known" by UV, just not exposed further to + // JS land + return 5; } } @@ -224,7 +219,13 @@ static void GuessHandleType(const FunctionCallbackInfo& args) { Local context = isolate->GetCurrentContext(); int fd; if (!args[0]->Int32Value(context).To(&fd)) return; - CHECK_GE(fd, 0); + + // If the provided file descriptor is not valid, we return `-1`, which in JS + // land will be marked as "INVALID" + if (fd < 0) [[unlikely]] { + args.GetReturnValue().Set(-1); + return; + } uv_handle_type t = uv_guess_handle(fd); args.GetReturnValue().Set(GetUVHandleTypeCode(t)); From 35b42c2b8903352f505c83e4ca849c1688533861 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 28 Apr 2025 11:19:14 +0300 Subject: [PATCH 02/15] fix: validate fd in JS land + add test --- lib/internal/util.js | 5 +++++ test/pseudo-tty/test-os-guessHandleType.js | 15 +++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 test/pseudo-tty/test-os-guessHandleType.js diff --git a/lib/internal/util.js b/lib/internal/util.js index cebbd7f77572c2..3147842694cc3e 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -9,6 +9,7 @@ const { ErrorCaptureStackTrace, FunctionPrototypeCall, FunctionPrototypeSymbolHasInstance, + NumberIsInteger, NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, @@ -901,6 +902,10 @@ const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN']; handleTypes[-1] = 'INVALID'; function guessHandleType(fd) { + if (!NumberIsInteger(fd)) { + return 'INVALID'; + } + const type = _guessHandleType(fd); return handleTypes[type]; } diff --git a/test/pseudo-tty/test-os-guessHandleType.js b/test/pseudo-tty/test-os-guessHandleType.js new file mode 100644 index 00000000000000..3540bf94a6ccda --- /dev/null +++ b/test/pseudo-tty/test-os-guessHandleType.js @@ -0,0 +1,15 @@ +'use strict'; + +require('../common'); +const { strictEqual } = require('assert'); +const { guessHandleType } = require('os'); + +strictEqual(guessHandleType(0), 'TTY', 'stdin reported to not be a tty, but it is'); +strictEqual(guessHandleType(1), 'TTY', 'stdout reported to not be a tty, but it is'); +strictEqual(guessHandleType(2), 'TTY', 'stderr reported to not be a tty, but it is'); + +strictEqual(guessHandleType(-1), 'INVALID', '-1 reported to be a tty, but it is not'); +strictEqual(guessHandleType(55555), 'UNKNOWN', '55555 reported to be a tty, but it is not'); +strictEqual(guessHandleType(2 ** 31), 'INVALID', '2^31 reported to be a tty, but it is not'); +strictEqual(guessHandleType(1.1), 'INVALID', '1.1 reported to be a tty, but it is not'); +strictEqual(guessHandleType('1'), 'INVALID', '\'1\' reported to be a tty, but it is not'); From fe58d8be178689c321989afada906ca932923f2e Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 28 Apr 2025 17:00:07 +0300 Subject: [PATCH 03/15] chore: fix test and add more checks --- test/pseudo-tty/test-os-guessHandleType.js | 2 ++ test/pseudo-tty/test-os-guessHandleType.out | 0 2 files changed, 2 insertions(+) create mode 100644 test/pseudo-tty/test-os-guessHandleType.out diff --git a/test/pseudo-tty/test-os-guessHandleType.js b/test/pseudo-tty/test-os-guessHandleType.js index 3540bf94a6ccda..06e97f94093bb8 100644 --- a/test/pseudo-tty/test-os-guessHandleType.js +++ b/test/pseudo-tty/test-os-guessHandleType.js @@ -13,3 +13,5 @@ strictEqual(guessHandleType(55555), 'UNKNOWN', '55555 reported to be a tty, but strictEqual(guessHandleType(2 ** 31), 'INVALID', '2^31 reported to be a tty, but it is not'); strictEqual(guessHandleType(1.1), 'INVALID', '1.1 reported to be a tty, but it is not'); strictEqual(guessHandleType('1'), 'INVALID', '\'1\' reported to be a tty, but it is not'); +strictEqual(guessHandleType({}), 'INVALID', '{} reported to be a tty, but it is not'); +strictEqual(guessHandleType(() => {}), 'INVALID', '() => {} reported to be a tty, but it is not'); diff --git a/test/pseudo-tty/test-os-guessHandleType.out b/test/pseudo-tty/test-os-guessHandleType.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 From e8940d867619efeae332ac9410a0f9ad4d794764 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Tue, 29 Apr 2025 15:50:44 +0300 Subject: [PATCH 04/15] chore: suggested changes --- doc/api/os.md | 33 +++++++-------------------------- lib/internal/util.js | 2 +- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/doc/api/os.md b/doc/api/os.md index e920ffee8156e0..31097b1c0ceead 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -523,32 +523,13 @@ is invalid. Currently, the following types for a handle can be returned: - - - - - - - - - - - - - - - - - - - - - - - - - -
Constant
TCP
TTY
UDP
FILE
PIPE
UNKNOWN
INVALID
+* `'TCP'` +* `'TTY'` +* `'UDP'` +* `'FILE'` +* `'PIPE'` +* `'UNKNOWN'` +* `'INVALID'` ## OS constants diff --git a/lib/internal/util.js b/lib/internal/util.js index 3147842694cc3e..055823061b5c34 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -899,7 +899,7 @@ function getCIDR(address, netmask, family) { } const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN']; -handleTypes[-1] = 'INVALID'; +setOwnProperty(handleTypes, -1, 'INVALID'); function guessHandleType(fd) { if (!NumberIsInteger(fd)) { From 8f2da7c20462f836db36283ee97cca762caded72 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sat, 3 May 2025 00:09:12 +0300 Subject: [PATCH 05/15] chore: rename to os.guessFileDescriptorType --- doc/api/os.md | 8 ++++---- lib/os.js | 2 +- .../test-os-guessFileDescriptorType.js | 17 +++++++++++++++++ ....out => test-os-guessFileDescriptorType.out} | 0 test/pseudo-tty/test-os-guessHandleType.js | 17 ----------------- 5 files changed, 22 insertions(+), 22 deletions(-) create mode 100644 test/pseudo-tty/test-os-guessFileDescriptorType.js rename test/pseudo-tty/{test-os-guessHandleType.out => test-os-guessFileDescriptorType.out} (100%) delete mode 100644 test/pseudo-tty/test-os-guessHandleType.js diff --git a/doc/api/os.md b/doc/api/os.md index 31097b1c0ceead..38726a85694d20 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -508,20 +508,20 @@ On POSIX systems, the operating system release is determined by calling available, `GetVersionExW()` will be used. See for more information. -## `os.guessHandleType(handle)` +## `os.guessFileDescriptorType(fd)` -* `handle` {integer} The handle number to try and guess the type of. +* `fd` {integer} The file descriptor number to try and guess the type of. * Returns: {string} -Returns the type of the handle passed in, or `'INVALID'` if the provided handle +Returns the type of the file descriptor passed in, or `'INVALID'` if the provided file descriptor is invalid. -Currently, the following types for a handle can be returned: +Currently, the following types for a file descriptor can be returned: * `'TCP'` * `'TTY'` diff --git a/lib/os.js b/lib/os.js index b339f6cc2d5b33..19e5721457a59b 100644 --- a/lib/os.js +++ b/lib/os.js @@ -329,7 +329,7 @@ module.exports = { uptime: getUptime, version: getOSVersion, machine: getMachine, - guessHandleType: _guessHandleType, + guessFileDescriptorType: _guessHandleType, }; ObjectFreeze(constants.signals); diff --git a/test/pseudo-tty/test-os-guessFileDescriptorType.js b/test/pseudo-tty/test-os-guessFileDescriptorType.js new file mode 100644 index 00000000000000..0130369b9cc023 --- /dev/null +++ b/test/pseudo-tty/test-os-guessFileDescriptorType.js @@ -0,0 +1,17 @@ +'use strict'; + +require('../common'); +const { strictEqual } = require('assert'); +const { guessFileDescriptorType } = require('os'); + +strictEqual(guessFileDescriptorType(0), 'TTY', 'stdin reported to not be a tty, but it is'); +strictEqual(guessFileDescriptorType(1), 'TTY', 'stdout reported to not be a tty, but it is'); +strictEqual(guessFileDescriptorType(2), 'TTY', 'stderr reported to not be a tty, but it is'); + +strictEqual(guessFileDescriptorType(-1), 'INVALID', '-1 reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType(2 ** 31), 'INVALID', '2^31 reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType(1.1), 'INVALID', '1.1 reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType('1'), 'INVALID', '\'1\' reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType({}), 'INVALID', '{} reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType(() => {}), 'INVALID', '() => {} reported to be a tty, but it is not'); diff --git a/test/pseudo-tty/test-os-guessHandleType.out b/test/pseudo-tty/test-os-guessFileDescriptorType.out similarity index 100% rename from test/pseudo-tty/test-os-guessHandleType.out rename to test/pseudo-tty/test-os-guessFileDescriptorType.out diff --git a/test/pseudo-tty/test-os-guessHandleType.js b/test/pseudo-tty/test-os-guessHandleType.js deleted file mode 100644 index 06e97f94093bb8..00000000000000 --- a/test/pseudo-tty/test-os-guessHandleType.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict'; - -require('../common'); -const { strictEqual } = require('assert'); -const { guessHandleType } = require('os'); - -strictEqual(guessHandleType(0), 'TTY', 'stdin reported to not be a tty, but it is'); -strictEqual(guessHandleType(1), 'TTY', 'stdout reported to not be a tty, but it is'); -strictEqual(guessHandleType(2), 'TTY', 'stderr reported to not be a tty, but it is'); - -strictEqual(guessHandleType(-1), 'INVALID', '-1 reported to be a tty, but it is not'); -strictEqual(guessHandleType(55555), 'UNKNOWN', '55555 reported to be a tty, but it is not'); -strictEqual(guessHandleType(2 ** 31), 'INVALID', '2^31 reported to be a tty, but it is not'); -strictEqual(guessHandleType(1.1), 'INVALID', '1.1 reported to be a tty, but it is not'); -strictEqual(guessHandleType('1'), 'INVALID', '\'1\' reported to be a tty, but it is not'); -strictEqual(guessHandleType({}), 'INVALID', '{} reported to be a tty, but it is not'); -strictEqual(guessHandleType(() => {}), 'INVALID', '() => {} reported to be a tty, but it is not'); From 68b1b9645c6611756b680daa76216bffd4291ae6 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Thu, 15 May 2025 12:57:44 +0300 Subject: [PATCH 06/15] chore: suggested changes Co-authored-by: James M Snell --- lib/os.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/os.js b/lib/os.js index 19e5721457a59b..34b228c4edb12a 100644 --- a/lib/os.js +++ b/lib/os.js @@ -40,7 +40,7 @@ const { }, hideStackFrames, } = require('internal/errors'); -const { getCIDR, guessHandleType: _guessHandleType } = require('internal/util'); +const { getCIDR, guessHandleType: guessFileDescriptorType } = require('internal/util'); const { validateInt32 } = require('internal/validators'); const { @@ -329,7 +329,7 @@ module.exports = { uptime: getUptime, version: getOSVersion, machine: getMachine, - guessFileDescriptorType: _guessHandleType, + guessFileDescriptorType, }; ObjectFreeze(constants.signals); From d2ae4489f762f4000f52e2d104893b2473300b90 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Fri, 16 May 2025 10:32:39 +0300 Subject: [PATCH 07/15] chore: requested changes --- doc/api/os.md | 5 ++--- lib/internal/util.js | 7 ++++--- src/node_util.cc | 4 ++-- .../test-os-guessFileDescriptorType.js | 18 ++++++++++-------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/doc/api/os.md b/doc/api/os.md index 38726a85694d20..31061a94781678 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -516,9 +516,9 @@ added: REPLACEME * `fd` {integer} The file descriptor number to try and guess the type of. -* Returns: {string} +* Returns: {string|null} -Returns the type of the file descriptor passed in, or `'INVALID'` if the provided file descriptor +Returns the type of the file descriptor passed in, or `null` if the provided file descriptor is invalid. Currently, the following types for a file descriptor can be returned: @@ -529,7 +529,6 @@ Currently, the following types for a file descriptor can be returned: * `'FILE'` * `'PIPE'` * `'UNKNOWN'` -* `'INVALID'` ## OS constants diff --git a/lib/internal/util.js b/lib/internal/util.js index 055823061b5c34..e2642e871e42d1 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -52,6 +52,7 @@ const { const { codes: { + ERR_INVALID_FD, ERR_NO_CRYPTO, ERR_NO_TYPESCRIPT, ERR_UNKNOWN_SIGNAL, @@ -899,11 +900,11 @@ function getCIDR(address, netmask, family) { } const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN']; -setOwnProperty(handleTypes, -1, 'INVALID'); +setOwnProperty(handleTypes, -1, null); function guessHandleType(fd) { - if (!NumberIsInteger(fd)) { - return 'INVALID'; + if (fd >> 0 !== fd || fd < 0) { + throw new ERR_INVALID_FD(fd); } const type = _guessHandleType(fd); diff --git a/src/node_util.cc b/src/node_util.cc index 230412da7f145b..4af59f7d6a1467 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -208,7 +208,7 @@ static uint32_t GetUVHandleTypeCode(const uv_handle_type type) { return 5; default: // For an unhandled handle type, we want to return `UNKNOWN` instead of - // `INVALID` since the type is "known" by UV, just not exposed further to + // `null` since the type is "known" by UV, just not exposed further to // JS land return 5; } @@ -221,7 +221,7 @@ static void GuessHandleType(const FunctionCallbackInfo& args) { if (!args[0]->Int32Value(context).To(&fd)) return; // If the provided file descriptor is not valid, we return `-1`, which in JS - // land will be marked as "INVALID" + // land will be marked as null if (fd < 0) [[unlikely]] { args.GetReturnValue().Set(-1); return; diff --git a/test/pseudo-tty/test-os-guessFileDescriptorType.js b/test/pseudo-tty/test-os-guessFileDescriptorType.js index 0130369b9cc023..1393ddee8e370e 100644 --- a/test/pseudo-tty/test-os-guessFileDescriptorType.js +++ b/test/pseudo-tty/test-os-guessFileDescriptorType.js @@ -1,17 +1,19 @@ 'use strict'; require('../common'); -const { strictEqual } = require('assert'); +const { strictEqual, throws } = require('assert'); const { guessFileDescriptorType } = require('os'); strictEqual(guessFileDescriptorType(0), 'TTY', 'stdin reported to not be a tty, but it is'); strictEqual(guessFileDescriptorType(1), 'TTY', 'stdout reported to not be a tty, but it is'); strictEqual(guessFileDescriptorType(2), 'TTY', 'stderr reported to not be a tty, but it is'); -strictEqual(guessFileDescriptorType(-1), 'INVALID', '-1 reported to be a tty, but it is not'); -strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a tty, but it is not'); -strictEqual(guessFileDescriptorType(2 ** 31), 'INVALID', '2^31 reported to be a tty, but it is not'); -strictEqual(guessFileDescriptorType(1.1), 'INVALID', '1.1 reported to be a tty, but it is not'); -strictEqual(guessFileDescriptorType('1'), 'INVALID', '\'1\' reported to be a tty, but it is not'); -strictEqual(guessFileDescriptorType({}), 'INVALID', '{} reported to be a tty, but it is not'); -strictEqual(guessFileDescriptorType(() => {}), 'INVALID', '() => {} reported to be a tty, but it is not'); +strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a handle, but it is not'); +strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN', '2^31-1 reported to be a handle, but it is not'); + +throws(() => guessFileDescriptorType(-1), /"fd" must be a positive integer/, '-1 reported to be a handle, but it is not'); +throws(() => guessFileDescriptorType(1.1), /"fd" must be a positive integer/, '1.1 reported to be a handle, but it is not'); +throws(() => guessFileDescriptorType('1'), /"fd" must be a positive integer/, '\'1\' reported to be a tty, but it is not'); +throws(() => guessFileDescriptorType({}), /"fd" must be a positive integer/, '{} reported to be a tty, but it is not'); +throws(() => guessFileDescriptorType(() => {}), /"fd" must be a positive integer/, '() => {} reported to be a tty, but it is not'); +throws(() => guessFileDescriptorType(2 ** 31), /"fd" must be a positive integer/, '2^31 reported to be a handle, but it is not (because the fd check rolls over the input to negative of it)'); From b37486234caff6a1c8644e17eace93b1eafccd17 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Fri, 20 Jun 2025 16:00:30 +0300 Subject: [PATCH 08/15] chore: remove wrong primordial destructure --- lib/internal/util.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index e2642e871e42d1..122873bf01bbf2 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -8,8 +8,6 @@ const { Error, ErrorCaptureStackTrace, FunctionPrototypeCall, - FunctionPrototypeSymbolHasInstance, - NumberIsInteger, NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, From 89497c9c5a29d342f7223405c14b6729ce37a8d1 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 13:57:14 +0300 Subject: [PATCH 09/15] chore: use forEach and assert based on error code Co-authored-by: Antoine du Hamel --- .../test-os-guessFileDescriptorType.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/test/pseudo-tty/test-os-guessFileDescriptorType.js b/test/pseudo-tty/test-os-guessFileDescriptorType.js index 1393ddee8e370e..99acc6a2c080c9 100644 --- a/test/pseudo-tty/test-os-guessFileDescriptorType.js +++ b/test/pseudo-tty/test-os-guessFileDescriptorType.js @@ -11,9 +11,18 @@ strictEqual(guessFileDescriptorType(2), 'TTY', 'stderr reported to not be a tty, strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a handle, but it is not'); strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN', '2^31-1 reported to be a handle, but it is not'); -throws(() => guessFileDescriptorType(-1), /"fd" must be a positive integer/, '-1 reported to be a handle, but it is not'); -throws(() => guessFileDescriptorType(1.1), /"fd" must be a positive integer/, '1.1 reported to be a handle, but it is not'); -throws(() => guessFileDescriptorType('1'), /"fd" must be a positive integer/, '\'1\' reported to be a tty, but it is not'); -throws(() => guessFileDescriptorType({}), /"fd" must be a positive integer/, '{} reported to be a tty, but it is not'); -throws(() => guessFileDescriptorType(() => {}), /"fd" must be a positive integer/, '() => {} reported to be a tty, but it is not'); -throws(() => guessFileDescriptorType(2 ** 31), /"fd" must be a positive integer/, '2^31 reported to be a handle, but it is not (because the fd check rolls over the input to negative of it)'); +[ + -1, + 1.1, + '1', + [], + {}, + () => {}, + 2 ** 31, + true, + false, + 1n, + Symbol(), + undefined, + null, +].forEach((val) => throws(() => guessFileDescriptorType(val), { code: 'ERR_INVALID_FD' })); From 7190ab01ff4813acb2d3b2726b363b706252207c Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 14:00:55 +0300 Subject: [PATCH 10/15] chore: remove reformatting changes --- src/node_util.cc | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index 4af59f7d6a1467..386298c8ef4606 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -67,18 +67,18 @@ static void GetOwnNonIndexProperties( PropertyFilter filter = FromV8Value(args[1]); - if (!object - ->GetPropertyNames(context, - KeyCollectionMode::kOwnOnly, - filter, - IndexFilter::kSkipIndices) - .ToLocal(&properties)) { + if (!object->GetPropertyNames( + context, KeyCollectionMode::kOwnOnly, + filter, + IndexFilter::kSkipIndices) + .ToLocal(&properties)) { return; } args.GetReturnValue().Set(properties); } -static void GetConstructorName(const FunctionCallbackInfo& args) { +static void GetConstructorName( + const FunctionCallbackInfo& args) { CHECK(args[0]->IsObject()); Local object = args[0].As(); @@ -87,7 +87,8 @@ static void GetConstructorName(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(name); } -static void GetExternalValue(const FunctionCallbackInfo& args) { +static void GetExternalValue( + const FunctionCallbackInfo& args) { CHECK(args[0]->IsExternal()); Isolate* isolate = args.GetIsolate(); Local external = args[0].As(); @@ -100,14 +101,15 @@ static void GetExternalValue(const FunctionCallbackInfo& args) { static void GetPromiseDetails(const FunctionCallbackInfo& args) { // Return undefined if it's not a Promise. - if (!args[0]->IsPromise()) return; + if (!args[0]->IsPromise()) + return; auto isolate = args.GetIsolate(); Local promise = args[0].As(); int state = promise->State(); - Local values[2] = {Integer::New(isolate, state)}; + Local values[2] = { Integer::New(isolate, state) }; size_t number_of_values = 1; if (state != Promise::PromiseState::kPending) values[number_of_values++] = promise->Result(); @@ -117,7 +119,8 @@ static void GetPromiseDetails(const FunctionCallbackInfo& args) { static void GetProxyDetails(const FunctionCallbackInfo& args) { // Return undefined if it's not a proxy. - if (!args[0]->IsProxy()) return; + if (!args[0]->IsProxy()) + return; Local proxy = args[0].As(); @@ -125,7 +128,10 @@ static void GetProxyDetails(const FunctionCallbackInfo& args) { // the util binding layer. It's accessed in the wild and `esm` would break in // case the check is removed. if (args.Length() == 1 || args[1]->IsTrue()) { - Local ret[] = {proxy->GetTarget(), proxy->GetHandler()}; + Local ret[] = { + proxy->GetTarget(), + proxy->GetHandler() + }; args.GetReturnValue().Set( Array::New(args.GetIsolate(), ret, arraysize(ret))); @@ -161,7 +167,8 @@ static void GetCallerLocation(const FunctionCallbackInfo& args) { } static void PreviewEntries(const FunctionCallbackInfo& args) { - if (!args[0]->IsObject()) return; + if (!args[0]->IsObject()) + return; Isolate* isolate = args.GetIsolate(); bool is_key_value; @@ -169,7 +176,8 @@ static void PreviewEntries(const FunctionCallbackInfo& args) { if (!args[0].As()->PreviewEntries(&is_key_value).ToLocal(&entries)) return; // Fast path for WeakMap and WeakSet. - if (args.Length() == 1) return args.GetReturnValue().Set(entries); + if (args.Length() == 1) + return args.GetReturnValue().Set(entries); Local ret[] = {entries, Boolean::New(isolate, is_key_value)}; return args.GetReturnValue().Set(Array::New(isolate, ret, arraysize(ret))); From c6421fd92579db259dec61274f65113116dfdcc5 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 14:03:31 +0300 Subject: [PATCH 11/15] chore: lint js file --- .../test-os-guessFileDescriptorType.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/pseudo-tty/test-os-guessFileDescriptorType.js b/test/pseudo-tty/test-os-guessFileDescriptorType.js index 99acc6a2c080c9..b927e7165f160a 100644 --- a/test/pseudo-tty/test-os-guessFileDescriptorType.js +++ b/test/pseudo-tty/test-os-guessFileDescriptorType.js @@ -12,17 +12,17 @@ strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a h strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN', '2^31-1 reported to be a handle, but it is not'); [ - -1, - 1.1, - '1', - [], - {}, - () => {}, - 2 ** 31, - true, - false, - 1n, - Symbol(), - undefined, - null, + -1, + 1.1, + '1', + [], + {}, + () => {}, + 2 ** 31, + true, + false, + 1n, + Symbol(), + undefined, + null, ].forEach((val) => throws(() => guessFileDescriptorType(val), { code: 'ERR_INVALID_FD' })); From 98b1ad3abbf9ab2d212316b1d1523b8ed1e2b2ea Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 14:39:53 +0300 Subject: [PATCH 12/15] fix: test fail --- lib/internal/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 122873bf01bbf2..d2c2a725ded493 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -901,7 +901,7 @@ const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN']; setOwnProperty(handleTypes, -1, null); function guessHandleType(fd) { - if (fd >> 0 !== fd || fd < 0) { + if (typeof fd !== 'number' || fd >> 0 !== fd || fd < 0) { throw new ERR_INVALID_FD(fd); } From 5246a4c85749c296289cdac8db0d802b227898a1 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Tue, 24 Jun 2025 12:29:39 +0300 Subject: [PATCH 13/15] chore: simplify return type Tested with `require('internal/test/binding').internalBinding('util').guessHandleType(2**31)` (not sure if there was a better way, but it works so) --- lib/internal/util.js | 3 +-- src/node_util.cc | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index d2c2a725ded493..1ab8be6ab6ad68 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -898,7 +898,6 @@ function getCIDR(address, netmask, family) { } const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN']; -setOwnProperty(handleTypes, -1, null); function guessHandleType(fd) { if (typeof fd !== 'number' || fd >> 0 !== fd || fd < 0) { @@ -906,7 +905,7 @@ function guessHandleType(fd) { } const type = _guessHandleType(fd); - return handleTypes[type]; + return handleTypes[type] || type; } class WeakReference { diff --git a/src/node_util.cc b/src/node_util.cc index 386298c8ef4606..f6bba30d9b7a05 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -228,10 +228,9 @@ static void GuessHandleType(const FunctionCallbackInfo& args) { int fd; if (!args[0]->Int32Value(context).To(&fd)) return; - // If the provided file descriptor is not valid, we return `-1`, which in JS - // land will be marked as null + // If the provided file descriptor is not valid, we return null if (fd < 0) [[unlikely]] { - args.GetReturnValue().Set(-1); + args.GetReturnValue().Set(v8::Null(env->isolate())); return; } From 8481ecfd89c49b9b36e41eb780bb4f4783676c5a Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Tue, 24 Jun 2025 12:32:44 +0300 Subject: [PATCH 14/15] docs: try to explain the use case for this function --- doc/api/os.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/os.md b/doc/api/os.md index 31061a94781678..cdac68de0f775f 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -520,6 +520,10 @@ added: REPLACEME Returns the type of the file descriptor passed in, or `null` if the provided file descriptor is invalid. +A common use case for this function is checking whether standard input is passed into your process, +and if it is, if it can be consumed by the process. For example, on Unix systems, if the type is `TTY`, it means +you can prompt the user for new data while the process is running, and if it's `FILE` or `PIPE`, it means there is data +available, but you shouldn't try to prompt for more. Currently, the following types for a file descriptor can be returned: From 520693d1aa49135212ed0083585e7ea98ab39987 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Thu, 9 Apr 2026 14:09:09 +0300 Subject: [PATCH 15/15] chore: fix lint Signed-off-by: Vlad Frangu --- lib/internal/util.js | 1 + src/node_util.cc | 2 +- test/pseudo-tty/test-os-guessFileDescriptorType.js | 14 +++++++------- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 1ab8be6ab6ad68..2bada838184488 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -8,6 +8,7 @@ const { Error, ErrorCaptureStackTrace, FunctionPrototypeCall, + FunctionPrototypeSymbolHasInstance, NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, diff --git a/src/node_util.cc b/src/node_util.cc index f6bba30d9b7a05..acd57d96e9826c 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -230,7 +230,7 @@ static void GuessHandleType(const FunctionCallbackInfo& args) { // If the provided file descriptor is not valid, we return null if (fd < 0) [[unlikely]] { - args.GetReturnValue().Set(v8::Null(env->isolate())); + args.GetReturnValue().Set(v8::Null(isolate)); return; } diff --git a/test/pseudo-tty/test-os-guessFileDescriptorType.js b/test/pseudo-tty/test-os-guessFileDescriptorType.js index b927e7165f160a..0efb4780aa9934 100644 --- a/test/pseudo-tty/test-os-guessFileDescriptorType.js +++ b/test/pseudo-tty/test-os-guessFileDescriptorType.js @@ -1,15 +1,15 @@ 'use strict'; require('../common'); -const { strictEqual, throws } = require('assert'); +const assert = require('node:assert'); const { guessFileDescriptorType } = require('os'); -strictEqual(guessFileDescriptorType(0), 'TTY', 'stdin reported to not be a tty, but it is'); -strictEqual(guessFileDescriptorType(1), 'TTY', 'stdout reported to not be a tty, but it is'); -strictEqual(guessFileDescriptorType(2), 'TTY', 'stderr reported to not be a tty, but it is'); +assert.strictEqual(guessFileDescriptorType(0), 'TTY'); +assert.strictEqual(guessFileDescriptorType(1), 'TTY'); +assert.strictEqual(guessFileDescriptorType(2), 'TTY'); -strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a handle, but it is not'); -strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN', '2^31-1 reported to be a handle, but it is not'); +assert.strictEqual(guessFileDescriptorType(55555), 'UNKNOWN'); +assert.strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN'); [ -1, @@ -25,4 +25,4 @@ strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN', '2^31-1 reported to Symbol(), undefined, null, -].forEach((val) => throws(() => guessFileDescriptorType(val), { code: 'ERR_INVALID_FD' })); +].forEach((val) => assert.throws(() => guessFileDescriptorType(val), { code: 'ERR_INVALID_FD' }));