From 5b51d421daf5d22261e18343ac7b5222abcdfba3 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 6 Oct 2025 19:47:05 -0700 Subject: [PATCH] fix(node:path): reverse iterate `path.resolve` arguments, and stop on absolute (#23293) ### What does this PR do? Matches node behavior. Fixes #20975 ### How did you verify your code works? Manually and added a test --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/node/path.zig | 46 ++++++++++++++++++++++--------- test/js/node/path/resolve.test.js | 11 +++++++- test/no-validate-exceptions.txt | 2 ++ 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/bun.js/node/path.zig b/src/bun.js/node/path.zig index 94a90dcb1e..c6011c3bb4 100644 --- a/src/bun.js/node/path.zig +++ b/src/bun.js/node/path.zig @@ -2767,35 +2767,55 @@ pub fn resolve(globalObject: *jsc.JSGlobalObject, isWindows: bool, args_ptr: [*] var stack_fallback = std.heap.stackFallback(stack_fallback_size_large, arena.allocator()); const allocator = stack_fallback.get(); - var paths = try allocator.alloc(string, args_len); - defer allocator.free(paths); - var path_count: usize = 0; + var paths_buf = try allocator.alloc(string, args_len); + defer allocator.free(paths_buf); + var paths_offset: usize = args_len; + var resolved_root = false; - for (0..args_len, args_ptr) |i, path_ptr| { - // Supress exeption in zig. It does globalThis.vm().throwError() in JS land. - try validateString(globalObject, path_ptr, "paths[{d}]", .{i}); - const pathZStr = try path_ptr.getZigString(globalObject); - if (pathZStr.len > 0) { - paths[path_count] = pathZStr.toSlice(allocator).slice(); - path_count += 1; + var i = args_len; + while (i > 0) { + i -= 1; + + if (resolved_root) { + break; + } + + const path = args_ptr[i]; + try validateString(globalObject, path, "paths[{d}]", .{i}); + const path_str = try path.toBunString(globalObject); + defer path_str.deref(); + + if (path_str.length() == 0) { + continue; + } + + paths_offset -= 1; + paths_buf[paths_offset] = path_str.toSlice(allocator).slice(); + + if (!isWindows) { + if (path_str.charAt(0) == CHAR_FORWARD_SLASH) { + resolved_root = true; + } } } + const paths = paths_buf[paths_offset..]; + if (comptime Environment.isPosix) { if (!isWindows) { // Micro-optimization #1: avoid creating a new string when passing no arguments or only empty strings. - if (path_count == 0) { + if (paths.len == 0) { return Process__getCachedCwd(globalObject); } // Micro-optimization #2: path.resolve(".") and path.resolve("./") === process.cwd() - else if (path_count == 1 and (strings.eqlComptime(paths[0], ".") or strings.eqlComptime(paths[0], "./"))) { + else if (paths.len == 1 and (strings.eqlComptime(paths[0], ".") or strings.eqlComptime(paths[0], "./"))) { return Process__getCachedCwd(globalObject); } } } - return resolveJS_T(u8, globalObject, allocator, isWindows, paths[0..path_count]); + return resolveJS_T(u8, globalObject, allocator, isWindows, paths); } /// Based on Node v21.6.1 path.win32.toNamespacedPath: diff --git a/test/js/node/path/resolve.test.js b/test/js/node/path/resolve.test.js index 7204751052..e620a7f60e 100644 --- a/test/js/node/path/resolve.test.js +++ b/test/js/node/path/resolve.test.js @@ -1,4 +1,4 @@ -import { describe, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { isWindows } from "harness"; import assert from "node:assert"; // import child from "node:child_process"; @@ -93,4 +93,13 @@ describe("path.resolve", () => { // } // } }); + + test("undefined argument are ignored if absolute path comes first (reverse loop through args)", () => { + expect(() => { + return path.posix.resolve(undefined, "hi"); + }).toThrow('The "paths[0]" property must be of type string, got undefined'); + expect(() => { + return path.posix.resolve(undefined, "/hi"); + }).not.toThrow(); + }); }); diff --git a/test/no-validate-exceptions.txt b/test/no-validate-exceptions.txt index 9ca358c41c..4c4aa54228 100644 --- a/test/no-validate-exceptions.txt +++ b/test/no-validate-exceptions.txt @@ -144,3 +144,5 @@ vendor/elysia/test/core/dynamic.test.ts vendor/elysia/test/lifecycle/error.test.ts vendor/elysia/test/validator/body.test.ts vendor/elysia/test/ws/message.test.ts + +test/js/node/test/parallel/test-worker-abort-on-uncaught-exception.js