Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix memory leak in ERR_INVALID_ARG_TYPE #17169

Merged
merged 5 commits into from
Feb 11, 2025
Merged
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
11 changes: 6 additions & 5 deletions src/bun.js/api/BunObject.zig
Original file line number Diff line number Diff line change
Expand Up @@ -533,17 +533,18 @@ pub fn inspect(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.J
return ret;
}

export fn Bun__inspect(globalThis: *JSGlobalObject, value: JSValue) ZigString {
export fn Bun__inspect(globalThis: *JSGlobalObject, value: JSValue) bun.String {
// very stable memory address
var array = MutableString.init(getAllocator(globalThis), 0) catch unreachable;
defer array.deinit();
var buffered_writer = MutableString.BufferedWriter{ .context = &array };
const writer = buffered_writer.writer();

var formatter = ConsoleObject.Formatter{ .globalThis = globalThis };
writer.print("{}", .{value.toFmt(&formatter)}) catch return ZigString.Empty;
buffered_writer.flush() catch return ZigString.Empty;

return ZigString.init(array.slice()).withEncoding();
defer formatter.deinit();
writer.print("{}", .{value.toFmt(&formatter)}) catch return .empty;
buffered_writer.flush() catch return .empty;
return bun.String.createUTF8(array.slice());
}

pub fn getInspect(globalObject: *JSC.JSGlobalObject, _: *JSC.JSObject) JSC.JSValue {
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/api/bun/spawn/stdio.zig
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ pub const Stdio = union(enum) {

if (file_fd >= std.math.maxInt(i32)) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
return globalThis.throwInvalidArguments("file descriptor must be a valid integer, received: {}", .{value.toFmt(&formatter)});
}

Expand Down
1 change: 1 addition & 0 deletions src/bun.js/api/server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1970,6 +1970,7 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
.globalThis = globalThis,
.quote_strings = true,
};
defer formatter.deinit();
Output.errGeneric("Expected a Response object, but received '{}'", .{value.toFmt(&formatter)});
} else {
Output.errGeneric("Expected a Response object", .{});
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/base.zig
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ pub fn getAllocator(_: js.JSContextRef) std.mem.Allocator {
/// Print a JSValue to stdout; this is only meant for debugging purposes
pub fn dump(value: JSC.WebCore.JSValue, globalObject: *JSC.JSGlobalObject) !void {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalObject };
defer formatter.deinit();
try Output.errorWriter().print("{}\n", .{value.toFmt(globalObject, &formatter)});
Output.flush();
}
Expand Down
8 changes: 3 additions & 5 deletions src/bun.js/bindings/ErrorCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ JSObject* createError(Zig::JSGlobalObject* globalObject, ErrorCode code, JSC::JS
return createError(vm, globalObject, code, message, isDOMExceptionPrototype);
}

// export fn Bun__inspect(globalThis: *JSGlobalObject, value: JSValue) ZigString
extern "C" ZigString Bun__inspect(JSC::JSGlobalObject* globalObject, JSValue value);
// export fn Bun__inspect(globalThis: *JSGlobalObject, value: JSValue) bun.String
extern "C" BunString Bun__inspect(JSC::JSGlobalObject* globalObject, JSValue value);

//
WTF::String JSValueToStringSafe(JSC::JSGlobalObject* globalObject, JSValue arg)
Expand Down Expand Up @@ -247,9 +247,7 @@ WTF::String JSValueToStringSafe(JSC::JSGlobalObject* globalObject, JSValue arg)
}
}

ZigString zstring = Bun__inspect(globalObject, arg);
BunString bstring(BunStringTag::ZigString, BunStringImpl(zstring));
return bstring.toWTFString();
return Bun__inspect(globalObject, arg).transferToWTFString();
}

