From 4e11cc9ea10ec70a5e53ee2b24eb495db659e955 Mon Sep 17 00:00:00 2001 From: Jacob Jonsson Date: Wed, 11 Mar 2026 00:48:02 +0100 Subject: [PATCH] fix: make admin command parsing more robust There was a bug in how we parsed admin commands, apparently we never tested if we could parse `!backlog X` or unknown admin commands. This commit also make updates to the backlog command construction to make sure that we don't try to access messages in the backlog that don't exist. --- src/bot.zig | 4 ++-- src/commands.zig | 35 ++++++++++++++++++++-------- src/main.zig | 60 ++++++++++++++++++++++++++++-------------------- src/parser.zig | 41 +++++++++++++++++++++++++++++---- 4 files changed, 100 insertions(+), 40 deletions(-) diff --git a/src/bot.zig b/src/bot.zig index a0ed0c2..a6da684 100644 --- a/src/bot.zig +++ b/src/bot.zig @@ -140,10 +140,10 @@ pub const Bot = struct { return .{ .JOIN = .{ .prefix = prefix, .channels = msg.channel } }; }, .backlog => |backlog| { - if (self.backlog.insertions == 0) { + if (self.backlog.len() == 0) { return Error.NoMessage; } - if (backlog.history > self.backlog.len()) { + if (backlog.history >= self.backlog.len()) { return Error.NoMessage; } if (self.backlog.get_backwards(backlog.history)) |message| { diff --git a/src/commands.zig b/src/commands.zig index 3f2dfb3..6a57a73 100644 --- a/src/commands.zig +++ b/src/commands.zig @@ -55,18 +55,22 @@ pub const AdminCommand = union(enum) { if (command.consume_str("status")) |_| { return .status; } - if (command.consume_str("join").?.consume_char(' ')) |join| { - if (join.rest[0] != '#') { - return .{ .err = .{ .message = "channels must start with \"#\"" } }; + if (command.consume_str("join")) |join| { + if (join.consume_space()) |channel| { + if (channel.rest[0] != '#') { + return .{ .err = .{ .message = "channels must start with \"#\"" } }; + } + return .{ .join = .{ .channel = join.rest } }; } - return .{ .join = .{ .channel = join.rest } }; } if (command.consume_str("backlog")) |backlog| { - const history = std.fmt.parseInt(u16, backlog.rest, 10) catch |err| { - std.debug.print("failed to parse int ('{s}') with error: {}\n", .{ backlog.rest, err }); - return null; - }; - return .{ .backlog = .{ .history = history } }; + if (backlog.consume_space()) |history| { + const historyOffset = std.fmt.parseInt(u16, history.rest, 10) catch |err| { + std.debug.print("failed to parse int ('{s}') with error: {}\n", .{ history.rest, err }); + return null; + }; + return .{ .backlog = .{ .history = historyOffset } }; + } } std.log.debug("unknown command: \"{s}\"", .{command.rest}); } @@ -104,3 +108,16 @@ test "parse admin commands" { cmd, ); } + +test "parse backlog admin commands" { + const cmd = AdminCommand.parse("!backlog 1") orelse unreachable; + try std.testing.expectEqual( + AdminCommand{ .backlog = .{ .history = 1 } }, + cmd, + ); +} + +test "parse unknown admin commands" { + const cmd = AdminCommand.parse("!history 1"); + try std.testing.expectEqual(null, cmd); +} diff --git a/src/main.zig b/src/main.zig index e200fb8..f484f2e 100644 --- a/src/main.zig +++ b/src/main.zig @@ -120,28 +120,38 @@ pub const BotAdapter = struct { } }; -// test "substitute" { -// var bot_adapter = try BotAdapter.init(std.testing.allocator); -// defer bot_adapter.deinit(); -// const prefix = zircon.Prefix{ .nick = "jassob", .user = "jassob", .host = "localhost" }; -// const msg = zircon.Message{ -// .PRIVMSG = .{ -// .prefix = prefix, -// .targets = "#test", -// .text = "hello world", -// }, -// }; -// if (bot_adapter.callback(msg)) |_| { -// @panic("unexpected response"); -// } -// const cmd_msg = zircon.Message{ -// .PRIVMSG = .{ -// .prefix = prefix, -// .targets = "#test", -// .text = "s/world/zig/", -// }, -// }; -// const response = bot_adapter.callback(cmd_msg); -// try std.testing.expect(response != null); -// try std.testing.expectEqualStrings("jassob: \"hello zig\"", response.?.PRIVMSG.text); -// } +test "substitute" { + var bot_adapter = try BotAdapter.init(std.testing.allocator); + defer bot_adapter.deinit(); + const prefix = zircon.Prefix{ .nick = "jassob", .user = "jassob", .host = "localhost" }; + const msg = zircon.Message{ + .PRIVMSG = .{ + .prefix = prefix, + .targets = "#test", + .text = "hello world", + }, + }; + if (bot_adapter.callback(msg)) |_| { + @panic("unexpected response"); + } + const cmd_msg = zircon.Message{ + .PRIVMSG = .{ + .prefix = prefix, + .targets = "#test", + .text = "s/world/zig/", + }, + }; + const response = bot_adapter.callback(cmd_msg); + try std.testing.expect(response != null); + try std.testing.expectEqualStrings("jassob: \"hello zig\"", response.?.PRIVMSG.text); +} + +test "get empty backlog message" { + var bot_adapter = try BotAdapter.init(std.testing.allocator); + defer bot_adapter.deinit(); + const prefix = zircon.Prefix{ .nick = "jassob", .user = "jassob", .host = "localhost" }; + const msg = zircon.Message{ + .PRIVMSG = .{ .prefix = prefix, .targets = "#eru-admin", .text = "!backlog 0" }, + }; + try std.testing.expectEqualDeep("no matching message", bot_adapter.callback(msg).?.PRIVMSG.text); +} diff --git a/src/parser.zig b/src/parser.zig index 98053e3..13acaea 100644 --- a/src/parser.zig +++ b/src/parser.zig @@ -9,10 +9,7 @@ pub const Parser = struct { rest: []const u8, end_idx: usize, - pub fn seek(self: *const Parser, skip: usize) Parser { - return .{ .original = self.original, .rest = self.rest[skip..], .end_idx = self.end_idx + skip }; - } - + // Initializes a Parser for s. pub fn init(s: []const u8) Parser { return .{ .original = s, @@ -21,6 +18,25 @@ pub const Parser = struct { }; } + // Seek the parser window of the text forward skip bytes and return a new Parser. + pub fn seek(self: *const Parser, skip: usize) Parser { + return .{ .original = self.original, .rest = self.rest[skip..], .end_idx = self.end_idx + skip }; + } + + // Attempts to consume at least one whitespace character from the input text. + pub fn consume_space(self: *const Parser) ?Parser { + if (!std.ascii.isWhitespace(self.rest[0])) { + return null; + } + for (self.rest[1..], 1..) |c, idx| { + if (!std.ascii.isWhitespace(c)) { + return self.seek(idx); + } + } + return self.seek(self.rest.len); + } + + // Attempts to consume a character c. pub fn consume_char(self: *const Parser, c: u8) ?Parser { if (self.rest[0] != c) { return null; @@ -28,6 +44,7 @@ pub const Parser = struct { return self.seek(1); } + // Attempts to consume a string s. pub fn consume_str(self: *const Parser, s: []const u8) ?Parser { const len = s.len; if (self.rest.len < len) { @@ -39,16 +56,32 @@ pub const Parser = struct { return self.seek(len); } + // Finds the next occurrence of c (idx) in the current parser + // window and extracts it. + // + // Returns a new parser window that starts after idx and the + // extracted byte slice. pub fn take_until_char(self: *const Parser, c: u8) struct { Parser, []const u8 } { const idx = std.mem.indexOfScalar(u8, self.rest, c) orelse unreachable; return .{ self.seek(idx), self.rest[0..idx] }; } + // Take the current character and advance the parser one step. pub fn take_char(self: *const Parser) struct { Parser, u8 } { return .{ self.seek(1), self.rest[0] }; } + // Return the currently accepted text. pub fn parsed(self: *const Parser) []const u8 { return self.original[0..self.end_idx]; } }; + +test "parser can skip whitespace" { + var parser = init("Hello, World"); + parser = parser.consume_str("Hello,").?; + parser = parser.consume_space().?; + parser = parser.consume_str("World").?; + + try std.testing.expectEqual("Hello, World", parser.parsed()); +}