Skip to content

Commit

Permalink
make validation match node
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Feb 11, 2025
1 parent f08c06b commit 08836f5
Show file tree
Hide file tree
Showing 5 changed files with 244 additions and 15 deletions.
23 changes: 18 additions & 5 deletions src/bun.js/api/bun/socket/SocketAddress.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -99,14 +99,23 @@ 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);
}

/// If you have raw socket address data, prefer `SocketAddress.new`.
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.
Expand Down Expand Up @@ -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 };
},
Expand Down Expand Up @@ -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");
};
Expand Down
19 changes: 19 additions & 0 deletions src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 19 additions & 2 deletions src/js/internal/net/socket_address.ts
Original file line number Diff line number Diff line change
@@ -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");

Expand Down Expand Up @@ -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);
}
}
Expand Down
17 changes: 9 additions & 8 deletions test/js/node/net/socketaddress.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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,
Expand All @@ -70,7 +71,7 @@ describe("SocketAddress.isSocketAddress", () => {
configurable: true,
});
});
});
}); // </SocketAddress.isSocketAddress>

describe("SocketAddress.parse", () => {
it("is a function that takes 1 argument", () => {
Expand Down Expand Up @@ -114,7 +115,7 @@ describe("SocketAddress.parse", () => {
])("(%s) == undefined", invalidInput => {
expect(SocketAddress.parse(invalidInput)).toBeUndefined();
});
});
}); // </SocketAddress.parse>

describe("SocketAddress.prototype.address", () => {
it("has the correct property descriptor", () => {
Expand All @@ -126,7 +127,7 @@ describe("SocketAddress.prototype.address", () => {
configurable: true,
});
});
});
}); // </SocketAddress.prototype.address>

describe("SocketAddress.prototype.port", () => {
it("has the correct property descriptor", () => {
Expand All @@ -138,7 +139,7 @@ describe("SocketAddress.prototype.port", () => {
configurable: true,
});
});
});
}); // </SocketAddress.prototype.port>

describe("SocketAddress.prototype.family", () => {
it("has the correct property descriptor", () => {
Expand All @@ -150,7 +151,7 @@ describe("SocketAddress.prototype.family", () => {
configurable: true,
});
});
});
}); // </SocketAddress.prototype.family>

describe("SocketAddress.prototype.flowlabel", () => {
it("has the correct property descriptor", () => {
Expand All @@ -162,7 +163,7 @@ describe("SocketAddress.prototype.flowlabel", () => {
configurable: true,
});
});
});
}); // </SocketAddress.prototype.flowlabel>

describe("SocketAddress.prototype.toJSON", () => {
it("is a function that takes 0 arguments", () => {
Expand Down
179 changes: 179 additions & 0 deletions test/js/node/test/parallel/needs-test/test-socketaddress.js
Original file line number Diff line number Diff line change
@@ -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);
});
});

});

0 comments on commit 08836f5

Please sign in to comment.