Skip to content

Commit 57918e9

Browse files
francisbouvierfrancisbouvier
committed
cdp: fix memory leak in msg parsing of the JSON
Signed-off-by: Francis Bouvier <[email protected]>
1 parent 1854074 commit 57918e9

File tree

10 files changed

+158
-87
lines changed

10 files changed

+158
-87
lines changed

‎src/cdp/browser.zig‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const server = @import("../server.zig");
2222
const Ctx = server.Ctx;
2323
const cdp = @import("cdp.zig");
2424
const result = cdp.result;
25-
const getMsg = cdp.getMsg;
25+
const Msg = cdp.Msg;
2626

2727
const log = std.log.scoped(.cdp);
2828

@@ -65,7 +65,8 @@ fn getVersion(
6565
) ![]const u8 {
6666

6767
// input
68-
const msg = try getMsg(alloc, _id, void, scanner);
68+
const msg = try Msg(void).get(alloc, _id, scanner);
69+
defer msg.deinit();
6970
log.debug("Req > id {d}, method {s}", .{ msg.id, "browser.getVersion" });
7071

7172
// ouput
@@ -94,7 +95,8 @@ fn setDownloadBehavior(
9495
downloadPath: ?[]const u8 = null,
9596
eventsEnabled: ?bool = null,
9697
};
97-
const msg = try getMsg(alloc, _id, Params, scanner);
98+
const msg = try Msg(Params).get(alloc, _id, scanner);
99+
defer msg.deinit();
98100
log.debug("REQ > id {d}, method {s}", .{ msg.id, "browser.setDownloadBehavior" });
99101

100102
// output
@@ -115,7 +117,8 @@ fn getWindowForTarget(
115117
const Params = struct {
116118
targetId: ?[]const u8 = null,
117119
};
118-
const msg = try cdp.getMsg(alloc, _id, ?Params, scanner);
120+
const msg = try cdp.Msg(?Params).get(alloc, _id, scanner);
121+
defer msg.deinit();
119122
std.debug.assert(msg.sessionID != null);
120123
log.debug("Req > id {d}, method {s}", .{ msg.id, "browser.getWindowForTarget" });
121124

@@ -142,7 +145,8 @@ fn setWindowBounds(
142145
) ![]const u8 {
143146

144147
// input
145-
const msg = try cdp.getMsg(alloc, _id, void, scanner);
148+
const msg = try cdp.Msg(void).get(alloc, _id, scanner);
149+
defer msg.deinit();
146150
log.debug("Req > id {d}, method {s}", .{ msg.id, "browser.setWindowBounds" });
147151

148152
// output

‎src/cdp/cdp.zig‎

Lines changed: 80 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ pub fn sendEvent(
230230
}
231231

232232
fn getParams(
233-
alloc: std.mem.Allocator,
233+
alloc: ?std.mem.Allocator,
234234
comptime T: type,
235235
scanner: *std.json.Scanner,
236236
key: []const u8,
@@ -250,15 +250,17 @@ fn getParams(
250250
}
251251
if (finished == 0) break;
252252
}
253-
return void{};
253+
return null;
254+
} else {
255+
std.debug.assert(alloc != null);
256+
257+
// parse "params"
258+
const options = std.json.ParseOptions{
259+
.max_value_len = scanner.input.len,
260+
.allocate = .alloc_always,
261+
};
262+
return try std.json.innerParse(T, alloc.?, scanner, options);
254263
}
255-
256-
// parse "params"
257-
const options = std.json.ParseOptions{
258-
.max_value_len = scanner.input.len,
259-
.allocate = .alloc_if_needed,
260-
};
261-
return try std.json.innerParse(T, alloc, scanner, options);
262264
}
263265

264266
fn getId(scanner: *std.json.Scanner, key: []const u8) !?u16 {
@@ -279,45 +281,78 @@ fn getSessionId(scanner: *std.json.Scanner, key: []const u8) !?[]const u8 {
279281
return try nextString(scanner);
280282
}
281283

282-
pub fn getMsg(
283-
alloc: std.mem.Allocator,
284-
_id: ?u16,
285-
comptime params_T: type,
286-
scanner: *std.json.Scanner,
287-
) !struct { id: u16, params: ?params_T, sessionID: ?[]const u8 } {
288-
var id_msg: ?u16 = null;
289-
var params: ?params_T = null;
290-
var sessionID: ?[]const u8 = null;
291-
292-
var t: std.json.Token = undefined;
293-
294-
while (true) {
295-
t = try scanner.next();
296-
if (t == .object_end) break;
297-
if (t != .string) {
298-
return error.WrongTokenType;
299-
}
300-
if (_id == null and id_msg == null) {
301-
id_msg = try getId(scanner, t.string);
302-
if (id_msg != null) continue;
303-
}
304-
if (params == null) {
305-
params = try getParams(alloc, params_T, scanner, t.string);
306-
if (params != null) continue;
307-
}
308-
if (sessionID == null) {
309-
sessionID = try getSessionId(scanner, t.string);
310-
}
311-
}
284+
pub fn Msg(T: type) type {
285+
return struct {
286+
arena: ?*std.heap.ArenaAllocator = null,
287+
id: u16,
288+
params: ?T,
289+
sessionID: ?[]const u8,
290+
291+
const Self = @This();
292+
293+
pub fn get(
294+
alloc: std.mem.Allocator,
295+
_id: ?u16,
296+
scanner: *std.json.Scanner,
297+
) !Self {
298+
var arena: ?*std.heap.ArenaAllocator = null;
299+
var allocator: ?std.mem.Allocator = null;
300+
301+
if (T != void) {
302+
arena = try alloc.create(std.heap.ArenaAllocator);
303+
errdefer alloc.destroy(arena.?);
304+
arena.?.* = std.heap.ArenaAllocator.init(alloc);
305+
errdefer arena.?.deinit();
306+
allocator = arena.?.allocator();
307+
}
308+
var id_msg: ?u16 = null;
309+
var params: ?T = null;
310+
var sessionID: ?[]const u8 = null;
311+
312+
var t: std.json.Token = undefined;
313+
314+
while (true) {
315+
t = try scanner.next();
316+
if (t == .object_end) break;
317+
if (t != .string) {
318+
return error.WrongTokenType;
319+
}
320+
if (_id == null and id_msg == null) {
321+
id_msg = try getId(scanner, t.string);
322+
if (id_msg != null) continue;
323+
}
324+
if (params == null) {
325+
params = try getParams(allocator, T, scanner, t.string);
326+
if (params != null) continue;
327+
}
328+
if (sessionID == null) {
329+
sessionID = try getSessionId(scanner, t.string);
330+
}
331+
}
312332

313-
// end
314-
t = try scanner.next();
315-
if (t != .end_of_document) return error.CDPMsgEnd;
333+
// end
334+
t = try scanner.next();
335+
if (t != .end_of_document) return error.CDPMsgEnd;
316336

317-
// check id
318-
if (_id == null and id_msg == null) return error.RequestWithoutID;
337+
// check id
338+
if (_id == null and id_msg == null) return error.RequestWithoutID;
319339

320-
return .{ .id = _id orelse id_msg.?, .params = params, .sessionID = sessionID };
340+
return .{
341+
.arena = arena,
342+
.id = _id orelse id_msg.?,
343+
.params = params,
344+
.sessionID = sessionID,
345+
};
346+
}
347+
348+
pub fn deinit(self: Self) void {
349+
if (self.arena) |arena| {
350+
const allocator = arena.child_allocator;
351+
arena.deinit();
352+
allocator.destroy(arena);
353+
}
354+
}
355+
};
321356
}
322357

323358
// Common

‎src/cdp/emulation.zig‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const server = @import("../server.zig");
2222
const Ctx = server.Ctx;
2323
const cdp = @import("cdp.zig");
2424
const result = cdp.result;
25-
const getMsg = cdp.getMsg;
25+
const Msg = cdp.Msg;
2626
const stringify = cdp.stringify;
2727

2828
const log = std.log.scoped(.cdp);
@@ -69,7 +69,8 @@ fn setEmulatedMedia(
6969
media: ?[]const u8 = null,
7070
features: ?[]MediaFeature = null,
7171
};
72-
const msg = try getMsg(alloc, _id, Params, scanner);
72+
const msg = try Msg(Params).get(alloc, _id, scanner);
73+
defer msg.deinit();
7374
log.debug("Req > id {d}, method {s}", .{ msg.id, "emulation.setEmulatedMedia" });
7475

7576
// output
@@ -88,7 +89,8 @@ fn setFocusEmulationEnabled(
8889
const Params = struct {
8990
enabled: bool,
9091
};
91-
const msg = try getMsg(alloc, _id, Params, scanner);
92+
const msg = try Msg(Params).get(alloc, _id, scanner);
93+
defer msg.deinit();
9294
log.debug("Req > id {d}, method {s}", .{ msg.id, "emulation.setFocusEmulationEnabled" });
9395

9496
// output
@@ -104,7 +106,8 @@ fn setDeviceMetricsOverride(
104106
) ![]const u8 {
105107

106108
// input
107-
const msg = try cdp.getMsg(alloc, _id, void, scanner);
109+
const msg = try cdp.Msg(void).get(alloc, _id, scanner);
110+
defer msg.deinit();
108111
log.debug("Req > id {d}, method {s}", .{ msg.id, "emulation.setDeviceMetricsOverride" });
109112

110113
// output
@@ -118,7 +121,8 @@ fn setTouchEmulationEnabled(
118121
scanner: *std.json.Scanner,
119122
_: *Ctx,
120123
) ![]const u8 {
121-
const msg = try cdp.getMsg(alloc, _id, void, scanner);
124+
const msg = try cdp.Msg(void).get(alloc, _id, scanner);
125+
defer msg.deinit();
122126
log.debug("Req > id {d}, method {s}", .{ msg.id, "emulation.setTouchEmulationEnabled" });
123127

124128
return result(alloc, msg.id, null, null, msg.sessionID);

‎src/cdp/fetch.zig‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const server = @import("../server.zig");
2222
const Ctx = server.Ctx;
2323
const cdp = @import("cdp.zig");
2424
const result = cdp.result;
25-
const getMsg = cdp.getMsg;
25+
const Msg = cdp.Msg;
2626

2727
const log = std.log.scoped(.cdp);
2828

@@ -52,7 +52,8 @@ fn disable(
5252
scanner: *std.json.Scanner,
5353
_: *Ctx,
5454
) ![]const u8 {
55-
const msg = try getMsg(alloc, _id, void, scanner);
55+
const msg = try Msg(void).get(alloc, _id, scanner);
56+
defer msg.deinit();
5657
log.debug("Req > id {d}, method {s}", .{ msg.id, "fetch.disable" });
5758

5859
return result(alloc, msg.id, null, null, msg.sessionID);

‎src/cdp/log.zig‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const server = @import("../server.zig");
2222
const Ctx = server.Ctx;
2323
const cdp = @import("cdp.zig");
2424
const result = cdp.result;
25-
const getMsg = cdp.getMsg;
25+
const Msg = cdp.Msg;
2626
const stringify = cdp.stringify;
2727

2828
const log_cdp = std.log.scoped(.cdp);
@@ -52,7 +52,8 @@ fn enable(
5252
scanner: *std.json.Scanner,
5353
_: *Ctx,
5454
) ![]const u8 {
55-
const msg = try getMsg(alloc, _id, void, scanner);
55+
const msg = try Msg(void).get(alloc, _id, scanner);
56+
defer msg.deinit();
5657
log_cdp.debug("Req > id {d}, method {s}", .{ msg.id, "log.enable" });
5758

5859
return result(alloc, msg.id, null, null, msg.sessionID);

‎src/cdp/network.zig‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const server = @import("../server.zig");
2222
const Ctx = server.Ctx;
2323
const cdp = @import("cdp.zig");
2424
const result = cdp.result;
25-
const getMsg = cdp.getMsg;
25+
const Msg = cdp.Msg;
2626

2727
const log = std.log.scoped(.cdp);
2828

@@ -55,7 +55,8 @@ fn enable(
5555
) ![]const u8 {
5656

5757
// input
58-
const msg = try getMsg(alloc, _id, void, scanner);
58+
const msg = try Msg(void).get(alloc, _id, scanner);
59+
defer msg.deinit();
5960
log.debug("Req > id {d}, method {s}", .{ msg.id, "network.enable" });
6061

6162
return result(alloc, msg.id, null, null, msg.sessionID);
@@ -70,7 +71,8 @@ fn setCacheDisabled(
7071
) ![]const u8 {
7172

7273
// input
73-
const msg = try getMsg(alloc, _id, void, scanner);
74+
const msg = try Msg(void).get(alloc, _id, scanner);
75+
defer msg.deinit();
7476
log.debug("Req > id {d}, method {s}", .{ msg.id, "network.setCacheDisabled" });
7577

7678
return result(alloc, msg.id, null, null, msg.sessionID);

‎src/cdp/page.zig‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const server = @import("../server.zig");
2222
const Ctx = server.Ctx;
2323
const cdp = @import("cdp.zig");
2424
const result = cdp.result;
25-
const getMsg = cdp.getMsg;
25+
const Msg = cdp.Msg;
2626
const stringify = cdp.stringify;
2727
const sendEvent = cdp.sendEvent;
2828

@@ -66,7 +66,8 @@ fn enable(
6666
) ![]const u8 {
6767

6868
// input
69-
const msg = try getMsg(alloc, _id, void, scanner);
69+
const msg = try Msg(void).get(alloc, _id, scanner);
70+
defer msg.deinit();
7071
log.debug("Req > id {d}, method {s}", .{ msg.id, "page.enable" });
7172

7273
return result(alloc, msg.id, null, null, msg.sessionID);
@@ -95,7 +96,8 @@ fn getFrameTree(
9596
) ![]const u8 {
9697

9798
// input
98-
const msg = try cdp.getMsg(alloc, _id, void, scanner);
99+
const msg = try cdp.Msg(void).get(alloc, _id, scanner);
100+
defer msg.deinit();
99101
log.debug("Req > id {d}, method {s}", .{ msg.id, "page.getFrameTree" });
100102

101103
// output
@@ -149,7 +151,8 @@ fn setLifecycleEventsEnabled(
149151
const Params = struct {
150152
enabled: bool,
151153
};
152-
const msg = try getMsg(alloc, _id, Params, scanner);
154+
const msg = try Msg(Params).get(alloc, _id, scanner);
155+
defer msg.deinit();
153156
log.debug("Req > id {d}, method {s}", .{ msg.id, "page.setLifecycleEventsEnabled" });
154157

155158
ctx.state.page_life_cycle_events = true;
@@ -180,7 +183,8 @@ fn addScriptToEvaluateOnNewDocument(
180183
includeCommandLineAPI: bool = false,
181184
runImmediately: bool = false,
182185
};
183-
const msg = try getMsg(alloc, _id, Params, scanner);
186+
const msg = try Msg(Params).get(alloc, _id, scanner);
187+
defer msg.deinit();
184188
log.debug("Req > id {d}, method {s}", .{ msg.id, "page.addScriptToEvaluateOnNewDocument" });
185189

186190
// output
@@ -216,7 +220,8 @@ fn createIsolatedWorld(
216220
worldName: []const u8,
217221
grantUniveralAccess: bool,
218222
};
219-
const msg = try getMsg(alloc, _id, Params, scanner);
223+
const msg = try Msg(Params).get(alloc, _id, scanner);
224+
defer msg.deinit();
220225
std.debug.assert(msg.sessionID != null);
221226
log.debug("Req > id {d}, method {s}", .{ msg.id, "page.createIsolatedWorld" });
222227
const params = msg.params.?;
@@ -261,7 +266,8 @@ fn navigate(
261266
frameId: ?[]const u8 = null,
262267
referrerPolicy: ?[]const u8 = null, // TODO: enum
263268
};
264-
const msg = try getMsg(alloc, _id, Params, scanner);
269+
const msg = try Msg(Params).get(alloc, _id, scanner);
270+
defer msg.deinit();
265271
std.debug.assert(msg.sessionID != null);
266272
log.debug("Req > id {d}, method {s}", .{ msg.id, "page.navigate" });
267273
const params = msg.params.?;

0 commit comments

Comments
 (0)