From 08836f5a916d5e172f18637a9c0bc9e062b8b168 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Mon, 10 Feb 2025 19:53:58 -0500 Subject: [PATCH] make validation match node --- src/bun.js/api/bun/socket/SocketAddress.zig | 23 ++- src/bun.js/bindings/bindings.zig | 19 ++ src/js/internal/net/socket_address.ts | 21 +- test/js/node/net/socketaddress.spec.ts | 17 +- .../parallel/needs-test/test-socketaddress.js | 179 ++++++++++++++++++ 5 files changed, 244 insertions(+), 15 deletions(-) create mode 100644 test/js/node/test/parallel/needs-test/test-socketaddress.js diff --git a/src/bun.js/api/bun/socket/SocketAddress.zig b/src/bun.js/api/bun/socket/SocketAddress.zig index a97ed00aff0413..ec88d941b713af 100644 --- a/src/bun.js/api/bun/socket/SocketAddress.zig +++ b/src/bun.js/api/bun/socket/SocketAddress.zig @@ -41,16 +41,16 @@ pub const Options = struct { } else if (bun.strings.eqlComptime(slice.slice(), "ipv6")) { break :blk AF.INET6; } else { - return global.throwInvalidArgumentTypeValue("options.family", "ipv4 or ipv6", fam); + return global.throwInvalidArgumentPropertyValue("options.family", "'ipv4' or 'ipv6'", fam); } } else if (fam.isUInt32AsAnyInt()) { break :blk switch (fam.toU32()) { AF.INET.int() => AF.INET, AF.INET6.int() => AF.INET6, - else => return global.throwInvalidArgumentTypeValue("options.family", "AF_INET or AF_INET6", fam), + else => return global.throwInvalidArgumentPropertyValue("options.family", "AF_INET or AF_INET6", fam), }; } else { - return global.throwInvalidArgumentTypeValue("options.family", "string or number", fam); + return global.throwInvalidArgumentTypeValue("options.family", "a string or number", fam); } } else AF.INET; @@ -99,6 +99,15 @@ pub fn constructor(global: *JSC.JSGlobalObject, frame: *JSC.CallFrame) bun.JSErr if (!options_obj.isObject()) return global.throwInvalidArgumentTypeValue("options", "object", options_obj); const options = try Options.fromJS(global, options_obj); + + // fast path for { family: 'ipv6' } + if (options.family == AF.INET6 and options.address == null and options.flowlabel == null and options.port == 0) { + return SocketAddress.new(.{ + ._addr = sockaddr.@"::", + ._presentation = WellKnownAddress.@"::", + }); + } + return SocketAddress.create(global, options); } @@ -106,7 +115,7 @@ pub fn constructor(global: *JSC.JSGlobalObject, frame: *JSC.CallFrame) bun.JSErr pub fn create(global: *JSC.JSGlobalObject, options: Options) bun.JSError!*SocketAddress { const presentation: bun.String = options.address orelse switch (options.family) { AF.INET => WellKnownAddress.@"127.0.0.1", - AF.INET6 => WellKnownAddress.@"::1", + AF.INET6 => WellKnownAddress.@"::", }; // We need a zero-terminated cstring for `ares_inet_pton`, which forces us to // copy the string. @@ -146,7 +155,7 @@ pub fn create(global: *JSC.JSGlobalObject, options: Options) bun.JSError!*Socket defer alloc.free(slice); try pton(global, C.AF_INET6, slice, &sin6.sin6_addr); } else { - sin6.sin6_addr = C.in6addr_loopback; + sin6.sin6_addr = C.in6addr_any; } break :v6 .{ .sin6 = sin6 }; }, @@ -374,10 +383,14 @@ const sockaddr = extern union { pub const @"127.0.0.1": sockaddr = sockaddr.v4(0, .{ .s_addr = C.INADDR_LOOPBACK }); pub const @"::1": sockaddr = sockaddr.v6(0, C.in6addr_loopback); + // TODO: check that `::` is all zeroes on all platforms. Should correspond + // to `IN6ADDR_ANY_INIT`. + pub const @"::": sockaddr = sockaddr.v6(0, std.mem.zeroes(C.struct_in6_addr), 0, 0); }; const WellKnownAddress = struct { const @"127.0.0.1": bun.String = bun.String.static("127.0.0.1"); + const @"::": bun.String = bun.String.static("::"); const @"::1": bun.String = bun.String.static("::1"); }; diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index 20663b35072296..2c5c4f7fe9bb86 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -3002,6 +3002,25 @@ pub const JSGlobalObject = opaque { return this.ERR_INVALID_ARG_VALUE("The \"{s}\" argument is invalid. Received {}", .{ argname, actual_string_value }).throw(); } + /// Throw an `ERR_INVALID_ARG_VALUE` when the invalid value is a property of an object. + /// Message depends on whether `expected` is present. + /// - "The property "{argname}" is invalid. Received {value}" + /// - "The property "{argname}" is invalid. Expected {expected}, received {value}" + pub fn throwInvalidArgumentPropertyValue( + this: *JSGlobalObject, + argname: []const u8, + comptime expected: ?[]const u8, + value: JSValue, + ) bun.JSError { + const actual_string_value = try determineSpecificType(this, value); + defer actual_string_value.deref(); + if (comptime expected) |_expected| { + return this.ERR_INVALID_ARG_VALUE("The property \"{s}\" is invalid. Expected {s}, received {}", .{ argname, _expected, actual_string_value }).throw(); + } else { + return this.ERR_INVALID_ARG_VALUE("The property \"{s}\" is invalid. Received {}", .{ argname, actual_string_value }).throw(); + } + } + extern "c" fn Bun__ErrorCode__determineSpecificType(*JSGlobalObject, JSValue) String; pub fn determineSpecificType(global: *JSGlobalObject, value: JSValue) JSError!String { diff --git a/src/js/internal/net/socket_address.ts b/src/js/internal/net/socket_address.ts index e8a18e1f530a01..ea580797b368cf 100644 --- a/src/js/internal/net/socket_address.ts +++ b/src/js/internal/net/socket_address.ts @@ -1,6 +1,6 @@ const { SocketAddressNative, AF_INET } = require("../net"); import type { SocketAddressInitOptions } from "node:net"; -const { validateObject, validatePort, validateString } = require("internal/validators"); +const { validateObject, validatePort, validateString, validateUint32 } = require("internal/validators"); const kHandle = Symbol("kHandle"); @@ -35,7 +35,24 @@ class SocketAddress { this[kHandle] = new SocketAddressNative(); } else { validateObject(options, "options"); - if (options.port !== undefined) validatePort(options.port, "options.port"); + let { address, port, flowlabel, family = "ipv4" } = options; + if (port !== undefined) validatePort(port, "options.port"); + if (address !== undefined) validateString(address, "options.address"); + if (flowlabel !== undefined) validateUint32(flowlabel, "options.flowlabel"); + // Bun's native SocketAddress allows `family` to be `AF_INET` or `AF_INET6`, + // but since we're aiming for nodejs compat in node:net this is not allowed. + if (typeof family?.toLowerCase === "function") { + options.family = family = family.toLowerCase(); + } + + switch (family) { + case "ipv4": + case "ipv6": + break; + default: + throw $ERR_INVALID_ARG_VALUE("options.family", options.family); + } + this[kHandle] = new SocketAddressNative(options); } } diff --git a/test/js/node/net/socketaddress.spec.ts b/test/js/node/net/socketaddress.spec.ts index e1d910a12b5b4c..2fb34124c53b09 100644 --- a/test/js/node/net/socketaddress.spec.ts +++ b/test/js/node/net/socketaddress.spec.ts @@ -17,6 +17,7 @@ describe("SocketAddress", () => { // @ts-expect-error -- types are wrong. expect(() => SocketAddress()).toThrow(TypeError); }); + describe.each([new SocketAddress(), new SocketAddress(undefined), new SocketAddress({})])( "new SocketAddress()", address => { @@ -43,9 +44,9 @@ describe("SocketAddress", () => { beforeAll(() => { address = new SocketAddress({ family: "ipv6" }); }); - it("creates a new ipv6 loopback address", () => { + it("creates a new ipv6 any address", () => { expect(address).toMatchObject({ - address: "::1", + address: "::", port: 0, family: "ipv6", flowlabel: 0, @@ -70,7 +71,7 @@ describe("SocketAddress.isSocketAddress", () => { configurable: true, }); }); -}); +}); // describe("SocketAddress.parse", () => { it("is a function that takes 1 argument", () => { @@ -114,7 +115,7 @@ describe("SocketAddress.parse", () => { ])("(%s) == undefined", invalidInput => { expect(SocketAddress.parse(invalidInput)).toBeUndefined(); }); -}); +}); // describe("SocketAddress.prototype.address", () => { it("has the correct property descriptor", () => { @@ -126,7 +127,7 @@ describe("SocketAddress.prototype.address", () => { configurable: true, }); }); -}); +}); // describe("SocketAddress.prototype.port", () => { it("has the correct property descriptor", () => { @@ -138,7 +139,7 @@ describe("SocketAddress.prototype.port", () => { configurable: true, }); }); -}); +}); // describe("SocketAddress.prototype.family", () => { it("has the correct property descriptor", () => { @@ -150,7 +151,7 @@ describe("SocketAddress.prototype.family", () => { configurable: true, }); }); -}); +}); // describe("SocketAddress.prototype.flowlabel", () => { it("has the correct property descriptor", () => { @@ -162,7 +163,7 @@ describe("SocketAddress.prototype.flowlabel", () => { configurable: true, }); }); -}); +}); // describe("SocketAddress.prototype.toJSON", () => { it("is a function that takes 0 arguments", () => { diff --git a/test/js/node/test/parallel/needs-test/test-socketaddress.js b/test/js/node/test/parallel/needs-test/test-socketaddress.js new file mode 100644 index 00000000000000..2b59d469457361 --- /dev/null +++ b/test/js/node/test/parallel/needs-test/test-socketaddress.js @@ -0,0 +1,179 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../../common'); +const { + ok, + strictEqual, + throws, +} = require('assert'); +const { + SocketAddress, +} = require('net'); + +// NOTE: we don't check node's internals. +// const { +// InternalSocketAddress, +// } = require('internal/socketaddress'); +// const { internalBinding } = require('internal/test/binding'); +// const { +// SocketAddress: _SocketAddress, +// AF_INET, +// } = internalBinding('block_list'); + +const { describe, it } = require('node:test'); + +describe('net.SocketAddress...', () => { + + it('is cloneable', () => { + const sa = new SocketAddress(); + strictEqual(sa.address, '127.0.0.1'); + strictEqual(sa.port, 0); + strictEqual(sa.family, 'ipv4'); + strictEqual(sa.flowlabel, 0); + + // NOTE: bun does not support `kClone` yet. + // const mc = new MessageChannel(); + // mc.port1.onmessage = common.mustCall(({ data }) => { + // ok(SocketAddress.isSocketAddress(data)); + + // strictEqual(data.address, '127.0.0.1'); + // strictEqual(data.port, 0); + // strictEqual(data.family, 'ipv4'); + // strictEqual(data.flowlabel, 0); + + // mc.port1.close(); + // }); + // mc.port2.postMessage(sa); + }); + + it('has reasonable defaults', () => { + const sa = new SocketAddress({}); + strictEqual(sa.address, '127.0.0.1'); + strictEqual(sa.port, 0); + strictEqual(sa.family, 'ipv4'); + strictEqual(sa.flowlabel, 0); + }); + + it('interprets simple ipv4 correctly', () => { + const sa = new SocketAddress({ + address: '123.123.123.123', + }); + strictEqual(sa.address, '123.123.123.123'); + strictEqual(sa.port, 0); + strictEqual(sa.family, 'ipv4'); + strictEqual(sa.flowlabel, 0); + }); + + it('sets the port correctly', () => { + const sa = new SocketAddress({ + address: '123.123.123.123', + port: 80 + }); + strictEqual(sa.address, '123.123.123.123'); + strictEqual(sa.port, 80); + strictEqual(sa.family, 'ipv4'); + strictEqual(sa.flowlabel, 0); + }); + + it('interprets simple ipv6 correctly', () => { + const sa = new SocketAddress({ + family: 'ipv6' + }); + strictEqual(sa.address, '::'); + strictEqual(sa.port, 0); + strictEqual(sa.family, 'ipv6'); + strictEqual(sa.flowlabel, 0); + }); + + it('uses the flowlabel correctly', () => { + const sa = new SocketAddress({ + family: 'ipv6', + flowlabel: 1, + }); + strictEqual(sa.address, '::'); + strictEqual(sa.port, 0); + strictEqual(sa.family, 'ipv6'); + strictEqual(sa.flowlabel, 1); + }); + + it('validates input correctly', () => { + [1, false, 'hello'].forEach((i) => { + throws(() => new SocketAddress(i), { + code: 'ERR_INVALID_ARG_TYPE' + }); + }); + + [1, false, {}, [], 'test'].forEach((family) => { + throws(() => new SocketAddress({ family }), { + code: 'ERR_INVALID_ARG_VALUE' + }); + }); + + [1, false, {}, []].forEach((address) => { + throws(() => new SocketAddress({ address }), { + code: 'ERR_INVALID_ARG_TYPE' + }); + }); + + [-1, false, {}, []].forEach((port) => { + throws(() => new SocketAddress({ port }), { + code: 'ERR_SOCKET_BAD_PORT' + }); + }); + + throws(() => new SocketAddress({ flowlabel: -1 }), { + code: 'ERR_OUT_OF_RANGE' + }); + }); + + // NOTE: we don't check node's internals. + // it('InternalSocketAddress correctly inherits from SocketAddress', () => { + // // Test that the internal helper class InternalSocketAddress correctly + // // inherits from SocketAddress and that it does not throw when its properties + // // are accessed. + + // const address = '127.0.0.1'; + // const port = 8080; + // const flowlabel = 0; + // const handle = new _SocketAddress(address, port, AF_INET, flowlabel); + // const addr = new InternalSocketAddress(handle); + // ok(addr instanceof SocketAddress); + // strictEqual(addr.address, address); + // strictEqual(addr.port, port); + // strictEqual(addr.family, 'ipv4'); + // strictEqual(addr.flowlabel, flowlabel); + // }); + + it('SocketAddress.parse() works as expected', () => { + const good = [ + { input: '1.2.3.4', address: '1.2.3.4', port: 0, family: 'ipv4' }, + { input: '192.168.257:1', address: '192.168.1.1', port: 1, family: 'ipv4' }, + { input: '256', address: '0.0.1.0', port: 0, family: 'ipv4' }, + { input: '999999999:12', address: '59.154.201.255', port: 12, family: 'ipv4' }, + { input: '0xffffffff', address: '255.255.255.255', port: 0, family: 'ipv4' }, + { input: '0x.0x.0', address: '0.0.0.0', port: 0, family: 'ipv4' }, + { input: '[1:0::]', address: '1::', port: 0, family: 'ipv6' }, + { input: '[1::8]:123', address: '1::8', port: 123, family: 'ipv6' }, + ]; + + good.forEach((i) => { + const addr = SocketAddress.parse(i.input); + strictEqual(addr.address, i.address); + strictEqual(addr.port, i.port); + strictEqual(addr.family, i.family); + }); + + const bad = [ + 'not an ip', + 'abc.123', + '259.1.1.1', + '12:12:12', + ]; + + bad.forEach((i) => { + strictEqual(SocketAddress.parse(i), undefined); + }); + }); + +});