From f3f58468266ba8a28a4ad039d35d848d96002347 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Mon, 31 Aug 2020 23:33:46 +0000 Subject: [PATCH] Fix command parser returning non-commands/empty messages as errors. This behavior became broken again after switching away from the macro-based command parsing. The bot would return any non !command message as an error, which would cause it to read more messages, and return those as errors, until finally the matrix SDK would throw up. Command parser now more properly handles empty messages and non-commands, but we also simply abort processing if the incoming message doesn't start with an exclamation point. --- src/bot.rs | 11 ++++++-- src/commands.rs | 7 +++--- src/commands/parser.rs | 57 +++++++++++++++++++++++++++++++++--------- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/bot.rs b/src/bot.rs index d13440a..be0c9c5 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -1,6 +1,6 @@ use crate::commands::parse_command; use dirs; -use log::{error, info, warn}; +use log::{error, info, trace, warn}; use matrix_sdk::{ self, events::{ @@ -92,12 +92,19 @@ impl EventEmitter for DiceBot { (String::new(), String::new()) }; + //Command parser can handle non-commands, but faster to + //not parse them. + if !msg_body.starts_with("!") { + trace!("Ignoring non-command: {}", msg_body); + return; + } + let (plain, html) = match parse_command(&msg_body) { Ok(Some(command)) => { let command = command.execute(); (command.plain().into(), command.html().into()) } - Ok(None) => return, + Ok(None) => return, //Ignore non-commands. Err(e) => { let message = format!("Error parsing command: {}", e); let html_message = format!("

{}

", message); diff --git a/src/commands.rs b/src/commands.rs index 337423c..85e2519 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -82,14 +82,15 @@ impl Command for HelpCommand { /// Parse a command string into a dynamic command execution trait /// object. Returns an error if a command was recognized but not -/// parsed correctly. Returns None if no command was recognized. +/// parsed correctly. Returns Ok(None) if no command was recognized. pub fn parse_command(s: &str) -> Result>, String> { match parser::parse_command(s) { Ok((input, command)) => match (input, &command) { - //This clause prevents bot from spamming messages to itself - //after executing a previous command. + //Any command, or text transformed into non-command is + //sent upwards. ("", Some(_)) | (_, None) => Ok(command), + //TODO replcae with nom all_consuming? //Any unconsumed input (whitespace should already be // stripped) is considered a parsing error. _ => Err(format!("{}: malformed expression", s)), diff --git a/src/commands/parser.rs b/src/commands/parser.rs index 2a9d381..d04e09d 100644 --- a/src/commands/parser.rs +++ b/src/commands/parser.rs @@ -2,7 +2,10 @@ use crate::cofd::parser::{create_chance_die, parse_dice_pool}; use crate::commands::{Command, HelpCommand, PoolRollCommand, RollCommand}; use crate::dice::parser::parse_element_expression; use crate::help::parse_help_topic; -use nom::{bytes::complete::tag, character::complete::alpha1, IResult}; +use nom::bytes::streaming::tag; +use nom::error::ErrorKind as NomErrorKind; +use nom::Err as NomErr; +use nom::{character::complete::alpha1, IResult}; // Parse a roll expression. fn parse_roll(input: &str) -> IResult<&str, Box> { @@ -30,9 +33,9 @@ fn help(topic: &str) -> IResult<&str, Box> { /// rest of the line) and returns a tuple of (command_input, command). /// Whitespace at the start and end of the command input is removed. fn split_command(input: &str) -> IResult<&str, &str> { - //let (input, _) = eat_whitespace(input)?; let input = input.trim_start(); let (input, _) = tag("!")(input)?; + let (mut command_input, command) = alpha1(input)?; command_input = command_input.trim(); Ok((command_input, command)) @@ -40,17 +43,28 @@ fn split_command(input: &str) -> IResult<&str, &str> { /// Potentially parse a command expression. If we recognize the /// command, an error should be raised if the command is misparsed. If -/// we don't recognize the command, ignore it and return none +/// we don't recognize the command, ignore it and return None. pub fn parse_command(input: &str) -> IResult<&str, Option>> { - let (cmd_input, cmd) = split_command(input)?; + match split_command(input) { + Ok((cmd_input, cmd)) => match cmd { + "r" | "roll" => parse_roll(cmd_input).map(|(input, command)| (input, Some(command))), + "rp" | "pool" => { + parse_pool_roll(cmd_input).map(|(input, command)| (input, Some(command))) + } + "chance" => chance_die().map(|(input, command)| (input, Some(command))), + "help" => help(cmd_input).map(|(input, command)| (input, Some(command))), + // No recognized command, ignore this. + _ => Ok((input, None)), + }, - match cmd { - "r" | "roll" => parse_roll(cmd_input).map(|(input, command)| (input, Some(command))), - "rp" | "pool" => parse_pool_roll(cmd_input).map(|(input, command)| (input, Some(command))), - "chance" => chance_die().map(|(input, command)| (input, Some(command))), - "help" => help(cmd_input).map(|(input, command)| (input, Some(command))), - // No recognized command, ignore this. - _ => Ok((input, None)), + //TODO better way to do this? + //If the input is not a command, or the message is incomplete + //(empty), we declare this to be a non-command, and don't do + //anything else with it. + Err(NomErr::Error((_, NomErrorKind::Tag))) | Err(NomErr::Incomplete(_)) => Ok(("", None)), + + //All other errors passed up. + Err(e) => Err(e), } } @@ -58,6 +72,26 @@ pub fn parse_command(input: &str) -> IResult<&str, Option>> { mod tests { use super::*; + #[test] + fn non_command_test() { + let result = parse_command("not a command"); + assert!(result.is_ok()); + assert!(result.unwrap().1.is_none()); + } + + #[test] + fn empty_message_test() { + let result = parse_command(""); + assert!(result.is_ok()); + assert!(result.unwrap().1.is_none()); + } + + #[test] + fn just_exclamation_point_test() { + let result = parse_command("!"); + assert!(result.is_err()); + } + #[test] fn basic_command_test() { assert_eq!( @@ -76,7 +110,6 @@ mod tests { #[test] fn whitespace_at_end_test() { - //TODO currently it does not chop whitespace off the end. assert_eq!( ("1d4", "roll"), split_command("!roll 1d4 ").expect("got parsing error")