diff --git a/src/js/node/dns.ts b/src/js/node/dns.ts index 2c136ed2128b..4e9d103cf574 100644 --- a/src/js/node/dns.ts +++ b/src/js/node/dns.ts @@ -405,7 +405,7 @@ function lookupService( if (isIP(address) === 0) { throw $ERR_INVALID_ARG_VALUE("address", address); } - validatePort(port, "port"); + validatePort(port); if (typeof callback !== "function") { throw $ERR_INVALID_ARG_TYPE("callback", "function", callback); } @@ -860,7 +860,7 @@ const promises = { if (isIP(address) === 0) { throw $ERR_INVALID_ARG_VALUE("address", address); } - validatePort(port, "port"); + validatePort(port); try { return translateErrorCode(dns.lookupService(address, +port)).then(([hostname, service]) => ({ diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 6caa13d8f86e..c96b316117c3 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -4477,10 +4477,6 @@ Server.prototype.listen = function listen(this: $Node.NetServer, port, hostname, hostname = hostname || "::"; } - if (typeof port === "number" && (port < 0 || port >= 65536)) { - throw $ERR_SOCKET_BAD_PORT(`options.port should be >= 0 and < 65536. Received type number: (${port})`); - } - if (this._handle) { throw $ERR_SERVER_ALREADY_LISTEN(); } diff --git a/src/jsc/bindings/ErrorCode.cpp b/src/jsc/bindings/ErrorCode.cpp index ea2987dd468f..7b762c7f983b 100644 --- a/src/jsc/bindings/ErrorCode.cpp +++ b/src/jsc/bindings/ErrorCode.cpp @@ -776,13 +776,14 @@ void determineSpecificType(JSC::VM& vm, JSC::JSGlobalObject* globalObject, Messa if (constructor.toBoolean(globalObject)) { auto name = constructor.get(globalObject, vm.propertyNames->name); RETURN_IF_EXCEPTION(scope, void()); - auto str = name.toString(globalObject); - RETURN_IF_EXCEPTION(scope, void()); - builder.append("an instance of "_s); - auto view = str->view(globalObject); - RETURN_IF_EXCEPTION(scope, ); - builder.append(view); - return; + // A name that is not a string is not converted to one: https://github.com/nodejs/node/blob/v26.12.1/lib/internal/errors.js#L1042-L1045 + if (name.isString()) { + builder.append("an instance of "_s); + auto view = asString(name)->view(globalObject); + RETURN_IF_EXCEPTION(scope, ); + builder.append(view); + return; + } } } @@ -1571,17 +1572,23 @@ JSC::EncodedJSValue UNKNOWN_SIGNAL(JSC::ThrowScope& throwScope, JSC::JSGlobalObj return {}; } +// https://github.com/nodejs/node/blob/v26.12.1/lib/internal/errors.js#L1807-L1812. An undefined `name` is "Port", the default of validatePort. JSC::EncodedJSValue SOCKET_BAD_PORT(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, JSC::JSValue name, JSC::JSValue port, bool allowZero) { ASCIILiteral op = allowZero ? ">="_s : ">"_s; MessageBuilder builder; - JSValueToStringSafe(globalObject, builder, name); - RELEASE_RETURN_IF_EXCEPTION(throwScope, {}); + if (name.isUndefined()) { + builder.append("Port"_s); + } else { + JSValueToStringSafe(globalObject, builder, name); + RELEASE_RETURN_IF_EXCEPTION(throwScope, {}); + } builder.append(" should be "_s); builder.append(op); builder.append(" 0 and < 65536. Received "_s); - JSValueToStringSafe(globalObject, builder, port); + determineSpecificType(JSC::getVM(globalObject), globalObject, builder, port); RELEASE_RETURN_IF_EXCEPTION(throwScope, {}); + builder.append('.'); throwScope.throwException(globalObject, createError(globalObject, ErrorCode::ERR_SOCKET_BAD_PORT, builder)); throwScope.release(); diff --git a/src/jsc/bindings/NodeValidatorBodies.h b/src/jsc/bindings/NodeValidatorBodies.h index 73e6dca3c8b1..dc867cb21abd 100644 --- a/src/jsc/bindings/NodeValidatorBodies.h +++ b/src/jsc/bindings/NodeValidatorBodies.h @@ -157,7 +157,6 @@ template ALWAYS_INLINE JSC::EncodedJSValue validatePort(JSC: if (port.isInt32() && port.asInt32() <= 0xffff && port.asInt32() >= (allowZero.isUndefined() || allowZero.isTrue() ? 0 : 1)) return JSValue::encode(port); - if (name.isUndefined()) name = jsString(vm, String("Port"_s)); if (allowZero.isUndefined()) allowZero = jsBoolean(true); auto allowZero_b = allowZero.toBoolean(globalObject); diff --git a/test/js/node/__snapshots__/members-of-prototype-objects-for-any-receiver.test.ts.snap b/test/js/node/__snapshots__/members-of-prototype-objects-for-any-receiver.test.ts.snap index b1a9e912caa4..63556e2e8d1b 100644 --- a/test/js/node/__snapshots__/members-of-prototype-objects-for-any-receiver.test.ts.snap +++ b/test/js/node/__snapshots__/members-of-prototype-objects-for-any-receiver.test.ts.snap @@ -3642,9 +3642,9 @@ node:dgram.Socket.prototype bind("a", 1) throws TypeError: undefined is not an object <- every object, a number, a string, undefined throws TypeError: null is not an object <- null node:dgram.Socket.prototype connect() - throws RangeError ERR_SOCKET_BAD_PORT: Port should be > 0 and < 65536. Received undefined <- every receiver + throws RangeError ERR_SOCKET_BAD_PORT: Port should be > 0 and < 65536. Received undefined. <- every receiver node:dgram.Socket.prototype connect("a", 1) - throws RangeError ERR_SOCKET_BAD_PORT: Port should be > 0 and < 65536. Received a <- every receiver + throws RangeError ERR_SOCKET_BAD_PORT: Port should be > 0 and < 65536. Received type string ('a'). <- every receiver node:dgram.Socket.prototype disconnect() throws TypeError: undefined is not an object <- every object, a number, a string, undefined throws TypeError: null is not an object <- null diff --git a/test/js/node/errors/error-code-messages.test.ts b/test/js/node/errors/error-code-messages.test.ts index c376420b6a07..3da4ba36ce8b 100644 --- a/test/js/node/errors/error-code-messages.test.ts +++ b/test/js/node/errors/error-code-messages.test.ts @@ -1,6 +1,8 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir, tls as tlsCert } from "harness"; import child_process from "node:child_process"; +import dgram from "node:dgram"; +import dns from "node:dns"; import { EventEmitter, once } from "node:events"; import fs from "node:fs"; import http from "node:http"; @@ -309,3 +311,144 @@ test("ERR_IPC_CHANNEL_CLOSED message from process.send() in the child", async () expect(stdout).toBe("ERR_IPC_CHANNEL_CLOSED | Error | Channel closed\n"); expect(exitCode).toBe(0); }); + +// ERR_SOCKET_BAD_PORT. Node builds its message in one place, from the name, the bound and the rejected value: +// https://github.com/nodejs/node/blob/v26.12.1/lib/internal/errors.js#L1807-L1812 +// Every expected message below is the text node v26.12.1 gives for the same call. + +function withUdpSocket(fn: (socket: dgram.Socket) => T): T { + const socket = dgram.createSocket("udp4"); + try { + return fn(socket); + } finally { + socket.close(); + } +} + +test("ERR_SOCKET_BAD_PORT has node's message at every call that rejects a port", () => { + const badPort = (name: string, bound: string, received: string) => + `ERR_SOCKET_BAD_PORT | RangeError | ${name} should be ${bound} 0 and < 65536. Received ${received}.`; + + expect({ + "dns.lookupService": capture(() => dns.lookupService("127.0.0.1", 99999, () => {})), + "dns.lookupService, string": capture(() => dns.lookupService("127.0.0.1", "99999" as any, () => {})), + "dns.promises.lookupService": capture(() => dns.promises.lookupService("127.0.0.1", 99999)), + "net.connect": capture(() => net.connect({ host: "127.0.0.1", port: 99999 })), + "net.connect, string": capture(() => net.connect({ host: "127.0.0.1", port: "99999" as any })), + "Socket#connect": capture(() => new net.Socket().connect(99999, "127.0.0.1")), + "tls.connect": capture(() => tls.connect({ host: "127.0.0.1", port: 99999 })), + "http.request": capture(() => http.request({ host: "127.0.0.1", port: 99999 })), + "net.Server#listen": capture(() => net.createServer().listen(99999)), + "net.Server#listen, options": capture(() => net.createServer().listen({ port: 99999 })), + "net.Server#listen, options, string": capture(() => net.createServer().listen({ port: "99999" as any })), + "tls.Server#listen": capture(() => tls.createServer().listen(99999)), + "tls.Server#listen, options": capture(() => tls.createServer().listen({ port: 99999 })), + "dgram connect": capture(() => withUdpSocket(socket => socket.connect(99999))), + "dgram connect, zero": capture(() => withUdpSocket(socket => socket.connect(0))), + "dgram send": capture(() => withUdpSocket(socket => socket.send("x", 99999, "127.0.0.1"))), + "dgram send, string": capture(() => withUdpSocket(socket => socket.send("x", "0" as any, "127.0.0.1"))), + }).toEqual({ + "dns.lookupService": badPort("Port", ">=", "type number (99999)"), + "dns.lookupService, string": badPort("Port", ">=", "type string ('99999')"), + "dns.promises.lookupService": badPort("Port", ">=", "type number (99999)"), + "net.connect": badPort("Port", ">=", "type number (99999)"), + "net.connect, string": badPort("Port", ">=", "type string ('99999')"), + "Socket#connect": badPort("Port", ">=", "type number (99999)"), + "tls.connect": badPort("Port", ">=", "type number (99999)"), + "http.request": badPort("Port", ">=", "type number (99999)"), + "net.Server#listen": badPort("options.port", ">=", "type number (99999)"), + "net.Server#listen, options": badPort("options.port", ">=", "type number (99999)"), + "net.Server#listen, options, string": badPort("options.port", ">=", "type string ('99999')"), + "tls.Server#listen": badPort("options.port", ">=", "type number (99999)"), + "tls.Server#listen, options": badPort("options.port", ">=", "type number (99999)"), + "dgram connect": badPort("Port", ">", "type number (99999)"), + "dgram connect, zero": badPort("Port", ">", "type number (0)"), + "dgram send": badPort("Port", ">", "type number (99999)"), + "dgram send, string": badPort("Port", ">", "type string ('0')"), + }); +}); + +test("ERR_SOCKET_BAD_PORT describes the rejected port by its type", () => { + class Foo {} + const received = (port: unknown) => + capture(() => dns.lookupService("127.0.0.1", port as any, () => {})).replace( + "ERR_SOCKET_BAD_PORT | RangeError | Port should be >= 0 and < 65536. Received ", + "", + ); + expect( + [ + NaN, + Infinity, + 1.5, + "", + "it's", + "longer than twenty-eight characters", + undefined, + null, + true, + 10n, + Symbol("sym"), + [], + new Foo(), + function named() {}, + ].map(received), + ).toEqual([ + "type number (NaN).", + "type number (Infinity).", + "type number (1.5).", + "type string ('').", + `type string ("it's").`, + "type string ('longer than twenty-eight ...').", + "undefined.", + "null.", + "type boolean (true).", + "type bigint (10n).", + "type symbol (Symbol(sym)).", + "an instance of Array.", + "an instance of Foo.", + "function named.", + ]); +}); + +test("an error thrown while ERR_SOCKET_BAD_PORT describes the port is the error of the call", () => { + // The message names the constructor of an object. Node reads `port.constructor.name` for it. + const port: any = { + get constructor() { + throw new Error("from the getter"); + }, + }; + expect({ + "dns.lookupService": capture(() => dns.lookupService("127.0.0.1", port, () => {})), + "dgram connect": capture(() => withUdpSocket(socket => socket.connect(port))), + }).toEqual({ + "dns.lookupService": "undefined | Error | from the getter", + "dgram connect": "undefined | Error | from the getter", + }); +}); + +test("a port whose constructor name is not a string keeps ERR_SOCKET_BAD_PORT", () => { + // Node does not convert such a name to a string: https://github.com/nodejs/node/blob/v26.12.1/lib/internal/errors.js#L1042-L1045 + const ports: any[] = [ + JSON.parse('{"constructor":{"name":{"toString":null}}}'), + { constructor: { name: Symbol("name") } }, + { constructor: { name: 5 } }, + { constructor: {} }, + ]; + const codeAndClass = (fn: () => unknown) => capture(fn).split(" | ").slice(0, 2).join(" | "); + const kept = "ERR_SOCKET_BAD_PORT | RangeError"; + expect( + ports.map(port => ({ + "dns.lookupService": codeAndClass(() => dns.lookupService("127.0.0.1", port, () => {})), + "dns.promises.lookupService": codeAndClass(() => dns.promises.lookupService("127.0.0.1", port)), + "dgram connect": codeAndClass(() => withUdpSocket(socket => socket.connect(port))), + "dgram send": codeAndClass(() => withUdpSocket(socket => socket.send("x", port, "127.0.0.1"))), + })), + ).toEqual( + ports.map(() => ({ + "dns.lookupService": kept, + "dns.promises.lookupService": kept, + "dgram connect": kept, + "dgram send": kept, + })), + ); +}); diff --git a/test/js/node/errors/invalid-arg-type-received.test.ts b/test/js/node/errors/invalid-arg-type-received.test.ts index bedfbefdc91d..18642d481ff7 100644 --- a/test/js/node/errors/invalid-arg-type-received.test.ts +++ b/test/js/node/errors/invalid-arg-type-received.test.ts @@ -40,6 +40,16 @@ test.each([ expect(received(value)).toBe(expected); }); +// Node does not convert a constructor name to a string: https://github.com/nodejs/node/blob/v26.12.1/lib/internal/errors.js#L1042-L1045 +test.each([[JSON.parse('{"constructor":{"name":{"toString":null}}}')], [{ constructor: { name: Symbol("name") } }]])( + "a constructor name that is not a string does not replace the error %#", + value => { + expect(() => Buffer.compare(value as Buffer, Buffer.alloc(1))).toThrow( + expect.objectContaining({ name: "TypeError", code: "ERR_INVALID_ARG_TYPE" }), + ); + }, +); + test.each([ [function foo() {}, "function foo"], [Foo, "function Foo"],