From d6e4f3e00c154b55f9260c25b508f1759024ad00 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:16:59 +0000 Subject: [PATCH 1/2] node: drop the fmod call from the C++ integer validators validateInteger, validateInt32, validateUint32 and Buffer's validateOffset tested a number for integrality with std::fmod(x, 1.0) != 0. On linux-gnu and windows-msvc that is a library call for each validated number. On linux-gnu the call lands in Rust's compiler_builtins fmod, which loops once per exponent bit below 2**32. Buffer#fill with offsets (since 1.4.0) and zlib.crc32 with a seed at or above 2**31 (since 1.4.4) pay for it. JSC::isInteger(x) is x - trunc(x) == 0.0. It is the same predicate for every double, NaN and the infinities included, and it compiles to one roundsd (x64) or frintz (arm64) with no call. validatePort gets the same spelling. clang already emitted the inline form there. No behaviour change. The new test cases pin the non-integer results of Buffer#write and of validateInteger, validateInt32 and validateUint32. They pass before and after. --- bench/snippets/validate-integer.mjs | 143 +++++++++++++++++++++++++ src/jsc/bindings/JSBuffer.cpp | 2 +- src/jsc/bindings/NodeValidator.cpp | 13 +-- src/jsc/bindings/NodeValidatorBodies.h | 3 +- test/js/node/buffer.test.js | 28 +++++ test/js/node/util/util.test.js | 62 +++++++++++ 6 files changed, 243 insertions(+), 8 deletions(-) create mode 100644 bench/snippets/validate-integer.mjs diff --git a/bench/snippets/validate-integer.mjs b/bench/snippets/validate-integer.mjs new file mode 100644 index 000000000000..47c1024eca6f --- /dev/null +++ b/bench/snippets/validate-integer.mjs @@ -0,0 +1,143 @@ +// Integer arguments of node:* APIs. The native validators behind these calls +// (validateInteger in NodeValidator.cpp, validateOffset in JSBuffer.cpp) test each +// numeric argument for "is an integer" before they use it. +// +// Every API runs over the input classes of that test: +// - an int32, +// - the same integer as a double (read from a Float64Array, so native code gets a double), +// - a value the test rejects (the call throws). +import { getDefaultHighWaterMark, setDefaultHighWaterMark } from "node:stream"; +import { crc32 } from "node:zlib"; +import { bench, group, run } from "../runner.mjs"; + +// One iteration makes this many calls. Divide the reported time by it for one call. +const calls = 16; + +const size = 16 * 1024 * 1024; +const a = Buffer.alloc(size, 1); +const b = Buffer.alloc(size, 1); +const chunk = Buffer.alloc(8, 1); + +// The buffer calls touch one byte, so only the magnitude of the offsets changes. +const offsets = [0, 4096, 65536, 12345678]; + +// Each API has its own loops, so that every call site sees one callee and one type of `n`. +const apis = [ + { + // Two validateInteger calls. + name: "buf.fill(1, n, n + 1)", + int32s: offsets, + doubles: offsets, + int32: n => () => { + for (let i = 0; i < calls; i++) a.fill(1, n, n + 1); + }, + double: (values, j) => () => { + for (let i = 0; i < calls; i++) a.fill(1, values[j], values[j] + 1); + }, + rejected: n => () => { + for (let i = 0; i < calls; i++) { + try { + a.fill(1, n, n + 1); + } catch {} + } + }, + }, + { + // Two validateOffset calls. + name: "buf.write('x', n, 1)", + int32s: offsets, + doubles: offsets, + int32: n => () => { + for (let i = 0; i < calls; i++) a.write("x", n, 1); + }, + double: (values, j) => () => { + for (let i = 0; i < calls; i++) a.write("x", values[j], 1); + }, + rejected: n => () => { + for (let i = 0; i < calls; i++) { + try { + a.write("x", n, 1); + } catch {} + } + }, + }, + { + // An int32 seed converts inline. A running CRC at or above 2**31 is a double + // and takes one validateInteger call. + name: "zlib.crc32(chunk, n)", + int32s: [...offsets, 2 ** 31 - 1], + doubles: [...offsets, 2 ** 31 - 1, 2 ** 31, 0xdeadbeef, 2 ** 32 - 1], + int32: n => () => { + for (let i = 0; i < calls; i++) crc32(chunk, n); + }, + double: (values, j) => () => { + for (let i = 0; i < calls; i++) crc32(chunk, values[j]); + }, + rejected: n => () => { + for (let i = 0; i < calls; i++) { + try { + crc32(chunk, n); + } catch {} + } + }, + }, + { + // Four validateInteger calls. + name: "buf.compare(target, n, n + 1, n, n + 1)", + int32s: offsets, + doubles: offsets, + int32: n => () => { + for (let i = 0; i < calls; i++) a.compare(b, n, n + 1, n, n + 1); + }, + double: (values, j) => () => { + for (let i = 0; i < calls; i++) a.compare(b, values[j], values[j] + 1, values[j], values[j] + 1); + }, + rejected: n => () => { + for (let i = 0; i < calls; i++) { + try { + a.compare(b, n, n + 1, n, n + 1); + } catch {} + } + }, + }, + { + // One validateInteger call. It also accepts integers past the uint32 range. + name: "stream.setDefaultHighWaterMark(false, n)", + int32s: [...offsets, 2 ** 31 - 1], + doubles: [...offsets, 2 ** 31 - 1, 2 ** 32 - 1, 2 ** 32, 2 ** 53 - 1], + int32: n => () => { + for (let i = 0; i < calls; i++) setDefaultHighWaterMark(false, n); + }, + double: (values, j) => () => { + for (let i = 0; i < calls; i++) setDefaultHighWaterMark(false, values[j]); + }, + rejected: n => () => { + for (let i = 0; i < calls; i++) { + try { + setDefaultHighWaterMark(false, n); + } catch {} + } + }, + }, +]; + +const savedHighWaterMark = getDefaultHighWaterMark(false); + +for (const api of apis) { + group(`${api.name}, n is an int32`, () => { + for (const n of api.int32s) bench(`n = ${n}`, api.int32(n)); + }); + + group(`${api.name}, n is a double`, () => { + const values = new Float64Array(api.doubles); + for (let j = 0; j < values.length; j++) bench(`n = ${values[j]}`, api.double(values, j)); + }); + + group(`${api.name}, n is rejected`, () => { + for (const n of [1.5, NaN, Infinity]) bench(`n = ${n}`, api.rejected(n)); + }); +} + +await run(); + +setDefaultHighWaterMark(false, savedHighWaterMark); diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 31eaa085c12a..6f3459146538 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -367,7 +367,7 @@ size_t validateOffset(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, if (!value.isNumber()) [[unlikely]] return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "number"_s, value); auto value_num = value.asNumber(); - if (std::fmod(value_num, 1.0) != 0) [[unlikely]] + if (!JSC::isInteger(value_num)) [[unlikely]] return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); if (value_num < min || value_num > max) [[unlikely]] return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, makeString(">= "_s, min, " && <= "_s, max), value); diff --git a/src/jsc/bindings/NodeValidator.cpp b/src/jsc/bindings/NodeValidator.cpp index 9574a5238c51..385411c4d0ea 100644 --- a/src/jsc/bindings/NodeValidator.cpp +++ b/src/jsc/bindings/NodeValidator.cpp @@ -9,6 +9,7 @@ #include "JavaScriptCore/JSType.h" #include "JavaScriptCore/TypedArrayType.h" #include "JavaScriptCore/ArrayConstructor.h" +#include "JavaScriptCore/MathCommon.h" #include #include @@ -46,7 +47,7 @@ template JSC::EncodedJSValue V::validateInteger(JSC::ThrowScope& sco auto max_num = max.toNumber(globalObject); RETURN_IF_EXCEPTION(scope, {}); - if (std::fmod(value_num, 1.0) != 0) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); + if (!JSC::isInteger(value_num)) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); if (value_num < min_num || value_num > max_num) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, conjunctiveRange(min_num, max_num), value); *out = value_num; @@ -64,7 +65,7 @@ template JSC::EncodedJSValue V::validateInteger(JSC::ThrowScope& sco auto max_num = max.toNumber(globalObject); RETURN_IF_EXCEPTION(scope, {}); - if (std::fmod(value_num, 1.0) != 0) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); + if (!JSC::isInteger(value_num)) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); if (value_num < min_num || value_num > max_num) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, conjunctiveRange(min_num, max_num), value); *out = value_num; @@ -223,7 +224,7 @@ EncodedJSValue V::validateInt32(ThrowScope& scope, JSC::JSGlobalObject* globalOb auto max_num = max.toNumber(globalObject); RETURN_IF_EXCEPTION(scope, {}); - if (std::fmod(value_num, 1.0) != 0) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); + if (!JSC::isInteger(value_num)) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); if (value_num < min_num || value_num > max_num) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, conjunctiveRange(min_num, max_num), value); return JSValue::encode(jsUndefined()); @@ -249,7 +250,7 @@ EncodedJSValue V::validateInt32(ThrowScope& scope, JSC::JSGlobalObject* globalOb auto max_num = max.toNumber(globalObject); RETURN_IF_EXCEPTION(scope, {}); - if (std::fmod(value_num, 1.0) != 0) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); + if (!JSC::isInteger(value_num)) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); if (value_num < min_num || value_num > max_num) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, conjunctiveRange(min_num, max_num), value); if (out) { @@ -265,7 +266,7 @@ JSC::EncodedJSValue V::validateUint32(JSC::ThrowScope& scope, JSC::JSGlobalObjec if (positive.isUndefined()) positive = jsBoolean(false); auto value_num = value.asNumber(); - if (std::fmod(value_num, 1.0) != 0) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); + if (!JSC::isInteger(value_num)) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); auto positive_b = positive.toBoolean(globalObject); auto min = positive_b ? 1 : 0; @@ -284,7 +285,7 @@ JSC::EncodedJSValue V::validateUint32(JSC::ThrowScope& scope, JSC::JSGlobalObjec if (positive.isUndefined()) positive = jsBoolean(false); auto value_num = value.asNumber(); - if (std::fmod(value_num, 1.0) != 0) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); + if (!JSC::isInteger(value_num)) return Bun::ERR::OUT_OF_RANGE(scope, globalObject, name, "an integer"_s, value); auto positive_b = positive.toBoolean(globalObject); auto min = positive_b ? 1 : 0; diff --git a/src/jsc/bindings/NodeValidatorBodies.h b/src/jsc/bindings/NodeValidatorBodies.h index 73e6dca3c8b1..d6c62a54a9ca 100644 --- a/src/jsc/bindings/NodeValidatorBodies.h +++ b/src/jsc/bindings/NodeValidatorBodies.h @@ -7,6 +7,7 @@ #include "JavaScriptCore/ArrayConstructor.h" #include "JavaScriptCore/JSArrayBufferView.h" #include "JavaScriptCore/JSObjectInlines.h" +#include "JavaScriptCore/MathCommon.h" #include "JavaScriptCore/TypedArrayType.h" #include #include @@ -204,7 +205,7 @@ template ALWAYS_INLINE JSC::EncodedJSValue validatePort(JSC: if (std::isnan(port_num)) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (std::isinf(port_num)) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); - if (std::fmod(port_num, 1.0) != 0) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); + if (!JSC::isInteger(port_num)) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (port_num < 0) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (port_num > 0xffff) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (port_num == 0 && !allowZero_b) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index b2439f486d4e..bd4f6f0ea740 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -4233,6 +4233,34 @@ describe("Buffer.fill offset/end argument handling", () => { }); }); +it("write() with a non-integer offset or length throws ERR_OUT_OF_RANGE", () => { + // Node routes offset and length through validateOffset (= validateInteger). + const outOfRange = (name, expected, received) => + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: `The value of "${name}" is out of range. It must be ${expected}. Received ${received}`, + }); + for (const value of [1.5, -1.5, NaN, Infinity, -Infinity]) { + expect(() => Buffer.alloc(8).write("abc", value)).toThrow(outOfRange("offset", "an integer", value)); + expect(() => Buffer.alloc(8).write("abc", 0, value)).toThrow(outOfRange("length", "an integer", value)); + expect(() => Buffer.alloc(8).write("abc", value, "utf8")).toThrow(outOfRange("offset", "an integer", value)); + } + + // -0 and integral doubles are integers. + const buf = Buffer.alloc(8, "x"); + expect(buf.write("abc", -0)).toBe(3); + expect(buf.write("abc", 10 / 2, 6 / 2)).toBe(3); + expect(buf.toString()).toBe("abcxxabc"); + + // An integral double past the buffer still gets the range message. + expect(() => Buffer.alloc(8).write("abc", 2 ** 53)).toThrow( + outOfRange("offset", ">= 0 && <= 8", "9_007_199_254_740_992"), + ); + expect(() => Buffer.alloc(8).write("abc", 0, 2 ** 53)).toThrow( + outOfRange("length", ">= 0 && <= 8", "9_007_199_254_740_992"), + ); +}); + describe("*Write methods with NaN/invalid offset and length", () => { // Regression test: NaN offset/length values must be handled safely. // NaN offset should be treated as 0, and length should be clamped to buffer size. diff --git a/test/js/node/util/util.test.js b/test/js/node/util/util.test.js index 1fbe9c5c5275..30c3fa021e0b 100644 --- a/test/js/node/util/util.test.js +++ b/test/js/node/util/util.test.js @@ -451,6 +451,68 @@ describe("util", () => { invalidArgType("The \"foo\" argument must be of type object. Received type string ('string')"), ); }); + // Ported from the validateInteger block of node's test/parallel/test-validators.js (v26.3.0). + it("validateInteger, validateInt32 and validateUint32 accept and reject like Node", () => { + const { validateInteger, validateInt32, validateUint32 } = exposedInternals["internal/validators"]; + const { MAX_SAFE_INTEGER, MIN_SAFE_INTEGER } = Number; + const outOfRange = expect.objectContaining({ code: "ERR_OUT_OF_RANGE", name: "RangeError" }); + const invalidType = expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" }); + + // validateInteger() defaults to validating safe integers. + validateInteger(MAX_SAFE_INTEGER, "foo"); + validateInteger(MIN_SAFE_INTEGER, "foo"); + expect(() => validateInteger(MAX_SAFE_INTEGER + 1, "foo")).toThrow(outOfRange); + expect(() => validateInteger(MIN_SAFE_INTEGER - 1, "foo")).toThrow(outOfRange); + + // validateInteger() works with unsafe integers. + validateInteger(MAX_SAFE_INTEGER + 1, "foo", 0, MAX_SAFE_INTEGER + 1); + validateInteger(MIN_SAFE_INTEGER - 1, "foo", MIN_SAFE_INTEGER - 1); + + // validateInt32() and validateUint32() + for (const val of [Symbol(), 1n, {}, [], false, true, undefined, null, () => {}, "", "1"]) { + expect(() => validateInt32(val, "name")).toThrow(invalidType); + expect(() => validateUint32(val, "name")).toThrow(invalidType); + } + for (const val of [2147483647 + 1, -2147483648 - 1, NaN]) { + expect(() => validateInt32(val, "name")).toThrow(outOfRange); + } + for (const val of [0, 1, -1]) validateInt32(val, "name"); + for (const val of [4294967296, -1, NaN]) { + expect(() => validateUint32(val, "name")).toThrow(outOfRange); + } + for (const val of [0, 1]) validateUint32(val, "name"); + + // Node's file stops here. A non-integer gets the "an integer" message before any range check. + const notAnInteger = val => + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: `The value of "name" is out of range. It must be an integer. Received ${val}`, + }); + for (const validate of [validateInteger, validateInt32, validateUint32]) { + for (const val of [1.5, -1.5, NaN, Infinity, -Infinity]) { + expect(() => validate(val, "name")).toThrow(notAnInteger(val)); + } + // -0 and integral doubles are integers. + validate(-0, "name"); + validate(10 / 2, "name"); + } + expect(() => validateInteger(1.5, "name", 0, 10)).toThrow(notAnInteger(1.5)); + expect(() => validateInt32(1.5, "name", 0, 10)).toThrow(notAnInteger(1.5)); + expect(() => validateUint32(1.5, "name", true)).toThrow(notAnInteger(1.5)); + validateInteger(5, "name", 0, 10); + validateInt32(5, "name", 0, 10); + validateUint32(5, "name", true); + + // An integral double out of range gets the range message. + const range = expected => + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: `The value of "name" is out of range. It must be ${expected}. Received 9_007_199_254_740_992`, + }); + expect(() => validateInteger(2 ** 53, "name")).toThrow(range(">= -9007199254740991 && <= 9007199254740991")); + expect(() => validateInt32(2 ** 53, "name")).toThrow(range(">= -2147483648 && <= 2147483647")); + expect(() => validateUint32(2 ** 53, "name")).toThrow(range(">= 0 && <= 4294967295")); + }); it("formatWithOptions", () => { expect(util.formatWithOptions({ colors: true }, "%s:%s", "foo")).toBe("foo:%s"); From 4817fab6e0aee4f08a9ffd20de6b3cedfa3e5c2e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 28 Oct 2026 06:22:02 +0000 Subject: [PATCH 2/2] node: delete validatePort's isnan and isinf returns JSC::isInteger is false for NaN and for both infinities, and all three returns raised the same ERR_SOCKET_BAD_PORT. --- src/jsc/bindings/NodeValidatorBodies.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/jsc/bindings/NodeValidatorBodies.h b/src/jsc/bindings/NodeValidatorBodies.h index d6c62a54a9ca..82c11e911155 100644 --- a/src/jsc/bindings/NodeValidatorBodies.h +++ b/src/jsc/bindings/NodeValidatorBodies.h @@ -203,8 +203,6 @@ template ALWAYS_INLINE JSC::EncodedJSValue validatePort(JSC: auto port_num = port.toNumber(globalObject); RETURN_IF_EXCEPTION(scope, {}); - if (std::isnan(port_num)) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); - if (std::isinf(port_num)) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (!JSC::isInteger(port_num)) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (port_num < 0) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b); if (port_num > 0xffff) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b);