From 9b76d5d73ba91ec730ce8218b4ee35614099b41d 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 Signed-off-by: Vlad Frangu --- 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 a2592a2f20cc..ea6db9990a46 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -504,6 +504,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 2f72e636ab90..c44e9c447f17 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -857,6 +857,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 5e53879bd6d5..b339f6cc2d5b 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 6d3373caae6c..97ad90ae93f3 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 5cae282ce76f75b0b52e653664992b0eabdc5c3c 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 Signed-off-by: Vlad Frangu --- 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 c44e9c447f17..fe87a4982574 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -9,6 +9,7 @@ const { ErrorCaptureStackTrace, FunctionPrototypeCall, FunctionPrototypeSymbolHasInstance, + NumberIsInteger, NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, @@ -860,6 +861,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 000000000000..3540bf94a6cc --- /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 e35dc45355c0764b61c73e76f9872dcc5da0e658 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 Signed-off-by: Vlad Frangu --- 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 3540bf94a6cc..06e97f94093b 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 000000000000..e69de29bb2d1 From 8185a402c87e44fabf4ee0c9cc21d2b922f70450 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 Signed-off-by: Vlad Frangu --- 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 ea6db9990a46..3d469538b5df 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -519,32 +519,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 fe87a4982574..cdb3c6b2328b 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -858,7 +858,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 bfd912bd04e665a1e4778f9bd96b7fa3f06385e5 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 Signed-off-by: Vlad Frangu --- 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 3d469538b5df..f349472e8066 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -504,20 +504,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 b339f6cc2d5b..19e5721457a5 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 000000000000..0130369b9cc0 --- /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 06e97f94093b..000000000000 --- 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 25ce0f36cb20f58530838bcdb75c3e98e5879450 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 Signed-off-by: Vlad Frangu --- lib/os.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/os.js b/lib/os.js index 19e5721457a5..34b228c4edb1 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 e16078c38cc373d30dfaa6869efb9728a3718f78 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 Signed-off-by: Vlad Frangu --- 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 f349472e8066..23ecedab200a 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -512,9 +512,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: @@ -525,7 +525,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 cdb3c6b2328b..8e369e4669bf 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -51,6 +51,7 @@ const { const { codes: { + ERR_INVALID_FD, ERR_NO_CRYPTO, ERR_NO_TYPESCRIPT, ERR_UNKNOWN_SIGNAL, @@ -858,11 +859,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 97ad90ae93f3..01406ff7d0e1 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 0130369b9cc0..1393ddee8e37 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 913f765921c3b34e05100053098509b436d0788d 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 Signed-off-by: Vlad Frangu --- lib/internal/util.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 8e369e4669bf..0600ce14a2d4 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 8fdfea73e9074307b698c778c612a4c5e802bdb0 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 Signed-off-by: Vlad Frangu --- .../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 1393ddee8e37..99acc6a2c080 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 c85e27812f204a0569f89c0121a939c4a540dc8d 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 Signed-off-by: Vlad Frangu --- 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 01406ff7d0e1..01d58964f921 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 f5dbd5b1c63d272b69c91d07aa566e6f526eb349 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 Signed-off-by: Vlad Frangu --- .../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 99acc6a2c080..b927e7165f16 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 c1157008b38244ebdf5b99f44dd45eae041d4791 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 Signed-off-by: Vlad Frangu --- 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 0600ce14a2d4..db5381bc8610 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -860,7 +860,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 826978c78700995bda97ec3afed86ffd27132976 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) Signed-off-by: Vlad Frangu --- 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 db5381bc8610..1d3c8adeb183 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -857,7 +857,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) { @@ -865,7 +864,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 01d58964f921..28b734ab9e26 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 45fec1b4c6e040202a29a84ca02a59cab6c1a020 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 Signed-off-by: Vlad Frangu --- doc/api/os.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/os.md b/doc/api/os.md index 23ecedab200a..cd9c6b51a47d 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -516,6 +516,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 ad060ccfb003a11cbb98a22bafb23eb2bd1f0996 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 1d3c8adeb183..c33a95f34f61 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 28b734ab9e26..6345ef5eef67 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 b927e7165f16..0efb4780aa99 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' }));