From ba62af9e985c19966699a6b614262bc68a07c519 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Wed, 5 Feb 2025 20:10:43 -0500 Subject: [PATCH] feat(node:net): `node:net` compatability --- src/js/internal/cluster/child.ts | 2 +- src/js/internal/errors.ts | 22 ++++++++ src/js/node/net.ts | 36 ++++++++++--- .../test/parallel/test-net-after-close.js | 51 +++++++++++++++++++ .../parallel/test-net-listen-invalid-port.js | 44 ++++++++++++++++ .../test/parallel/test-net-listen-ipv6only.js | 30 +++++++++++ 6 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 test/js/node/test/parallel/test-net-after-close.js create mode 100644 test/js/node/test/parallel/test-net-listen-invalid-port.js create mode 100644 test/js/node/test/parallel/test-net-listen-ipv6only.js diff --git a/src/js/internal/cluster/child.ts b/src/js/internal/cluster/child.ts index c1e6ebdb38b183..2b7578a2d94091 100644 --- a/src/js/internal/cluster/child.ts +++ b/src/js/internal/cluster/child.ts @@ -54,7 +54,7 @@ cluster._setupWorker = function () { }; // `obj` is a net#Server or a dgram#Socket object. -cluster._getServer = function (obj, options, cb) { +cluster._getServer = function _getServer(obj, options, cb) { let address = options.address; // Resolve unix socket paths to absolute paths diff --git a/src/js/internal/errors.ts b/src/js/internal/errors.ts index b0e84f460e81e7..d7b4be0cf18550 100644 --- a/src/js/internal/errors.ts +++ b/src/js/internal/errors.ts @@ -2,6 +2,8 @@ const { SafeArrayIterator } = require("internal/primordials"); const ArrayIsArray = Array.isArray; const ArrayPrototypePush = Array.prototype.push; +const ReflectApply = Reflect.$apply; +const ErrorCaptureStackTrace = Error.captureStackTrace; function aggregateTwoErrors(innerError, outerError) { if (innerError && outerError && innerError !== outerError) { @@ -17,6 +19,26 @@ function aggregateTwoErrors(innerError, outerError) { return innerError || outerError; } +/** + * This function removes unnecessary frames from Node.js core errors. + * @template {(...args: unknown[]) => unknown} T + * @param {T} fn + * @returns {T} + */ +function hideStackFrames(fn) { + function wrappedFn(...args) { + try { + return ReflectApply(fn, this, args); + } catch (error) { + Error.stackTraceLimit && $isObject(error) && ErrorCaptureStackTrace(error, wrappedFn); + throw error; + } + } + wrappedFn.withoutStackTrace = fn; + return wrappedFn; +} + export default { aggregateTwoErrors, + hideStackFrames, }; diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 927a2dab051da5..c08cebcd0054f4 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -24,6 +24,7 @@ const { Duplex } = require("node:stream"); const EventEmitter = require("node:events"); const { addServerName, upgradeDuplexToTLS, isNamedPipeSocket } = require("../internal/net"); const { ExceptionWithHostPort } = require("internal/shared"); +const { validatePort } = require("internal/validators"); // IPv4 Segment const v4Seg = "(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])"; @@ -1203,13 +1204,14 @@ Server.prototype.getConnections = function (callback) { return this; }; -Server.prototype.listen = function (port, hostname, onListen) { +Server.prototype.listen = function listen(port, hostname, onListen) { let backlog; let path; let exclusive = false; let allowHalfOpen = false; let reusePort = false; let ipv6Only = false; + let options: Record | undefined; //port is actually path if (typeof port === "string") { if (Number.isSafeInteger(hostname)) { @@ -1235,13 +1237,17 @@ Server.prototype.listen = function (port, hostname, onListen) { onListen = port; port = 0; } else if (typeof port === "object") { - const options = port; + if (port === null) { + throw $ERR_INVALID_ARG_VALUE("options", port, "a non-null object"); + } + options = port; + $assert(options); // make tsc happy options.signal?.addEventListener("abort", () => this.close()); hostname = options.host; exclusive = options.exclusive; path = options.path; - port = options.port; + port = validatePort(options.port, "options.port"); ipv6Only = options.ipv6Only; allowHalfOpen = options.allowHalfOpen; reusePort = options.reusePort; @@ -1271,8 +1277,6 @@ Server.prototype.listen = function (port, hostname, onListen) { error.code = "ERR_INVALID_ARG_VALUE"; throw error; } - } else if (!Number.isSafeInteger(port) || port < 0) { - port = 0; } // port @@ -1286,8 +1290,11 @@ Server.prototype.listen = function (port, hostname, onListen) { // signal An AbortSignal that may be used to close a listening server. if (typeof options.callback === "function") onListen = options?.callback; - } else if (!Number.isSafeInteger(port) || port < 0) { - port = 0; + } + + // port is already validated when an options object gets passed + if ($isUndefinedOrNull(options)) { + port = validatePort(port, "port"); } hostname = hostname || "::"; } @@ -1445,8 +1452,23 @@ function listenInCluster( onListen, ) { exclusive = !!exclusive; + // shim until SocketAddress gets added. lazy-loaded b/c isIP is quite expensive. + const getConnKeyShim = (() => { + let key; + return () => { + if (!key) { + key = isIP(hostname) + ":" + hostname + ":" + port; + } + return key; + }; + })(); if (cluster === undefined) cluster = require("node:cluster"); + Object.defineProperty(server, "_connectionKey", { + get() { + return getConnKeyShim(); + }, + }); if (cluster.isPrimary || exclusive) { server[kRealListen](path, port, hostname, exclusive, ipv6Only, allowHalfOpen, reusePort, tls, contexts, onListen); diff --git a/test/js/node/test/parallel/test-net-after-close.js b/test/js/node/test/parallel/test-net-after-close.js new file mode 100644 index 00000000000000..413e8f75992de9 --- /dev/null +++ b/test/js/node/test/parallel/test-net-after-close.js @@ -0,0 +1,51 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const server = net.createServer(common.mustCall((s) => { + console.error('SERVER: got connection'); + s.end(); +})); + +server.listen(0, common.mustCall(() => { + const c = net.createConnection(server.address().port); + c.on('close', common.mustCall(() => { + /* eslint-disable no-unused-expressions */ + console.error('connection closed'); + assert.strictEqual(c._handle, null); + // Calling functions / accessing properties of a closed socket should not + // throw. + c.setNoDelay(); + c.setKeepAlive(); + c.bufferSize; + c.pause(); + c.resume(); + c.address(); + c.remoteAddress; + c.remotePort; + server.close(); + /* eslint-enable no-unused-expressions */ + })); +})); diff --git a/test/js/node/test/parallel/test-net-listen-invalid-port.js b/test/js/node/test/parallel/test-net-listen-invalid-port.js new file mode 100644 index 00000000000000..9e8ba1b715195c --- /dev/null +++ b/test/js/node/test/parallel/test-net-listen-invalid-port.js @@ -0,0 +1,44 @@ +'use strict'; +const common = require('../common'); + +// This test ensures that port numbers are validated in *all* kinds of `listen` +// calls. If an invalid port is supplied, ensures a `RangeError` is thrown. +// https://github.com/nodejs/node/issues/5727 + +const assert = require('assert'); +const net = require('net'); + +const invalidPort = -1 >>> 0; + +const s = net.Server(); +s.listen(0, function() { + const address = this.address(); + const key = `${address.family.slice(-1)}:${address.address}:0`; + + assert.strictEqual(this._connectionKey, key); + this.close(); +}); + +// The first argument is a configuration object +assert.throws(() => { + net.Server().listen({ port: invalidPort }, common.mustNotCall()); +}, { + code: 'ERR_SOCKET_BAD_PORT', + name: 'RangeError' +}); + +// The first argument is the port, no IP given. +assert.throws(() => { + net.Server().listen(invalidPort, common.mustNotCall()); +}, { + code: 'ERR_SOCKET_BAD_PORT', + name: 'RangeError' +}); + +// The first argument is the port, the second an IP. +assert.throws(() => { + net.Server().listen(invalidPort, '0.0.0.0', common.mustNotCall()); +}, { + code: 'ERR_SOCKET_BAD_PORT', + name: 'RangeError' +}); diff --git a/test/js/node/test/parallel/test-net-listen-ipv6only.js b/test/js/node/test/parallel/test-net-listen-ipv6only.js new file mode 100644 index 00000000000000..a329011bcc8a23 --- /dev/null +++ b/test/js/node/test/parallel/test-net-listen-ipv6only.js @@ -0,0 +1,30 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasIPv6) + common.skip('no IPv6 support'); + +// This test ensures that dual-stack support is disabled when +// we specify the `ipv6Only` option in `net.Server.listen()`. +const assert = require('assert'); +const net = require('net'); + +const host = '::'; +const server = net.createServer(); +server.listen({ + host, + port: 0, + ipv6Only: true, +}, common.mustCall(() => { + const { port } = server.address(); + const socket = net.connect({ + host: '0.0.0.0', + port, + }); + + socket.on('connect', common.mustNotCall()); + socket.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ECONNREFUSED'); + server.close(); + })); +}));