Skip to content

Commit

Permalink
Fix argument validation with callbacks in node:fs (#12412)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner authored Jul 8, 2024
1 parent 150ae03 commit c2a5451
Show file tree
Hide file tree
Showing 7 changed files with 424 additions and 107 deletions.
24 changes: 18 additions & 6 deletions src/bun.js/event_loop.zig
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,11 @@ pub const JSCScheduler = struct {
export fn Bun__queueJSCDeferredWorkTaskConcurrently(jsc_vm: *VirtualMachine, task: *JSCScheduler.JSCDeferredWorkTask) void {
JSC.markBinding(@src());
var loop = jsc_vm.eventLoop();
var concurrent_task = bun.default_allocator.create(ConcurrentTask) catch bun.outOfMemory();
loop.enqueueTaskConcurrent(concurrent_task.from(task, .auto_deinit));
loop.enqueueTaskConcurrent(ConcurrentTask.new(.{
.task = Task.init(task),
.next = null,
.auto_delete = true,
}));
}

comptime {
Expand Down Expand Up @@ -1256,8 +1259,7 @@ pub const EventLoop = struct {
_ = this.tickConcurrentWithCount();
}

pub fn tickConcurrentWithCount(this: *EventLoop) usize {
JSC.markBinding(@src());
fn updateCounts(this: *EventLoop) void {
const delta = this.concurrent_ref.swap(0, .monotonic);
const loop = this.virtual_machine.event_loop_handle.?;
if (comptime Environment.isWindows) {
Expand All @@ -1275,6 +1277,10 @@ pub const EventLoop = struct {
loop.active -= @intCast(-delta);
}
}
}

pub fn tickConcurrentWithCount(this: *EventLoop) usize {
this.updateCounts();

var concurrent = this.concurrent_tasks.popBatch();
const count = concurrent.count;
Expand Down Expand Up @@ -1575,25 +1581,31 @@ pub const EventLoop = struct {
}
}
pub fn enqueueTaskConcurrent(this: *EventLoop, task: *ConcurrentTask) void {
JSC.markBinding(@src());
if (comptime Environment.allow_assert) {
if (this.virtual_machine.has_terminated) {
@panic("EventLoop.enqueueTaskConcurrent: VM has terminated");
}
}

if (comptime Environment.isDebug) {
log("enqueueTaskConcurrent({s})", .{task.task.typeName() orelse "[unknown]"});
}

this.concurrent_tasks.push(task);
this.wakeup();
}

pub fn enqueueTaskConcurrentBatch(this: *EventLoop, batch: ConcurrentTask.Queue.Batch) void {
JSC.markBinding(@src());
if (comptime Environment.allow_assert) {
if (this.virtual_machine.has_terminated) {
@panic("EventLoop.enqueueTaskConcurrent: VM has terminated");
}
}

if (comptime Environment.isDebug) {
log("enqueueTaskConcurrentBatch({d})", .{batch.count});
}

this.concurrent_tasks.pushBatch(batch.front.?, batch.last.?, batch.count);
this.wakeup();
}
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/node/node_fs_stat_watcher.zig
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ const log = bun.Output.scoped(.StatWatcher, false);

fn statToJSStats(globalThis: *JSC.JSGlobalObject, stats: bun.Stat, bigint: bool) JSC.JSValue {
if (bigint) {
return bun.new(StatsBig, StatsBig.init(stats)).toJS(globalThis);
return StatsBig.new(StatsBig.init(stats)).toJS(globalThis);
} else {
return bun.new(StatsSmall, StatsSmall.init(stats)).toJS(globalThis);
return StatsSmall.new(StatsSmall.init(stats)).toJS(globalThis);
}
}

Expand Down
15 changes: 5 additions & 10 deletions src/bun.js/node/types.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,7 @@ pub fn StatType(comptime Big: bool) type {

return extern struct {
pub usingnamespace if (Big) JSC.Codegen.JSBigIntStats else JSC.Codegen.JSStats;
pub usingnamespace bun.New(@This());

// Stats stores these as i32, but BigIntStats stores all of these as i64
// On windows, these two need to be u64 as the numbers are often very large.
Expand Down Expand Up @@ -1555,7 +1556,7 @@ pub fn StatType(comptime Big: bool) type {
// TODO: BigIntStats includes a `_checkModeProperty` but I dont think anyone actually uses it.

pub fn finalize(this: *This) callconv(.C) void {
bun.destroy(this);
this.destroy();
}

pub fn init(stat_: bun.Stat) This {
Expand Down Expand Up @@ -1588,12 +1589,6 @@ pub fn StatType(comptime Big: bool) type {
};
}

pub fn initWithAllocator(allocator: std.mem.Allocator, stat: bun.Stat) *This {
const this = allocator.create(This) catch bun.outOfMemory();
this.* = init(stat);
return this;
}

pub fn constructor(globalObject: *JSC.JSGlobalObject, callFrame: *JSC.CallFrame) callconv(JSC.conv) ?*This {
if (Big) {
globalObject.throwInvalidArguments("BigIntStats is not a constructor", .{});
Expand All @@ -1608,7 +1603,7 @@ pub fn StatType(comptime Big: bool) type {
const ctime_ms: f64 = if (args.len > 12 and args[12].isNumber()) args[12].asNumber() else 0;
const birthtime_ms: f64 = if (args.len > 13 and args[13].isNumber()) args[13].asNumber() else 0;

const this = bun.new(This, .{
const this = This.new(.{
.dev = if (args.len > 0 and args[0].isNumber()) @intCast(args[0].toInt32()) else 0,
.mode = if (args.len > 1 and args[1].isNumber()) args[1].toInt32() else 0,
.nlink = if (args.len > 2 and args[2].isNumber()) args[2].toInt32() else 0,
Expand Down Expand Up @@ -1658,8 +1653,8 @@ pub const Stats = union(enum) {

pub fn toJSNewlyCreated(this: *const Stats, globalObject: *JSC.JSGlobalObject) JSC.JSValue {
return switch (this.*) {
.big => bun.new(StatsBig, this.big).toJS(globalObject),
.small => bun.new(StatsSmall, this.small).toJS(globalObject),
.big => StatsBig.new(this.big).toJS(globalObject),
.small => StatsSmall.new(this.small).toJS(globalObject),
};
}

Expand Down
6 changes: 6 additions & 0 deletions src/js/node/fs.promises.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,12 @@ export default exports;
return this[kFd];
}

[kCloseResolve];
[kFd];
[kFlag];
[kClosePromise];
[kRefs];

async appendFile(data, options: object | string | undefined) {
const fd = this[kFd];
throwEBADFIfNecessary(writeFile, fd);
Expand Down
Loading

0 comments on commit c2a5451

Please sign in to comment.