Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions src/jsc/VirtualMachine.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1892,6 +1892,7 @@ pub fn resolveMaybeNeedsTrailingSlash(
error.NameTooLong,
if (is_esm) .stmt else if (is_user_require_resolve) .require_resolve else .require,
) catch |err| bun.handleOom(err);
defer bun.default_allocator.free(printed);
const msg = logger.Msg{
.data = logger.rangeData(
null,
Expand Down Expand Up @@ -1955,6 +1956,8 @@ pub fn resolveMaybeNeedsTrailingSlash(
}
jsc_vm._resolve(&result, specifier_utf8.slice(), normalizeSource(source_utf8.slice()), is_esm, is_a_file_path) catch |err_| {
var err = err_;
var printed: []const u8 = "";
defer if (printed.len > 0) jsc_vm.allocator.free(printed);
const msg: logger.Msg = brk: {
const msgs: []logger.Msg = log.msgs.items;

Expand All @@ -1972,7 +1975,7 @@ pub fn resolveMaybeNeedsTrailingSlash(
else
.require;

const printed = try bun.api.ResolveMessage.fmt(
printed = try bun.api.ResolveMessage.fmt(
jsc_vm.allocator,
specifier_utf8.slice(),
source_utf8.slice(),
Expand All @@ -1995,7 +1998,7 @@ pub fn resolveMaybeNeedsTrailingSlash(
};

{
res.* = ErrorableString.err(err, (try bun.api.ResolveMessage.create(global, VirtualMachine.get().allocator, msg, source_utf8.slice())));
res.* = ErrorableString.err(err, (try bun.api.ResolveMessage.create(global, jsc_vm.allocator, msg, source_utf8.slice())));
}

return;
Expand Down Expand Up @@ -2032,21 +2035,12 @@ pub fn drainMicrotasks(this: *VirtualMachine) void {
pub fn processFetchLog(globalThis: *JSGlobalObject, specifier: bun.String, referrer: bun.String, log: *logger.Log, ret: *ErrorableResolvedSource, err: anyerror) void {
switch (log.msgs.items.len) {
0 => {
const msg: logger.Msg = brk: {
if (err == error.UnexpectedPendingResolution) {
break :brk logger.Msg{
.data = logger.rangeData(
null,
logger.Range.None,
std.fmt.allocPrint(globalThis.allocator(), "Unexpected pending import in \"{f}\". To automatically install npm packages with Bun, please use an import statement instead of require() or dynamic import().\nThis error can also happen if dependencies import packages which are not referenced anywhere. Worst case, run `bun install` and opt-out of the node_modules folder until we come up with a better way to handle this error.", .{specifier}) catch unreachable,
),
};
}

break :brk logger.Msg{
.data = logger.rangeData(null, logger.Range.None, std.fmt.allocPrint(globalThis.allocator(), "{s} while building {f}", .{ @errorName(err), specifier }) catch unreachable),
};
};
const text = if (err == error.UnexpectedPendingResolution)
std.fmt.allocPrint(globalThis.allocator(), "Unexpected pending import in \"{f}\". To automatically install npm packages with Bun, please use an import statement instead of require() or dynamic import().\nThis error can also happen if dependencies import packages which are not referenced anywhere. Worst case, run `bun install` and opt-out of the node_modules folder until we come up with a better way to handle this error.", .{specifier}) catch unreachable
else
std.fmt.allocPrint(globalThis.allocator(), "{s} while building {f}", .{ @errorName(err), specifier }) catch unreachable;
defer globalThis.allocator().free(text);
const msg: logger.Msg = .{ .data = logger.rangeData(null, logger.Range.None, text) };
{
ret.* = ErrorableResolvedSource.err(err, (bun.api.BuildMessage.create(globalThis, globalThis.allocator(), msg) catch |e| globalThis.takeException(e)));
}
Expand Down
32 changes: 32 additions & 0 deletions test/js/bun/resolve/resolve-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,38 @@ describe("ResolveMessage", () => {
}
expect().pass();
});

it("does not leak the formatted message on the bare-package error path", () => {
// resolveMaybeNeedsTrailingSlash() allocPrints a "Cannot find package
// '...' from '...'" message into the VM arena before handing it to
// ResolveMessage.create(), which immediately clones it. The original
// was never freed, so every failed bare-specifier resolve leaked one
// string into the same mimalloc heap that ResolveMessage structs are
// freed back into. In a long-running Fuzzilli REPRL process this
// surfaced as a flaky use-after-poison in the resolver.
for (let i = 0; i < 50; i++) {
let errs: any[] = [];
for (let j = 0; j < 5; j++) {
try {
require("804");
} catch (e) {
errs.push(e);
}
}
for (const e of errs) {
void e.message;
void e.code;
void e.specifier;
void e.referrer;
void e.level;
void e.importKind;
void String(e);
}
errs = [];
Bun.gc(true);
}
expect().pass();
});
});

// These tests reproduce panics where the module resolver wrote past fixed-size
Expand Down
Loading