From a3158f436666edeee7a88aae0a3561a89d9360ea Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 28 Apr 2025 10:51:26 +0300 Subject: [PATCH 01/14] 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 | 62 +++++++++++++++++++++----------------------- 4 files changed, 76 insertions(+), 33 deletions(-) diff --git a/doc/api/os.md b/doc/api/os.md index 625d395e056786..3f3897f921e7f6 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 180ca49b3207eb..af4199b9706d03 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -876,6 +876,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 c44147f0e1170d..520579bee98fc7 100644 --- a/lib/os.js +++ b/lib/os.js @@ -39,7 +39,7 @@ const { }, hideStackFrames, } = require('internal/errors'); -const { getCIDR } = require('internal/util'); +const { getCIDR, guessHandleType: _guessHandleType } = require('internal/util'); const { validateInt32 } = require('internal/validators'); const { @@ -328,6 +328,7 @@ module.exports = { uptime: getUptime, version: getOSVersion, machine: getMachine, + guessHandleType: _guessHandleType, }; ObjectDefineProperties(module.exports, { diff --git a/src/node_util.cc b/src/node_util.cc index 85ef1c205e0d50..753d1081a01a27 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -49,8 +49,7 @@ CHAR_TEST(16, IsUnicodeSurrogate, (ch & 0xF800) == 0xD800) // If a UTF-16 surrogate is a low/trailing one. CHAR_TEST(16, IsUnicodeSurrogateTrail, (ch & 0x400) != 0) -static void GetOwnNonIndexProperties( - const FunctionCallbackInfo& args) { +static void GetOwnNonIndexProperties(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Local context = env->context(); @@ -62,20 +61,20 @@ static void GetOwnNonIndexProperties( Local properties; PropertyFilter filter = - static_cast(args[1].As()->Value()); - - if (!object->GetPropertyNames( - context, KeyCollectionMode::kOwnOnly, - filter, - IndexFilter::kSkipIndices) - .ToLocal(&properties)) { + static_cast(args[1].As()->Value()); + + 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(); @@ -84,8 +83,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(); @@ -98,15 +96,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(); @@ -116,8 +113,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(); @@ -125,10 +121,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))); @@ -164,8 +157,7 @@ static void GetCallerLocation(const FunctionCallbackInfo& args) { } static void PreviewEntries(const FunctionCallbackInfo& args) { - if (!args[0]->IsObject()) - return; + if (!args[0]->IsObject()) return; Environment* env = Environment::GetCurrent(args); bool is_key_value; @@ -173,13 +165,9 @@ 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(env->isolate(), is_key_value) - }; + Local ret[] = {entries, Boolean::New(env->isolate(), is_key_value)}; return args.GetReturnValue().Set( Array::New(env->isolate(), ret, arraysize(ret))); } @@ -216,15 +204,25 @@ 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; } } static void GuessHandleType(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + int fd; if (!args[0]->Int32Value(env->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 53db798d420d7edbb13087a136a0d909e025d66a Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 28 Apr 2025 11:19:14 +0300 Subject: [PATCH 02/14] 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 af4199b9706d03..09b9bc91f4c9b2 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -8,6 +8,7 @@ const { Error, ErrorCaptureStackTrace, FunctionPrototypeCall, + NumberIsInteger, NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, @@ -879,6 +880,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 7833ca7ab3649f1a402182cbdb6e398b08046805 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 28 Apr 2025 17:00:07 +0300 Subject: [PATCH 03/14] 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 72876650a3f57d59fb204f4b20e397497f2b04b8 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Tue, 29 Apr 2025 15:50:44 +0300 Subject: [PATCH 04/14] 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 3f3897f921e7f6..335af65bae6aee 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 09b9bc91f4c9b2..a41bb9cd1a42ad 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -877,7 +877,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 d0f4463a2487cd1b974e15285418cab2b3dd580c Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sat, 3 May 2025 00:09:12 +0300 Subject: [PATCH 05/14] 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 335af65bae6aee..2100c18744e5f4 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 520579bee98fc7..dd876b16de6dda 100644 --- a/lib/os.js +++ b/lib/os.js @@ -328,7 +328,7 @@ module.exports = { uptime: getUptime, version: getOSVersion, machine: getMachine, - guessHandleType: _guessHandleType, + guessFileDescriptorType: _guessHandleType, }; ObjectDefineProperties(module.exports, { 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 aa5d90ffb0e3e723324e57efb076db28e7199d4e Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Thu, 15 May 2025 12:57:44 +0300 Subject: [PATCH 06/14] 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 dd876b16de6dda..d63c1ee319c6e0 100644 --- a/lib/os.js +++ b/lib/os.js @@ -39,7 +39,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 { @@ -328,7 +328,7 @@ module.exports = { uptime: getUptime, version: getOSVersion, machine: getMachine, - guessFileDescriptorType: _guessHandleType, + guessFileDescriptorType, }; ObjectDefineProperties(module.exports, { From fc85d5bf0b371240474ff79b1c26ce72e194593f Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Fri, 16 May 2025 10:32:39 +0300 Subject: [PATCH 07/14] 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 2100c18744e5f4..8170ab77d348ea 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 a41bb9cd1a42ad..0369fa0e3e9891 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -49,6 +49,7 @@ const { const { codes: { + ERR_INVALID_FD, ERR_NO_CRYPTO, ERR_NO_TYPESCRIPT, ERR_UNKNOWN_SIGNAL, @@ -877,11 +878,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 753d1081a01a27..2ddcb41c1cbc4e 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -205,7 +205,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; } @@ -218,7 +218,7 @@ static void GuessHandleType(const FunctionCallbackInfo& args) { if (!args[0]->Int32Value(env->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 a65ced539dd2e456686f6c15e724ce487b93874e Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Fri, 20 Jun 2025 16:00:30 +0300 Subject: [PATCH 08/14] chore: remove wrong primordial destructure --- lib/internal/util.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 0369fa0e3e9891..f34fb8f67aabe8 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -8,7 +8,6 @@ const { Error, ErrorCaptureStackTrace, FunctionPrototypeCall, - NumberIsInteger, NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, From 655f0522fb72fc81c62deb7c8593858a04447e9e Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 13:57:14 +0300 Subject: [PATCH 09/14] 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 7c4c796dba15cfe6108a9a312b83e0b5a2203e79 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 14:00:55 +0300 Subject: [PATCH 10/14] chore: remove reformatting changes --- src/node_util.cc | 49 +++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index 2ddcb41c1cbc4e..f4164ac85c1906 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -49,7 +49,8 @@ CHAR_TEST(16, IsUnicodeSurrogate, (ch & 0xF800) == 0xD800) // If a UTF-16 surrogate is a low/trailing one. CHAR_TEST(16, IsUnicodeSurrogateTrail, (ch & 0x400) != 0) -static void GetOwnNonIndexProperties(const FunctionCallbackInfo& args) { +static void GetOwnNonIndexProperties( + const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Local context = env->context(); @@ -61,20 +62,20 @@ static void GetOwnNonIndexProperties(const FunctionCallbackInfo& args) { Local properties; PropertyFilter filter = - static_cast(args[1].As()->Value()); - - if (!object - ->GetPropertyNames(context, - KeyCollectionMode::kOwnOnly, - filter, - IndexFilter::kSkipIndices) - .ToLocal(&properties)) { + static_cast(args[1].As()->Value()); + + 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(); @@ -83,7 +84,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(); @@ -96,14 +98,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(); @@ -113,7 +116,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(); @@ -121,7 +125,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))); @@ -157,7 +164,8 @@ static void GetCallerLocation(const FunctionCallbackInfo& args) { } static void PreviewEntries(const FunctionCallbackInfo& args) { - if (!args[0]->IsObject()) return; + if (!args[0]->IsObject()) + return; Environment* env = Environment::GetCurrent(args); bool is_key_value; @@ -165,9 +173,13 @@ 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(env->isolate(), is_key_value)}; + Local ret[] = { + entries, + Boolean::New(env->isolate(), is_key_value) + }; return args.GetReturnValue().Set( Array::New(env->isolate(), ret, arraysize(ret))); } @@ -213,7 +225,6 @@ static uint32_t GetUVHandleTypeCode(const uv_handle_type type) { static void GuessHandleType(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - int fd; if (!args[0]->Int32Value(env->context()).To(&fd)) return; From 3b85e0b1e5c2fed75db459eecbd1aec9edcc6226 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 14:03:31 +0300 Subject: [PATCH 11/14] 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 e986407f19b7bdb27a6198c6be4153c3f33ddb65 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Sun, 22 Jun 2025 14:39:53 +0300 Subject: [PATCH 12/14] 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 f34fb8f67aabe8..f244fff7f5fca1 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -880,7 +880,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 b8914879eec445f6c663b4d050e46728980b3434 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Tue, 24 Jun 2025 12:29:39 +0300 Subject: [PATCH 13/14] 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 f244fff7f5fca1..c1c374fbcb5ce0 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -877,7 +877,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) { @@ -885,7 +884,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 f4164ac85c1906..b5b88b87ac4993 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(env->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 52dd18213e5b10bc8ccbbb6a410bb90caab7d6e5 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Tue, 24 Jun 2025 12:32:44 +0300 Subject: [PATCH 14/14] 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 8170ab77d348ea..572c3756c78acf 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: