From fefc1931b980c62c5d2533388732f31bd25ae1a9 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 10 Jun 2026 12:33:22 +0200 Subject: [PATCH] webapi: address DataTransfer review feedback - DataTransferItem/DataTransferItemList/Iterator forward acquireRef/releaseRef to the owning DataTransfer, so its pooled arena outlives any JS-held wrapper (fixes a potential use-after-free when JS keeps an item/list/iterator alive past the DataTransfer). - addItem reads strings straight into the persistent arena via toStringSliceWithAlloc, removing the intermediate call_arena copy. - normalizeFormat uses std.ascii.allocLowerString instead of dupe + lowerString. - setData: rename dup -> owned_data for clarity. --- src/browser/webapi/DataTransfer.zig | 16 +++++++--------- src/browser/webapi/DataTransferItem.zig | 14 ++++++++++++++ src/browser/webapi/DataTransferItemList.zig | 17 +++++++++++++++++ 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/browser/webapi/DataTransfer.zig b/src/browser/webapi/DataTransfer.zig index 28bc78a6..d53eb07d 100644 --- a/src/browser/webapi/DataTransfer.zig +++ b/src/browser/webapi/DataTransfer.zig @@ -98,8 +98,7 @@ fn normalizeFormat(arena: Allocator, format: []const u8) ![]const u8 { if (std.ascii.eqlIgnoreCase(format, "url")) { return "text/uri-list"; } - const buf = try arena.dupe(u8, format); - return std.ascii.lowerString(buf, buf); + return std.ascii.allocLowerString(arena, format); } pub fn getData(self: *const DataTransfer, format: []const u8, frame: *Frame) ![]const u8 { @@ -114,15 +113,15 @@ pub fn getData(self: *const DataTransfer, format: []const u8, frame: *Frame) ![] pub fn setData(self: *DataTransfer, format: []const u8, data: []const u8) !void { const norm = try normalizeFormat(self._arena, format); - const dup = try self._arena.dupe(u8, data); + const owned_data = try self._arena.dupe(u8, data); for (self._items.items) |it| { if (it._kind == .string and std.mem.eql(u8, it._type, norm)) { - it._payload = .{ .string = dup }; + it._payload = .{ .string = owned_data }; return; } } const it = try self._arena.create(DataTransferItem); - it.* = .{ ._kind = .string, ._type = norm, ._payload = .{ .string = dup } }; + it.* = .{ ._data_transfer = self, ._kind = .string, ._type = norm, ._payload = .{ .string = owned_data } }; try self._items.append(self._arena, it); } @@ -159,11 +158,10 @@ pub fn addItem(self: *DataTransfer, data: js.Value, type_: ?[]const u8, frame: * return try self.addFileItem(file, frame); } else |_| {} - const s = try data.toZig([]const u8); + const owned_data = try data.toStringSliceWithAlloc(self._arena); const norm = try normalizeFormat(self._arena, type_ orelse ""); - const dup = try self._arena.dupe(u8, s); const it = try self._arena.create(DataTransferItem); - it.* = .{ ._kind = .string, ._type = norm, ._payload = .{ .string = dup } }; + it.* = .{ ._data_transfer = self, ._kind = .string, ._type = norm, ._payload = .{ .string = owned_data } }; try self._items.append(self._arena, it); return it; } @@ -171,7 +169,7 @@ pub fn addItem(self: *DataTransfer, data: js.Value, type_: ?[]const u8, frame: * fn addFileItem(self: *DataTransfer, file: *File, frame: *Frame) !*DataTransferItem { file._proto.acquireRef(); const it = try self._arena.create(DataTransferItem); - it.* = .{ ._kind = .file, ._type = file._proto.getType(), ._payload = .{ .file = file } }; + it.* = .{ ._data_transfer = self, ._kind = .file, ._type = file._proto.getType(), ._payload = .{ .file = file } }; try self._items.append(self._arena, it); try self.rebuildFiles(frame); return it; diff --git a/src/browser/webapi/DataTransferItem.zig b/src/browser/webapi/DataTransferItem.zig index 2ee1eaa8..a3c973fe 100644 --- a/src/browser/webapi/DataTransferItem.zig +++ b/src/browser/webapi/DataTransferItem.zig @@ -22,6 +22,8 @@ const lp = @import("lightpanda"); const js = @import("../js/js.zig"); const File = @import("File.zig"); +const Page = @import("../Page.zig"); +const DataTransfer = @import("DataTransfer.zig"); const log = lp.log; @@ -30,6 +32,10 @@ const DataTransferItem = @This(); pub const Kind = enum { string, file }; +// The owning DataTransfer. The item lives in that DataTransfer's arena and is +// handed to JS, so it forwards acquireRef/releaseRef to keep the arena alive as +// long as JS holds the item. +_data_transfer: *DataTransfer, _kind: Kind, // For string items: the normalized format (e.g. "text/plain"). // For file items: the File's MIME type. @@ -41,6 +47,14 @@ pub const Payload = union(Kind) { file: *File, }; +pub fn acquireRef(self: *DataTransferItem) void { + self._data_transfer.acquireRef(); +} + +pub fn releaseRef(self: *DataTransferItem, page: *Page) void { + self._data_transfer.releaseRef(page); +} + pub fn getKind(self: *const DataTransferItem) []const u8 { return switch (self._kind) { .string => "string", diff --git a/src/browser/webapi/DataTransferItemList.zig b/src/browser/webapi/DataTransferItemList.zig index a69af1ea..92fb6c7b 100644 --- a/src/browser/webapi/DataTransferItemList.zig +++ b/src/browser/webapi/DataTransferItemList.zig @@ -18,6 +18,7 @@ const js = @import("../js/js.zig"); const Frame = @import("../Frame.zig"); +const Page = @import("../Page.zig"); const DataTransfer = @import("DataTransfer.zig"); const DataTransferItem = @import("DataTransferItem.zig"); @@ -30,6 +31,14 @@ const DataTransferItemList = @This(); _data_transfer: *DataTransfer, +pub fn acquireRef(self: *DataTransferItemList) void { + self._data_transfer.acquireRef(); +} + +pub fn releaseRef(self: *DataTransferItemList, page: *Page) void { + self._data_transfer.releaseRef(page); +} + pub fn getLength(self: *const DataTransferItemList) u32 { return @intCast(self._data_transfer._items.items.len); } @@ -69,6 +78,14 @@ pub const Iterator = GenericIterator(struct { index: u32, list: *DataTransferItemList, + pub fn acquireRef(self: *@This()) void { + self.list.acquireRef(); + } + + pub fn releaseRef(self: *@This(), page: *Page) void { + self.list.releaseRef(page); + } + pub fn next(self: *@This(), _: *const js.Execution) ?*DataTransferItem { const index = self.index; const it = self.list.item(index) orelse return null;