node: drop the fmod call from the C++ integer validators

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);

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);

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 <cmath>
 #include <limits>
 
@@ -46,7 +47,7 @@ template<typename T> 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<typename T> 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;

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 <cmath>
 #include <limits>
@@ -202,9 +203,7 @@ template<typename Arguments> 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 (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);

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.

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");