From 9646bf1a383179895080d29697337f1229b9dda0 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 23 Apr 2025 16:42:33 -0700 Subject: [PATCH] Fixes #16671 (#19227) Co-authored-by: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> --- src/bun.js/ModuleLoader.zig | 100 ++++++++++++------ src/bun.js/bindings/BunPlugin.cpp | 45 +++----- src/bun.js/bindings/JSGlobalObject.zig | 12 ++- src/bun.js/bindings/ModuleLoader.cpp | 7 +- src/transpiler.zig | 4 +- .../plugin/plugin-recursive-fixture-run.ts | 7 ++ .../js/bun/plugin/plugin-recursive-fixture.ts | 11 ++ test/js/bun/plugin/plugins.test.ts | 39 +++++++ 8 files changed, 153 insertions(+), 72 deletions(-) create mode 100644 test/js/bun/plugin/plugin-recursive-fixture-run.ts create mode 100644 test/js/bun/plugin/plugin-recursive-fixture.ts diff --git a/src/bun.js/ModuleLoader.zig b/src/bun.js/ModuleLoader.zig index 1437fb30a9..0473e7711a 100644 --- a/src/bun.js/ModuleLoader.zig +++ b/src/bun.js/ModuleLoader.zig @@ -427,15 +427,23 @@ pub const AsyncModule = struct { this.poll_ref.unref(jsc_vm); outer: { errorable = JSC.ErrorableResolvedSource.ok(this.resumeLoadingModule(&log) catch |err| { - VirtualMachine.processFetchLog( - this.globalThis, - bun.String.init(this.specifier), - bun.String.init(this.referrer), - &log, - &errorable, - err, - ); - break :outer; + switch (err) { + error.JSError => { + errorable = .err(error.JSError, this.globalThis.takeError(error.JSError).asVoid()); + break :outer; + }, + else => { + VirtualMachine.processFetchLog( + this.globalThis, + bun.String.init(this.specifier), + bun.String.init(this.referrer), + &log, + &errorable, + err, + ); + break :outer; + }, + } }); } @@ -455,7 +463,7 @@ pub const AsyncModule = struct { pub fn fulfill( globalThis: *JSGlobalObject, promise: JSValue, - resolved_source: ResolvedSource, + resolved_source: *ResolvedSource, err: ?anyerror, specifier_: bun.String, referrer_: bun.String, @@ -471,16 +479,27 @@ pub const AsyncModule = struct { var errorable: JSC.ErrorableResolvedSource = undefined; if (err) |e| { - VirtualMachine.processFetchLog( - globalThis, - specifier, - referrer, - log, - &errorable, - e, - ); + defer { + if (resolved_source.source_code_needs_deref) { + resolved_source.source_code_needs_deref = false; + resolved_source.source_code.deref(); + } + } + + if (e == error.JSError) { + errorable = JSC.ErrorableResolvedSource.err(error.JSError, globalThis.takeError(error.JSError).asVoid()); + } else { + VirtualMachine.processFetchLog( + globalThis, + specifier, + referrer, + log, + &errorable, + e, + ); + } } else { - errorable = JSC.ErrorableResolvedSource.ok(resolved_source); + errorable = JSC.ErrorableResolvedSource.ok(resolved_source.*); } log.deinit(); @@ -1782,17 +1801,21 @@ pub export fn Bun__transpileFile( globalObject, FetchFlags.transpile, ) catch |err| { - if (err == error.AsyncModule) { - bun.assert(promise != null); - return promise; + switch (err) { + error.AsyncModule => { + bun.assert(promise != null); + return promise; + }, + error.PluginError => return null, + error.JSError => { + ret.* = JSC.ErrorableResolvedSource.err(error.JSError, globalObject.takeError(error.JSError).asVoid()); + return null; + }, + else => { + VirtualMachine.processFetchLog(globalObject, specifier_ptr.*, referrer.*, &log, ret, err); + return null; + }, } - - if (err == error.PluginError) { - return null; - } - - VirtualMachine.processFetchLog(globalObject, specifier_ptr.*, referrer.*, &log, ret, err); - return null; }, ); return promise; @@ -1816,8 +1839,9 @@ export fn Bun__runVirtualModule(globalObject: *JSGlobalObject, specifier_ptr: *c else specifier[@min(namespace.len + 1, specifier.len)..]; - return globalObject.runOnLoadPlugins(bun.String.init(namespace), bun.String.init(after_namespace), .bun) orelse + return globalObject.runOnLoadPlugins(bun.String.init(namespace), bun.String.init(after_namespace), .bun) catch { return JSValue.zero; + } orelse return .zero; } fn getHardcodedModule(jsc_vm: *VirtualMachine, specifier: bun.String, hardcoded: HardcodedModule) ?ResolvedSource { @@ -1957,11 +1981,17 @@ export fn Bun__transpileVirtualModule( globalObject, FetchFlags.transpile, ) catch |err| { - if (err == error.PluginError) { - return true; + switch (err) { + error.PluginError => return true, + error.JSError => { + ret.* = JSC.ErrorableResolvedSource.err(error.JSError, globalObject.takeError(error.JSError).asVoid()); + return true; + }, + else => { + VirtualMachine.processFetchLog(globalObject, specifier_ptr.*, referrer_ptr.*, &log, ret, err); + return true; + }, } - VirtualMachine.processFetchLog(globalObject, specifier_ptr.*, referrer_ptr.*, &log, ret, err); - return true; }, ); Analytics.Features.virtual_modules += 1; @@ -2258,7 +2288,7 @@ pub const RuntimeTranspilerStore = struct { _ = vm.transpiler_store.store.put(this); - ModuleLoader.AsyncModule.fulfill(globalThis, promise, resolved_source, parse_error, specifier, referrer, &log); + ModuleLoader.AsyncModule.fulfill(globalThis, promise, &resolved_source, parse_error, specifier, referrer, &log); } pub fn schedule(this: *TranspilerJob) void { diff --git a/src/bun.js/bindings/BunPlugin.cpp b/src/bun.js/bindings/BunPlugin.cpp index d8d95b3415..1fb8fed838 100644 --- a/src/bun.js/bindings/BunPlugin.cpp +++ b/src/bun.js/bindings/BunPlugin.cpp @@ -30,7 +30,7 @@ #include "BunClientData.h" #include "JSCommonJSModule.h" #include "isBuiltinModule.h" - +#include "AsyncContextFrame.h" #include "ImportMetaObject.h" namespace Zig { @@ -707,6 +707,8 @@ EncodedJSValue BunPlugin::OnLoad::run(JSC::JSGlobalObject* globalObject, BunStri JSC::MarkedArgumentBuffer arguments; auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + scope.assertNoExceptionExceptTermination(); JSC::JSObject* paramsObject = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 1); const auto& builtinNames = WebCore::builtinNames(vm); @@ -715,16 +717,8 @@ EncodedJSValue BunPlugin::OnLoad::run(JSC::JSGlobalObject* globalObject, BunStri jsString(vm, pathString)); arguments.append(paramsObject); - auto throwScope = DECLARE_THROW_SCOPE(vm); - auto scope = DECLARE_CATCH_SCOPE(vm); - scope.assertNoExceptionExceptTermination(); - - JSC::CallData callData = JSC::getCallData(function); - - auto result = call(globalObject, function, callData, JSC::jsUndefined(), arguments); - if (UNLIKELY(scope.exception())) { - return JSValue::encode(scope.exception()); - } + auto result = AsyncContextFrame::call(globalObject, function, JSC::jsUndefined(), arguments); + RETURN_IF_EXCEPTION(scope, {}); if (auto* promise = JSC::jsDynamicCast(result)) { switch (promise->status(vm)) { @@ -740,11 +734,11 @@ EncodedJSValue BunPlugin::OnLoad::run(JSC::JSGlobalObject* globalObject, BunStri } if (!result.isObject()) { - JSC::throwTypeError(globalObject, throwScope, "onLoad() expects an object returned"_s); + JSC::throwTypeError(globalObject, scope, "onLoad() expects an object returned"_s); return JSValue::encode({}); } - RELEASE_AND_RETURN(throwScope, JSValue::encode(result)); + RELEASE_AND_RETURN(scope, JSValue::encode(result)); } std::optional BunPlugin::OnLoad::resolveVirtualModule(const String& path, const String& from) @@ -780,8 +774,10 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, BunS } auto& callbacks = group.callbacks; - + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); WTF::String pathString = path->toWTFString(BunString::ZeroCopy); + for (size_t i = 0; i < filters.size(); i++) { if (!filters[i].get()->match(globalObject, pathString, 0)) { continue; @@ -792,7 +788,6 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, BunS } JSC::MarkedArgumentBuffer arguments; - auto& vm = JSC::getVM(globalObject); JSC::JSObject* paramsObject = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); const auto& builtinNames = WebCore::builtinNames(vm); @@ -804,18 +799,8 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, BunS Bun::toJS(globalObject, *importer)); arguments.append(paramsObject); - auto throwScope = DECLARE_THROW_SCOPE(vm); - auto scope = DECLARE_CATCH_SCOPE(vm); - scope.assertNoExceptionExceptTermination(); - - JSC::CallData callData = JSC::getCallData(function); - - auto result = call(globalObject, function, callData, JSC::jsUndefined(), arguments); - if (UNLIKELY(scope.exception())) { - JSC::Exception* exception = scope.exception(); - scope.clearException(); - return JSValue::encode(exception); - } + auto result = AsyncContextFrame::call(globalObject, function, JSC::jsUndefined(), arguments); + RETURN_IF_EXCEPTION(scope, {}); if (result.isUndefinedOrNull()) { continue; @@ -824,7 +809,7 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, BunS if (auto* promise = JSC::jsDynamicCast(result)) { switch (promise->status(vm)) { case JSPromise::Status::Pending: { - JSC::throwTypeError(globalObject, throwScope, "onResolve() doesn't support pending promises yet"_s); + JSC::throwTypeError(globalObject, scope, "onResolve() doesn't support pending promises yet"_s); return JSValue::encode({}); } case JSPromise::Status::Rejected: { @@ -840,11 +825,11 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, BunS } if (!result.isObject()) { - JSC::throwTypeError(globalObject, throwScope, "onResolve() expects an object returned"_s); + JSC::throwTypeError(globalObject, scope, "onResolve() expects an object returned"_s); return JSValue::encode({}); } - RELEASE_AND_RETURN(throwScope, JSValue::encode(result)); + RELEASE_AND_RETURN(scope, JSValue::encode(result)); } return JSValue::encode(JSC::jsUndefined()); diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index b7e4dd0b28..9629e2c81a 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -230,9 +230,12 @@ pub const JSGlobalObject = opaque { extern fn Bun__runOnLoadPlugins(*JSC.JSGlobalObject, ?*const bun.String, *const bun.String, BunPluginTarget) JSValue; extern fn Bun__runOnResolvePlugins(*JSC.JSGlobalObject, ?*const bun.String, *const bun.String, *const String, BunPluginTarget) JSValue; - pub fn runOnLoadPlugins(this: *JSGlobalObject, namespace_: bun.String, path: bun.String, target: BunPluginTarget) ?JSValue { + pub fn runOnLoadPlugins(this: *JSGlobalObject, namespace_: bun.String, path: bun.String, target: BunPluginTarget) bun.JSError!?JSValue { JSC.markBinding(@src()); const result = Bun__runOnLoadPlugins(this, if (namespace_.length() > 0) &namespace_ else null, &path, target); + if (this.hasException()) { + return error.JSError; + } if (result.isEmptyOrUndefinedOrNull()) { return null; } @@ -240,10 +243,15 @@ pub const JSGlobalObject = opaque { return result; } - pub fn runOnResolvePlugins(this: *JSGlobalObject, namespace_: bun.String, path: bun.String, source: bun.String, target: BunPluginTarget) ?JSValue { + pub fn runOnResolvePlugins(this: *JSGlobalObject, namespace_: bun.String, path: bun.String, source: bun.String, target: BunPluginTarget) bun.JSError!?JSValue { JSC.markBinding(@src()); const result = Bun__runOnResolvePlugins(this, if (namespace_.length() > 0) &namespace_ else null, &path, &source, target); + + if (this.hasException()) { + return error.JSError; + } + if (result.isEmptyOrUndefinedOrNull()) { return null; } diff --git a/src/bun.js/bindings/ModuleLoader.cpp b/src/bun.js/bindings/ModuleLoader.cpp index 110997a8a7..face386f1c 100644 --- a/src/bun.js/bindings/ModuleLoader.cpp +++ b/src/bun.js/bindings/ModuleLoader.cpp @@ -101,8 +101,7 @@ static JSC::SyntheticSourceProvider::SyntheticSourceGenerator generateInternalMo JSC::EnsureStillAliveScope stillAlive(object); PropertyNameArray properties(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude); - object->getPropertyNames(globalObject, properties, DontEnumPropertiesMode::Exclude); - + object->getOwnPropertyNames(object, globalObject, properties, DontEnumPropertiesMode::Exclude); RETURN_IF_EXCEPTION(throwScope, void()); auto len = properties.size() + 1; @@ -116,7 +115,9 @@ static JSC::SyntheticSourceProvider::SyntheticSourceGenerator generateInternalMo hasDefault = true; } exportNames.append(entry); - exportValues.append(object->get(globalObject, entry)); + JSValue value = object->get(globalObject, entry); + RETURN_IF_EXCEPTION(throwScope, void()); + exportValues.append(value); } if (!hasDefault) { diff --git a/src/transpiler.zig b/src/transpiler.zig index b7787ee27d..77f9ebbefa 100644 --- a/src/transpiler.zig +++ b/src/transpiler.zig @@ -151,7 +151,7 @@ pub const PluginRunner = struct { bun.String.init(namespace_slice) else bun.String.empty; - const on_resolve_plugin = global.runOnResolvePlugins( + const on_resolve_plugin = try global.runOnResolvePlugins( namespace, bun.String.init(specifier).substring(if (namespace.length() > 0) namespace.length() + 1 else 0), bun.String.init(importer), @@ -241,7 +241,7 @@ pub const PluginRunner = struct { pub fn onResolveJSC(this: *const PluginRunner, namespace: bun.String, specifier: bun.String, importer: bun.String, target: JSC.JSGlobalObject.BunPluginTarget) bun.JSError!?JSC.ErrorableString { var global = this.global_object; - const on_resolve_plugin = global.runOnResolvePlugins( + const on_resolve_plugin = try global.runOnResolvePlugins( if (namespace.length() > 0 and !namespace.eqlComptime("file")) namespace else diff --git a/test/js/bun/plugin/plugin-recursive-fixture-run.ts b/test/js/bun/plugin/plugin-recursive-fixture-run.ts new file mode 100644 index 0000000000..ca9ab4a163 --- /dev/null +++ b/test/js/bun/plugin/plugin-recursive-fixture-run.ts @@ -0,0 +1,7 @@ +try { + require("recursive:recursive"); +} catch (e: any) { + console.log(e.message); +} + +await import("recursive:recursive"); diff --git a/test/js/bun/plugin/plugin-recursive-fixture.ts b/test/js/bun/plugin/plugin-recursive-fixture.ts new file mode 100644 index 0000000000..71234ded2a --- /dev/null +++ b/test/js/bun/plugin/plugin-recursive-fixture.ts @@ -0,0 +1,11 @@ +Bun.plugin({ + name: "recursive", + setup(build) { + build.onResolve({ filter: /recursive$/, namespace: "recursive" }, args => { + return { + path: require.resolve("recursive:" + args.path), + namespace: "recursive", + }; + }); + }, +}); diff --git a/test/js/bun/plugin/plugins.test.ts b/test/js/bun/plugin/plugins.test.ts index efca70591c..0703b65b66 100644 --- a/test/js/bun/plugin/plugins.test.ts +++ b/test/js/bun/plugin/plugins.test.ts @@ -35,6 +35,16 @@ plugin({ }, }); +plugin({ + name: "recursion", + setup(builder) { + builder.onResolve({ filter: /.*/, namespace: "recursion" }, ({ path }) => ({ + path: require.resolve("recursion:" + path), + namespace: "recursion", + })); + }, +}); + plugin({ name: "boop beep beep", setup(builder) { @@ -190,6 +200,7 @@ plugin({ import "../../third_party/svelte"; import "./module-plugins"; import { render as svelteRender } from "svelte/server"; +import { bunEnv, bunExe } from "harness"; describe("require", () => { it("SSRs `

Hello world!

` with Svelte", () => { @@ -510,3 +521,31 @@ it("import(...) without __esModule", async () => { const { default: mod } = await import("my-virtual-module-with-default"); expect(mod).toBe("world"); }); + +it("recursion throws stack overflow", () => { + expect(() => { + require("recursion:recursion"); + }).toThrow("Maximum call stack size exceeded"); + + try { + require("recursion:recursion"); + throw -1; + } catch (e: any) { + if (e === -1) { + throw new Error("Expected error"); + } + expect(e.message).toMatchInlineSnapshot(`"Maximum call stack size exceeded."`); + } +}); + +it("recursion throws stack overflow at entry point", () => { + const result = Bun.spawnSync({ + cmd: [bunExe(), "--preload=./plugin-recursive-fixture.ts", "plugin-recursive-fixture-run.ts"], + stdout: "pipe", + stderr: "pipe", + env: bunEnv, + cwd: import.meta.dir, + }); + + expect(result.stderr.toString()).toContain("RangeError: Maximum call stack size exceeded."); +});