Skip to content

Commit

Permalink
fix memory leak in ERR_INVALID_ARG_TYPE (#17169)
Browse files Browse the repository at this point in the history
  • Loading branch information
nektro authored Feb 11, 2025
1 parent c2edbe8 commit 7adb2b9
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 26 deletions.
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

0 comments on commit 7adb2b9

Please sign in to comment.