Cleanup fs.utimesSync (#3363)

* Fix UB in fs.utimesSync when passing a number with an integer greater than i32

* Fix make headers

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

7 files changed, 147 insertions, 22 deletions

diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp
index 74357f225..2ec1bd902 100644
--- a/src/bun.js/bindings/bindings.cpp
+++ b/src/bun.js/bindings/bindings.cpp
@@ -3111,6 +3111,19 @@ int32_t JSC__JSValue__toInt32(JSC__JSValue JSValue0)
     return JSC::JSValue::decode(JSValue0).asInt32();
 }
 
+CPP_DECL double JSC__JSValue__coerceToDouble(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1)
+{
+    JSC::JSValue value = JSC::JSValue::decode(JSValue0);
+    auto catchScope = DECLARE_CATCH_SCOPE(arg1->vm());
+    double result = value.toNumber(arg1);
+    if (catchScope.exception()) {
+        result = PNaN;
+        catchScope.clearException();
+    }
+
+    return result;
+}
+
 // truncates values larger than int32
 int32_t JSC__JSValue__coerceToInt32(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1)
 {
@@ -4374,6 +4387,16 @@ extern "C" JSC__JSValue WebCore__AbortSignal__createTimeoutError(const ZigString
     return JSC::JSValue::encode(error);
 }
 
+CPP_DECL double JSC__JSValue__getUnixTimestamp(JSC__JSValue timeValue)
+{
+    JSC::JSValue decodedValue = JSC::JSValue::decode(timeValue);
+    JSC::DateInstance* date = JSC::jsDynamicCast<JSC::DateInstance*>(decodedValue);
+    if (!date)
+        return PNaN;
+
+    return date->internalNumber();
+}
+
 #pragma mark - WebCore::DOMFormData
 
 CPP_DECL void WebCore__DOMFormData__append(WebCore__DOMFormData* arg0, ZigString* arg1, ZigString* arg2)
diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
index c9c733ebc..18aaf3db9 100644
--- a/src/bun.js/bindings/bindings.zig
+++ b/src/bun.js/bindings/bindings.zig
@@ -3352,10 +3352,24 @@ pub const JSValue = enum(JSValueReprInt) {
         cppFn("forEachPropertyOrdered", .{ this, globalObject, ctx, callback });
     }
 
+    pub fn coerceToDouble(
+        this: JSValue,
+        globalObject: *JSC.JSGlobalObject,
+    ) f64 {
+        return cppFn("coerceToDouble", .{ this, globalObject });
+    }
+
     pub fn coerce(this: JSValue, comptime T: type, globalThis: *JSC.JSGlobalObject) T {
         return switch (T) {
             ZigString => this.getZigString(globalThis),
             bool => this.toBooleanSlow(globalThis),
+            f64 => {
+                if (this.isNumber()) {
+                    return this.asDouble();
+                }
+
+                return this.coerceToDouble(globalThis);
+            },
             i32 => {
                 if (this.isInt32()) {
                     return this.asInt32();
@@ -4429,6 +4443,14 @@ pub const JSValue = enum(JSValueReprInt) {
         });
     }
 
+    /// Get the internal number of the `JSC::DateInstance` object
+    /// Returns NaN if the value is not a `JSC::DateInstance` (`Date` in JS)
+     pub fn getUnixTimestamp(this: JSValue) f64 {
+        return cppFn("getUnixTimestamp", .{
+            this,
+        });
+    }
+
     pub fn toFmt(
         this: JSValue,
         global: *JSGlobalObject,
@@ -4670,6 +4692,7 @@ pub const JSValue = enum(JSValueReprInt) {
         "asObject",
         "asPromise",
         "asString",
+        "coerceToDouble",
         "coerceToInt32",
         "coerceToInt64",
         "createEmptyArray",
@@ -4682,10 +4705,11 @@ pub const JSValue = enum(JSValueReprInt) {
         "createTypeError",
         "createUninitializedUint8Array",
         "deepEquals",
+        "deepMatch",
         "eqlCell",
         "eqlValue",
-        "fastGet_",
         "fastGetDirect_",
+        "fastGet_",
         "forEach",
         "forEachProperty",
         "forEachPropertyOrdered",
@@ -4705,6 +4729,7 @@ pub const JSValue = enum(JSValueReprInt) {
         "getPrototype",
         "getStaticProperty",
         "getSymbolDescription",
+        "getUnixTimestamp",
         "hasProperty",
         "isAggregateError",
         "isAnyError",
@@ -4714,11 +4739,13 @@ pub const JSValue = enum(JSValueReprInt) {
         "isBoolean",
         "isCallable",
         "isClass",
+        "isConstructor",
         "isCustomGetterSetter",
         "isError",
         "isException",
         "isGetterSetter",
         "isHeapBigInt",
+        "isInstanceOf",
         "isInt32",
         "isInt32AsAnyInt",
         "isIterable",
@@ -4748,6 +4775,7 @@ pub const JSValue = enum(JSValueReprInt) {
         "putIndex",
         "putRecord",
         "strictDeepEquals",
+        "stringIncludes",
         "symbolFor",
         "symbolKeyFor",
         "toBoolean",
@@ -4755,6 +4783,7 @@ pub const JSValue = enum(JSValueReprInt) {
         "toError_",
         "toInt32",
         "toInt64",
+        "toMatch",
         "toObject",
         "toPropertyKeyValue",
         "toString",
@@ -4763,11 +4792,6 @@ pub const JSValue = enum(JSValueReprInt) {
         "toWTFString",
         "toZigException",
         "toZigString",
-        "toMatch",
-        "isConstructor",
-        "isInstanceOf",
-        "stringIncludes",
-        "deepMatch",
     };
 };
 
diff --git a/src/bun.js/bindings/headers.h b/src/bun.js/bindings/headers.h
index 92639843a..4b3875d83 100644
--- a/src/bun.js/bindings/headers.h
+++ b/src/bun.js/bindings/headers.h
@@ -307,6 +307,7 @@ CPP_DECL double JSC__JSValue__asNumber(JSC__JSValue JSValue0);
 CPP_DECL bJSC__JSObject JSC__JSValue__asObject(JSC__JSValue JSValue0);
 CPP_DECL JSC__JSPromise* JSC__JSValue__asPromise(JSC__JSValue JSValue0);
 CPP_DECL JSC__JSString* JSC__JSValue__asString(JSC__JSValue JSValue0);
+CPP_DECL double JSC__JSValue__coerceToDouble(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1);
 CPP_DECL int32_t JSC__JSValue__coerceToInt32(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1);
 CPP_DECL int64_t JSC__JSValue__coerceToInt64(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1);
 CPP_DECL JSC__JSValue JSC__JSValue__createEmptyArray(JSC__JSGlobalObject* arg0, size_t arg1);
@@ -338,6 +339,7 @@ CPP_DECL double JSC__JSValue__getLengthIfPropertyExistsInternal(JSC__JSValue JSV
 CPP_DECL void JSC__JSValue__getNameProperty(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1, ZigString* arg2);
 CPP_DECL JSC__JSValue JSC__JSValue__getPrototype(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1);
 CPP_DECL void JSC__JSValue__getSymbolDescription(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1, ZigString* arg2);
+CPP_DECL double JSC__JSValue__getUnixTimestamp(JSC__JSValue JSValue0);
 CPP_DECL bool JSC__JSValue__isAggregateError(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1);
 CPP_DECL bool JSC__JSValue__isAnyError(JSC__JSValue JSValue0);
 CPP_DECL bool JSC__JSValue__isAnyInt(JSC__JSValue JSValue0);
diff --git a/src/bun.js/bindings/headers.zig b/src/bun.js/bindings/headers.zig
index 99af5aecc..403be20f6 100644
--- a/src/bun.js/bindings/headers.zig
+++ b/src/bun.js/bindings/headers.zig
@@ -207,6 +207,7 @@ pub extern fn JSC__JSValue__asNumber(JSValue0: JSC__JSValue) f64;
 pub extern fn JSC__JSValue__asObject(JSValue0: JSC__JSValue) bJSC__JSObject;
 pub extern fn JSC__JSValue__asPromise(JSValue0: JSC__JSValue) ?*bindings.JSPromise;
 pub extern fn JSC__JSValue__asString(JSValue0: JSC__JSValue) [*c]bindings.JSString;
+pub extern fn JSC__JSValue__coerceToDouble(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject) f64;
 pub extern fn JSC__JSValue__coerceToInt32(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject) i32;
 pub extern fn JSC__JSValue__coerceToInt64(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject) i64;
 pub extern fn JSC__JSValue__createEmptyArray(arg0: *bindings.JSGlobalObject, arg1: usize) JSC__JSValue;
@@ -238,6 +239,7 @@ pub extern fn JSC__JSValue__getLengthIfPropertyExistsInternal(JSValue0: JSC__JSV
 pub extern fn JSC__JSValue__getNameProperty(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject, arg2: [*c]ZigString) void;
 pub extern fn JSC__JSValue__getPrototype(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject) JSC__JSValue;
 pub extern fn JSC__JSValue__getSymbolDescription(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject, arg2: [*c]ZigString) void;
+pub extern fn JSC__JSValue__getUnixTimestamp(JSValue0: JSC__JSValue) f64;
 pub extern fn JSC__JSValue__isAggregateError(JSValue0: JSC__JSValue, arg1: *bindings.JSGlobalObject) bool;
 pub extern fn JSC__JSValue__isAnyError(JSValue0: JSC__JSValue) bool;
 pub extern fn JSC__JSValue__isAnyInt(JSValue0: JSC__JSValue) bool;
diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig
index 7daddb254..1fe378a84 100644
--- a/src/bun.js/node/types.zig
+++ b/src/bun.js/node/types.zig
@@ -894,29 +894,29 @@ pub fn fileDescriptorFromJS(ctx: JSC.C.JSContextRef, value: JSC.JSValue, excepti
     return @truncate(bun.FileDescriptor, fd);
 }
 
-var _get_time_prop_string: ?JSC.C.JSStringRef = null;
-pub fn timeLikeFromJS(ctx: JSC.C.JSContextRef, value_: JSC.JSValue, exception: JSC.C.ExceptionRef) ?TimeLike {
-    var value = value_;
-    if (JSC.C.JSValueIsDate(ctx, value.asObjectRef())) {
-        // TODO: make this faster
-        var get_time_prop = _get_time_prop_string orelse brk: {
-            var str = JSC.C.JSStringCreateStatic("getTime", "getTime".len);
-            _get_time_prop_string = str;
-            break :brk str;
-        };
+// Node.js docs:
+// > Values can be either numbers representing Unix epoch time in seconds, Dates, or a numeric string like '123456789.0'.
+// > If the value can not be converted to a number, or is NaN, Infinity, or -Infinity, an Error will be thrown.
+pub fn timeLikeFromJS(globalThis: *JSC.JSGlobalObject, value: JSC.JSValue, _: JSC.C.ExceptionRef) ?TimeLike {
+    if (value.jsType() == .JSDate) {
+        const milliseconds = value.getUnixTimestamp();
+        if (!std.math.isFinite(milliseconds)) {
+            return null;
+        }
 
-        var getTimeFunction = JSC.C.JSObjectGetProperty(ctx, value.asObjectRef(), get_time_prop, exception);
-        if (exception.* != null) return null;
-        value = JSC.JSValue.fromRef(JSC.C.JSObjectCallAsFunction(ctx, getTimeFunction, value.asObjectRef(), 0, null, exception) orelse return null);
-        if (exception.* != null) return null;
+        return @truncate(TimeLike, @floatToInt(i64, milliseconds / @as(f64, std.time.ms_per_s)));
+    }
+
+    if (!value.isNumber() and !value.isString()) {
+        return null;
     }
 
-    const seconds = value.asNumber();
+    const seconds = value.coerce(f64, globalThis);
     if (!std.math.isFinite(seconds)) {
         return null;
     }
 
-    return @floatToInt(TimeLike, @max(@floor(seconds), std.math.minInt(TimeLike)));
+    return @truncate(TimeLike, @floatToInt(i64, seconds));
 }
 
 pub fn modeFromJS(ctx: JSC.C.JSContextRef, value: JSC.JSValue, exception: JSC.C.ExceptionRef) ?Mode {
diff --git a/src/string.zig b/src/string.zig
index cf4109d40..2c75dcc33 100644
--- a/src/string.zig
+++ b/src/string.zig
@@ -243,10 +243,12 @@ pub const String = extern struct {
     extern fn BunString__fromBytes(bytes: [*]const u8, len: usize) String;
 
     pub fn createLatin1(bytes: []const u8) String {
+        JSC.markBinding(@src());
         return BunString__fromLatin1(bytes.ptr, bytes.len);
     }
 
     pub fn create(bytes: []const u8) String {
+        JSC.markBinding(@src());
         return BunString__fromBytes(bytes.ptr, bytes.len);
     }
 
diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts
index 999547c93..37c3253a4 100644
--- a/test/js/node/fs/fs.test.ts
+++ b/test/js/node/fs/fs.test.ts
@@ -1221,3 +1221,75 @@ it("existsSync with invalid path doesn't throw", () => {
   expect(existsSync(undefined as any)).toBe(false);
   expect(existsSync({ invalid: 1 } as any)).toBe(false);
 });
+
+describe("utimesSync", () => {
+  it("works", () => {
+    const tmp = join(tmpdir(), "utimesSync-test-file-" + Math.random().toString(36).slice(2));
+    writeFileSync(tmp, "test");
+    const prevStats = fs.statSync(tmp);
+    const prevModifiedTime = prevStats.mtime;
+    const prevAccessTime = prevStats.atime;
+
+    prevModifiedTime.setMilliseconds(0);
+    prevAccessTime.setMilliseconds(0);
+
+    prevModifiedTime.setFullYear(1996);
+    prevAccessTime.setFullYear(1996);
+
+    // Get the current time to change the timestamps
+    const newModifiedTime = new Date();
+    const newAccessTime = new Date();
+
+    newModifiedTime.setMilliseconds(0);
+    newAccessTime.setMilliseconds(0);
+
+    fs.utimesSync(tmp, newAccessTime, newModifiedTime);
+
+    const newStats = fs.statSync(tmp);
+
+    expect(newStats.mtime).toEqual(newModifiedTime);
+    expect(newStats.atime).toEqual(newAccessTime);
+
+    fs.utimesSync(tmp, prevAccessTime, prevModifiedTime);
+
+    const finalStats = fs.statSync(tmp);
+
+    expect(finalStats.mtime).toEqual(prevModifiedTime);
+    expect(finalStats.atime).toEqual(prevAccessTime);
+  });
+
+  it("accepts a Number(value).toString()", () => {
+    const tmp = join(tmpdir(), "utimesSync-test-file2-" + Math.random().toString(36).slice(2));
+    writeFileSync(tmp, "test");
+    const prevStats = fs.statSync(tmp);
+    const prevModifiedTime = prevStats.mtime;
+    const prevAccessTime = prevStats.atime;
+
+    prevModifiedTime.setMilliseconds(0);
+    prevAccessTime.setMilliseconds(0);
+
+    prevModifiedTime.setFullYear(1996);
+    prevAccessTime.setFullYear(1996);
+
+    // Get the current time to change the timestamps
+    const newModifiedTime = new Date();
+    const newAccessTime = new Date();
+
+    newModifiedTime.setMilliseconds(0);
+    newAccessTime.setMilliseconds(0);
+
+    fs.utimesSync(tmp, newAccessTime.getTime() / 1000 + "", newModifiedTime.getTime() / 1000 + "");
+
+    const newStats = fs.statSync(tmp);
+
+    expect(newStats.mtime).toEqual(newModifiedTime);
+    expect(newStats.atime).toEqual(newAccessTime);
+
+    fs.utimesSync(tmp, prevAccessTime.getTime() / 1000 + "", prevModifiedTime.getTime() / 1000 + "");
+
+    const finalStats = fs.statSync(tmp);
+
+    expect(finalStats.mtime).toEqual(prevModifiedTime);
+    expect(finalStats.atime).toEqual(prevAccessTime);
+  });
+});