Fix crash with path parse in win32 (#3187)bun-v0.6.7

* Update CommonJSModuleRecord.cpp

* smaller

* [node:path] Fix crash, mark TODO

---------

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

5 files changed, 86 insertions, 15 deletions

diff --git a/bench/snippets/crypto-hasher.mjs b/bench/snippets/crypto-hasher.mjs
index 2f96dae7b..36f67739a 100644
--- a/bench/snippets/crypto-hasher.mjs
+++ b/bench/snippets/crypto-hasher.mjs
@@ -3,13 +3,13 @@ import { bench, run } from "./runner.mjs";
 
 import crypto from "node:crypto";
 
-var foo = Buffer.allocUnsafe(16384);
+var foo = Buffer.allocUnsafe(512);
 foo.fill(123);
 
 // if ("Bun" in globalThis) {
 //   const { CryptoHasher } = Bun;
-//   bench("CryptoHasher Blake2b256", () => {
-//     var hasher = new CryptoHasher("blake2b256");
+//   bench("Bun.CryptoHasher(sha512)", () => {
+//     var hasher = new CryptoHasher("sha512");
 //     hasher.update(foo);
 //     hasher.digest();
 //   });
diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp
index 11cd7b727..fcf3dfe8c 100644
--- a/src/bun.js/bindings/CommonJSModuleRecord.cpp
+++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp
@@ -392,7 +392,7 @@ JSC::SourceCode createCommonJSModule(
                 //      seems harder to do correctly.
                 {
                     // We must use a global scope extension or else the JSWithScope will be collected unexpectedly.
-                    // https://github.com/oven-sh/issues/3161
+                    // https://github.com/oven-sh/bun/issues/3161
                     globalObject->clearGlobalScopeExtension();
 
                     JSWithScope* withScope = JSWithScope::create(vm, globalObject, globalObject->globalScope(), scopeExtensionObject);
diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig
index 8f358a6a3..987b30d3c 100644
--- a/src/bun.js/node/types.zig
+++ b/src/bun.js/node/types.zig
@@ -1687,6 +1687,7 @@ pub const Path = struct {
         return JSC.JSValue.jsBoolean(zig_str.len > 0 and isAbsoluteString(zig_str, isWindows));
     }
     fn isZigStringAbsoluteWindows(zig_str: JSC.ZigString) bool {
+        std.debug.assert(zig_str.len > 0); // caller must check
         if (zig_str.is16Bit()) {
             var buf = [4]u16{ 0, 0, 0, 0 };
             var u16_slice = zig_str.utf16Slice();
@@ -1770,7 +1771,7 @@ pub const Path = struct {
         if (str_slice.isAllocated()) out_str.setOutputEncoding();
         return out_str.toValueGC(globalThis);
     }
-    pub fn parse(globalThis: *JSC.JSGlobalObject, isWindows: bool, args_ptr: [*]JSC.JSValue, args_len: u16) callconv(.C) JSC.JSValue {
+    pub fn parse(globalThis: *JSC.JSGlobalObject, win32: bool, args_ptr: [*]JSC.JSValue, args_len: u16) callconv(.C) JSC.JSValue {
         if (comptime is_bindgen) return JSC.JSValue.jsUndefined();
         if (args_len == 0 or !args_ptr[0].jsType().isStringLike()) {
             return JSC.toInvalidArguments("path string is required", .{}, globalThis);
@@ -1778,14 +1779,17 @@ pub const Path = struct {
         var path_slice: JSC.ZigString.Slice = args_ptr[0].toSlice(globalThis, heap_allocator);
         defer path_slice.deinit();
         var path = path_slice.slice();
-        var path_name = Fs.NodeJSPathName.init(path);
+        const path_name = Fs.NodeJSPathName.init(
+            path,
+            if (win32) std.fs.path.sep_windows else std.fs.path.sep_posix,
+        );
         var dir = JSC.ZigString.init(path_name.dir);
-        const is_absolute = (isWindows and isZigStringAbsoluteWindows(dir)) or (!isWindows and path.len > 0 and path[0] == '/');
+        const is_absolute = (win32 and dir.len > 0 and isZigStringAbsoluteWindows(dir)) or (!win32 and path.len > 0 and path[0] == '/');
 
         // if its not absolute root must be empty
         var root = JSC.ZigString.Empty;
         if (is_absolute) {
-            root = JSC.ZigString.init(if (isWindows) std.fs.path.sep_str_windows else std.fs.path.sep_str_posix);
+            root = JSC.ZigString.init(if (win32) std.fs.path.sep_str_windows else std.fs.path.sep_str_posix);
             // if is absolute and dir is empty, then dir = root
             if (path_name.dir.len == 0) {
                 dir = root;
diff --git a/src/fs.zig b/src/fs.zig
index c0f3cd9dd..2dca8169c 100644
--- a/src/fs.zig
+++ b/src/fs.zig
@@ -1197,19 +1197,19 @@ pub const NodeJSPathName = struct {
     ext: string,
     filename: string,
 
-    pub fn init(_path: string) NodeJSPathName {
+    pub fn init(_path: string, sep: u8) NodeJSPathName {
         var path = _path;
         var base = path;
         // ext must be empty if not detected
         var ext: string = "";
         var dir = path;
         var is_absolute = true;
-        var _i = strings.lastIndexOfChar(path, '/');
+        var _i = strings.lastIndexOfChar(path, sep);
         var first = true;
         while (_i) |i| {
 
             // Stop if we found a non-trailing slash
-            if (i + 1 != path.len) {
+            if (i + 1 != path.len and path.len >= i + 1) {
                 base = path[i + 1 ..];
                 dir = path[0..i];
                 is_absolute = false;
@@ -1228,11 +1228,11 @@ pub const NodeJSPathName = struct {
 
             path = path[0..i];
 
-            _i = strings.lastIndexOfChar(path, '/');
+            _i = strings.lastIndexOfChar(path, sep);
         }
 
         // clean trailing slashs
-        if (base.len > 1 and base[base.len - 1] == '/') {
+        if (base.len > 1 and base[base.len - 1] == sep) {
             base = base[0 .. base.len - 1];
         }
 
@@ -1245,7 +1245,8 @@ pub const NodeJSPathName = struct {
             var _dot = strings.lastIndexOfChar(filename, '.');
             if (_dot) |dot| {
                 ext = filename[dot..];
-                filename = filename[0..dot];
+                if (dot > 0)
+                    filename = filename[0..dot];
             }
         }
 
@@ -1333,7 +1334,7 @@ pub const PathName = struct {
         var _i = strings.lastIndexOfChar(path, '/');
         while (_i) |i| {
             // Stop if we found a non-trailing slash
-            if (i + 1 != path.len) {
+            if (i + 1 != path.len and path.len > i + 1) {
                 base = path[i + 1 ..];
                 dir = path[0..i];
                 is_absolute = false;
diff --git a/test/js/node/path/path.test.js b/test/js/node/path/path.test.js
index caaf12db0..94b0568f6 100644
--- a/test/js/node/path/path.test.js
+++ b/test/js/node/path/path.test.js
@@ -3,12 +3,18 @@ const { file } = import.meta;
 import { describe, it, expect } from "bun:test";
 import * as path from "node:path";
 import assert from "assert";
+import { hideFromStackTrace } from "harness";
 
 const strictEqual = (...args) => {
   assert.strictEqual(...args);
   expect(true).toBe(true);
 };
 
+const expectStrictEqual = (actual, expected) => {
+  expect(actual).toBe(expected);
+};
+hideFromStackTrace(expectStrictEqual);
+
 it("should not inherit Object.prototype", () => {
   expect(path).not.toHaveProperty("toString");
 });
@@ -35,6 +41,66 @@ it("path.dirname", () => {
   }
 });
 
+it("path.parse().name", () => {
+  expectStrictEqual(path.parse(file).name, "path.test");
+  expectStrictEqual(path.parse(".js").name, ".js");
+  expectStrictEqual(path.parse("..js").name, ".");
+  expectStrictEqual(path.parse("").name, "");
+  expectStrictEqual(path.parse(".").name, ".");
+  expectStrictEqual(path.parse("dir/name.ext").name, "name");
+  expectStrictEqual(path.parse("/dir/name.ext").name, "name");
+  expectStrictEqual(path.parse("/name.ext").name, "name");
+  expectStrictEqual(path.parse("name.ext").name, "name");
+  expectStrictEqual(path.parse("name.ext/").name, "name");
+  expectStrictEqual(path.parse("name.ext//").name, "name");
+  expectStrictEqual(path.parse("aaa/bbb").name, "bbb");
+  expectStrictEqual(path.parse("aaa/bbb/").name, "bbb");
+  expectStrictEqual(path.parse("aaa/bbb//").name, "bbb");
+  expectStrictEqual(path.parse("/aaa/bbb").name, "bbb");
+  expectStrictEqual(path.parse("/aaa/bbb/").name, "bbb");
+  expectStrictEqual(path.parse("/aaa/bbb//").name, "bbb");
+  expectStrictEqual(path.parse("//aaa/bbb").name, "bbb");
+  expectStrictEqual(path.parse("//aaa/bbb/").name, "bbb");
+  expectStrictEqual(path.parse("//aaa/bbb//").name, "bbb");
+  expectStrictEqual(path.parse("///aaa").name, "aaa");
+  expectStrictEqual(path.parse("//aaa").name, "aaa");
+  expectStrictEqual(path.parse("/aaa").name, "aaa");
+  expectStrictEqual(path.parse("aaa.").name, "aaa");
+
+  // On unix a backslash is just treated as any other character.
+  expectStrictEqual(path.posix.parse("\\dir\\name.ext").name, "\\dir\\name");
+  expectStrictEqual(path.posix.parse("\\name.ext").name, "\\name");
+  expectStrictEqual(path.posix.parse("name.ext").name, "name");
+  expectStrictEqual(path.posix.parse("name.ext\\").name, "name");
+  expectStrictEqual(path.posix.parse("name.ext\\\\").name, "name");
+});
+
+it("path.parse() windows edition", () => {
+  // On Windows a backslash acts as a path separator.
+  expectStrictEqual(path.win32.parse("\\dir\\name.ext").name, "name");
+  expectStrictEqual(path.win32.parse("\\name.ext").name, "name");
+  expectStrictEqual(path.win32.parse("name.ext").name, "name");
+  expectStrictEqual(path.win32.parse("name.ext\\").name, "name");
+  expectStrictEqual(path.win32.parse("name.ext\\\\").name, "name");
+  expectStrictEqual(path.win32.parse("name").name, "name");
+  expectStrictEqual(path.win32.parse(".name").name, ".name");
+  expectStrictEqual(path.win32.parse("file:stream").name, "file:stream");
+});
+
+it.todo("path.parse() windows edition - drive letter", () => {
+  expectStrictEqual(path.win32.parse("C:").name, "");
+  expectStrictEqual(path.win32.parse("C:.").name, ".");
+  expectStrictEqual(path.win32.parse("C:\\").name, "");
+  expectStrictEqual(path.win32.parse("C:\\.").name, ".");
+  expectStrictEqual(path.win32.parse("C:\\.ext").name, ".ext");
+  expectStrictEqual(path.win32.parse("C:\\dir\\name.ext").name, "name");
+  expectStrictEqual(path.win32.parse("C:name.ext").name, "name");
+  expectStrictEqual(path.win32.parse("C:name.ext\\").name, "name");
+  expectStrictEqual(path.win32.parse("C:name.ext\\\\").name, "name");
+  expectStrictEqual(path.win32.parse("C:foo").name, "foo");
+  expectStrictEqual(path.win32.parse("C:.foo").name, ".foo");
+});
+
 it("path.basename", () => {
   strictEqual(path.basename(file), "path.test.js");
   strictEqual(path.basename(file, ".js"), "path.test");