WTF::String determineSpecificType(JSC::JSGlobalObject* globalObject, JSValue value)
Expand Down
2 changes: 2 additions & 0 deletions src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6647,6 +6647,7 @@ pub fn toJSHostFunction(comptime Function: JSHostZigFunction) JSC.JSHostFunction
if (value != .zero) {
if (globalThis.hasException()) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
bun.Output.err("Assertion failed",
\\Native function returned a non-zero JSValue while an exception is pending
\\
Expand Down Expand Up @@ -6683,6 +6684,7 @@ pub fn toJSHostFunctionWithContext(comptime ContextType: type, comptime Function
if (value != .zero) {
if (globalThis.hasException()) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
bun.Output.err("Assertion failed",
\\Native function returned a non-zero JSValue while an exception is pending
\\
Expand Down
2 changes: 2 additions & 0 deletions src/bun.js/node/node_cluster_binding.zig
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub fn sendHelperChild(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFram

// similar code as Bun__Process__send
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
if (Environment.isDebug) log("child: {}", .{message.toFmt(&formatter)});

const ipc_instance = vm.getIPCInstance().?;
Expand Down Expand Up @@ -205,6 +206,7 @@ pub fn sendHelperPrimary(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFr

// similar code as bun.JSC.Subprocess.doSend
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
if (Environment.isDebug) log("primary: {}", .{message.toFmt(&formatter)});

_ = handle;
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/node/types.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ pub fn modeFromJS(ctx: JSC.C.JSContextRef, value: JSC.JSValue) bun.JSError!?Mode

break :brk std.fmt.parseInt(Mode, slice, 8) catch {
var formatter = bun.JSC.ConsoleObject.Formatter{ .globalThis = ctx };
defer formatter.deinit();
return ctx.throwValue(ctx.ERR_INVALID_ARG_VALUE("The argument 'mode' must be a 32-bit unsigned integer or an octal string. Received {}", .{value.toFmt(&formatter)}).toJS());
};
};
Expand Down
4 changes: 4 additions & 0 deletions src/bun.js/node/util/validators.zig
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,14 @@ pub fn validateInt32(globalThis: *JSGlobalObject, value: JSValue, comptime name_
}
if (!value.isAnyInt()) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
return throwRangeError(globalThis, "The value of \"" ++ name_fmt ++ "\" is out of range. It must be an integer. Received {}", name_args ++ .{value.toFmt(&formatter)});
}
const num = value.asNumber();
// Use floating point comparison here to ensure values out of i32 range get caught instead of clamp/truncated.
if (num < @as(f64, @floatFromInt(min)) or num > @as(f64, @floatFromInt(max))) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
return throwRangeError(globalThis, "The value of \"" ++ name_fmt ++ "\" is out of range. It must be >= {d} and <= {d}. Received {}", name_args ++ .{ min, max, value.toFmt(&formatter) });
}
return @intFromFloat(num);
Expand All @@ -129,13 +131,15 @@ pub fn validateUint32(globalThis: *JSGlobalObject, value: JSValue, comptime name
}
if (!value.isAnyInt()) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
return throwRangeError(globalThis, "The value of \"" ++ name_fmt ++ "\" is out of range. It must be an integer. Received {}", name_args ++ .{value.toFmt(&formatter)});
}
const num: i64 = value.asInt52();
const min: i64 = if (greater_than_zero) 1 else 0;
const max: i64 = @intCast(std.math.maxInt(u32));
if (num < min or num > max) {
var formatter = JSC.ConsoleObject.Formatter{ .globalThis = globalThis };
defer formatter.deinit();
return throwRangeError(globalThis, "The value of \"" ++ name_fmt ++ "\" is out of range. It must be >= {d} and <= {d}. Received {}", name_args ++ .{ min, max, value.toFmt(&formatter) });
}
return @truncate(@as(u63, @intCast(num)));
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/test/diff_format.zig
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ pub const DiffFormatter = struct {
.none => {
const fmt = "Expected: <green>{any}<r>\nReceived: <red>{any}<r>";
var formatter = ConsoleObject.Formatter{ .globalThis = this.globalThis, .quote_strings = true };
defer formatter.deinit();
if (Output.enable_ansi_colors) {
try writer.print(Output.prettyFmt(fmt, true), .{
expected.toFmt(&formatter),
Expand Down
Loading