diff --git a/src/client.rs b/src/client.rs index 78d1a2f5..b87bc49d 100644 --- a/src/client.rs +++ b/src/client.rs @@ -41,8 +41,15 @@ impl OptionExt for Option { /// grammar](https://tools.ietf.org/html/rfc3501#section-9) /// calls "quoted", which is reachable from "string" et al. /// Also ensure it doesn't contain a colliding command-delimiter (newline). -pub(crate) fn validate_str(value: &str) -> Result { - validate_str_noquote(value)?; +/// +/// The arguments `synopsis` and `arg_name` are used to construct the error message of +/// [ValidateError] in case validation fails. +pub(crate) fn validate_str( + synopsis: impl Into, + arg_name: impl Into, + value: &str, +) -> Result { + validate_str_noquote(synopsis, arg_name, value)?; Ok(quote!(value)) } @@ -61,13 +68,26 @@ pub(crate) fn validate_str(value: &str) -> Result { /// > "BODY.PEEK" section ["<" number "." nz-number ">"] /// /// Note the lack of reference to any of the string-like rules or the quote characters themselves. -fn validate_str_noquote(value: &str) -> Result<&str> { +/// +/// The arguments `synopsis` and `arg_name` are used to construct the error message of +/// [ValidateError] in case validation fails. +fn validate_str_noquote( + synopsis: impl Into, + arg_name: impl Into, + value: &str, +) -> Result<&str> { value .matches(|c| c == '\n' || c == '\r') .next() .and_then(|s| s.chars().next()) - .map(|offender| Error::Validate(ValidateError(offender))) - .err()?; + .err() + .map_err(|c| { + Error::Validate(ValidateError { + command_synopsis: synopsis.into(), + argument: arg_name.into(), + offending_char: c, + }) + })?; Ok(value) } @@ -84,13 +104,23 @@ fn validate_str_noquote(value: &str) -> Result<&str> { /// /// Note the lack of reference to SP or any other such whitespace terminals. /// Per this grammar, in theory we ought to be even more restrictive than "no whitespace". -fn validate_sequence_set(value: &str) -> Result<&str> { +fn validate_sequence_set( + synopsis: impl Into, + arg_name: impl Into, + value: &str, +) -> Result<&str> { value .matches(|c: char| c.is_ascii_whitespace()) .next() .and_then(|s| s.chars().next()) - .map(|offender| Error::Validate(ValidateError(offender))) - .err()?; + .err() + .map_err(|c| { + Error::Validate(ValidateError { + command_synopsis: synopsis.into(), + argument: arg_name.into(), + offending_char: c, + }) + })?; Ok(value) } @@ -347,8 +377,11 @@ impl Client { username: U, password: P, ) -> ::std::result::Result, (Error, Client)> { - let u = ok_or_unauth_client_err!(validate_str(username.as_ref()), self); - let p = ok_or_unauth_client_err!(validate_str(password.as_ref()), self); + let synopsis = "LOGIN username password"; + let u = + ok_or_unauth_client_err!(validate_str(synopsis, "username", username.as_ref()), self); + let p = + ok_or_unauth_client_err!(validate_str(synopsis, "password", password.as_ref()), self); ok_or_unauth_client_err!( self.run_command_and_check_ok(&format!("LOGIN {} {}", u, p)), self @@ -494,8 +527,11 @@ impl Session { /// `EXISTS`, `FETCH`, and `EXPUNGE` responses. You can get them from the /// `unsolicited_responses` channel of the [`Session`](struct.Session.html). pub fn select>(&mut self, mailbox_name: S) -> Result { - self.run(&format!("SELECT {}", validate_str(mailbox_name.as_ref())?)) - .and_then(|(lines, _)| parse_mailbox(&lines[..], &mut self.unsolicited_responses_tx)) + self.run(&format!( + "SELECT {}", + validate_str("SELECT mailbox", "mailbox", mailbox_name.as_ref())? + )) + .and_then(|(lines, _)| parse_mailbox(&lines[..], &mut self.unsolicited_responses_tx)) } /// The `EXAMINE` command is identical to [`Session::select`] and returns the same output; @@ -503,8 +539,11 @@ impl Session { /// of the mailbox, including per-user state, will happen in a mailbox opened with `examine`; /// in particular, messagess cannot lose [`Flag::Recent`] in an examined mailbox. pub fn examine>(&mut self, mailbox_name: S) -> Result { - self.run(&format!("EXAMINE {}", validate_str(mailbox_name.as_ref())?)) - .and_then(|(lines, _)| parse_mailbox(&lines[..], &mut self.unsolicited_responses_tx)) + self.run(&format!( + "EXAMINE {}", + validate_str("EXAMINE mailbox", "mailbox", mailbox_name.as_ref())? + )) + .and_then(|(lines, _)| parse_mailbox(&lines[..], &mut self.unsolicited_responses_tx)) } /// Fetch retrieves data associated with a set of messages in the mailbox. @@ -573,10 +612,11 @@ impl Session { if sequence_set.as_ref().is_empty() { Fetches::parse(vec![], &mut self.unsolicited_responses_tx) } else { + let synopsis = "FETCH seq query"; self.run_command_and_read_response(&format!( "FETCH {} {}", - validate_sequence_set(sequence_set.as_ref())?, - validate_str_noquote(query.as_ref())? + validate_sequence_set(synopsis, "seq", sequence_set.as_ref())?, + validate_str_noquote(synopsis, "query", query.as_ref())? )) .and_then(|lines| Fetches::parse(lines, &mut self.unsolicited_responses_tx)) } @@ -592,10 +632,11 @@ impl Session { if uid_set.as_ref().is_empty() { Fetches::parse(vec![], &mut self.unsolicited_responses_tx) } else { + let synopsis = "UID FETCH seq query"; self.run_command_and_read_response(&format!( "UID FETCH {} {}", - validate_sequence_set(uid_set.as_ref())?, - validate_str_noquote(query.as_ref())? + validate_sequence_set(synopsis, "seq", uid_set.as_ref())?, + validate_str_noquote(synopsis, "query", query.as_ref())? )) .and_then(|lines| Fetches::parse(lines, &mut self.unsolicited_responses_tx)) } @@ -646,7 +687,10 @@ impl Session { /// See the description of the [`UID` /// command](https://tools.ietf.org/html/rfc3501#section-6.4.8) for more detail. pub fn create>(&mut self, mailbox_name: S) -> Result<()> { - self.run_command_and_check_ok(&format!("CREATE {}", validate_str(mailbox_name.as_ref())?)) + self.run_command_and_check_ok(&format!( + "CREATE {}", + validate_str("CREATE mailbox", "mailbox", mailbox_name.as_ref())? + )) } /// The [`DELETE` command](https://tools.ietf.org/html/rfc3501#section-6.3.4) permanently @@ -669,7 +713,10 @@ impl Session { /// See the description of the [`UID` /// command](https://tools.ietf.org/html/rfc3501#section-6.4.8) for more detail. pub fn delete>(&mut self, mailbox_name: S) -> Result<()> { - self.run_command_and_check_ok(&format!("DELETE {}", validate_str(mailbox_name.as_ref())?)) + self.run_command_and_check_ok(&format!( + "DELETE {}", + validate_str("DELETE mailbox", "mailbox", mailbox_name.as_ref())? + )) } /// The [`RENAME` command](https://tools.ietf.org/html/rfc3501#section-6.3.5) changes the name @@ -944,7 +991,7 @@ impl Session { self.run_command_and_check_ok(&format!( "MOVE {} {}", sequence_set.as_ref(), - validate_str(mailbox_name.as_ref())? + validate_str("MOVE seq mailbox", "mailbox", mailbox_name.as_ref())? )) } @@ -960,7 +1007,7 @@ impl Session { self.run_command_and_check_ok(&format!( "UID MOVE {} {}", uid_set.as_ref(), - validate_str(mailbox_name.as_ref())? + validate_str("UID MOVE seq mailbox", "mailbox", mailbox_name.as_ref())? )) } @@ -1078,7 +1125,7 @@ impl Session { let mailbox_name = mailbox_name.as_ref(); self.run_command_and_read_response(&format!( "STATUS {} {}", - validate_str(mailbox_name)?, + validate_str("STATUS mailbox dataitems", "mailbox", mailbox_name)?, data_items.as_ref() )) .and_then(|lines| { @@ -1435,6 +1482,65 @@ impl Connection { } } +#[cfg(test)] +pub(crate) mod testutils { + use crate::mock_stream::MockStream; + + use super::*; + + pub(crate) fn assert_validation_error_client( + run_command: F, + expected_synopsis: &'static str, + expected_argument: &'static str, + expected_char: char, + ) where + F: FnOnce( + Client, + ) -> std::result::Result, (Error, Client)>, + { + let response = Vec::new(); + let mock_stream = MockStream::new(response); + let client = Client::new(mock_stream); + assert_eq!( + run_command(client) + .expect_err("Error expected, but got success") + .0 + .to_string(), + Error::Validate(ValidateError { + command_synopsis: expected_synopsis.to_owned(), + argument: expected_argument.to_string(), + offending_char: expected_char + }) + .to_string() + ); + } + + pub(crate) fn assert_validation_error_session( + run_command: F, + expected_synopsis: &'static str, + expected_argument: &'static str, + expected_char: char, + ) where + F: FnOnce(Session) -> Result, + { + let response = Vec::new(); + let mock_stream = MockStream::new(response); + let session = Session::new(Client::new(mock_stream).conn); + assert_eq!( + run_command(session) + .map(|_| ()) + .expect_err("Error expected, but got success") + .to_string(), + Error::Validate(ValidateError { + command_synopsis: expected_synopsis.to_owned(), + argument: expected_argument.to_string(), + offending_char: expected_char + }) + .to_string() + ); + } +} + #[cfg(test)] mod tests { use super::super::error::Result; @@ -1443,6 +1549,8 @@ mod tests { use imap_proto::types::*; use std::borrow::Cow; + use super::testutils::*; + macro_rules! mock_session { ($s:expr) => { Session::new(Client::new($s).conn) @@ -1575,6 +1683,30 @@ mod tests { ); } + #[test] + fn login_validation_username() { + let username = "username\n"; + let password = "password"; + assert_validation_error_client( + |client| client.login(username, password), + "LOGIN username password", + "username", + '\n', + ); + } + + #[test] + fn login_validation_password() { + let username = "username"; + let password = "passw\rord"; + assert_validation_error_client( + |client| client.login(username, password), + "LOGIN username password", + "password", + '\r', + ); + } + #[test] fn logout() { let response = b"a1 OK Logout completed.\r\n".to_vec(); @@ -1743,6 +1875,16 @@ mod tests { assert_eq!(mailbox, expected_mailbox); } + #[test] + fn examine_validation() { + assert_validation_error_session( + |mut session| session.examine("INB\nOX"), + "EXAMINE mailbox", + "mailbox", + '\n', + ) + } + #[test] fn select() { let response = b"* FLAGS (\\Answered \\Flagged \\Deleted \\Seen \\Draft)\r\n\ @@ -1791,6 +1933,16 @@ mod tests { assert_eq!(mailbox, expected_mailbox); } + #[test] + fn select_validation() { + assert_validation_error_session( + |mut session| session.select("INB\nOX"), + "SELECT mailbox", + "mailbox", + '\n', + ) + } + #[test] fn search() { let response = b"* SEARCH 1 2 3 4 5\r\n\ @@ -1906,6 +2058,16 @@ mod tests { ); } + #[test] + fn create_validation() { + assert_validation_error_session( + |mut session| session.create("INB\nOX"), + "CREATE mailbox", + "mailbox", + '\n', + ) + } + #[test] fn delete() { let response = b"a1 OK DELETE completed\r\n".to_vec(); @@ -1920,6 +2082,16 @@ mod tests { ); } + #[test] + fn delete_validation() { + assert_validation_error_session( + |mut session| session.delete("INB\nOX"), + "DELETE mailbox", + "mailbox", + '\n', + ) + } + #[test] fn noop() { let response = b"a1 OK NOOP completed\r\n".to_vec(); @@ -2008,6 +2180,16 @@ mod tests { ); } + #[test] + fn mv_validation_query() { + assert_validation_error_session( + |mut session| session.mv("1:2", "MEE\nTING"), + "MOVE seq mailbox", + "mailbox", + '\n', + ) + } + #[test] fn uid_mv() { let response = b"* OK [COPYUID 1511554416 142,399 41:42] Moved UIDs.\r\n\ @@ -2026,11 +2208,41 @@ mod tests { ); } + #[test] + fn uid_mv_validation_query() { + assert_validation_error_session( + |mut session| session.uid_mv("1:2", "MEE\nTING"), + "UID MOVE seq mailbox", + "mailbox", + '\n', + ) + } + #[test] fn fetch() { generic_fetch(" ", |c, seq, query| c.fetch(seq, query)) } + #[test] + fn fetch_validation_seq() { + assert_validation_error_session( + |mut session| session.fetch("\r1", "BODY[]"), + "FETCH seq query", + "seq", + '\r', + ) + } + + #[test] + fn fetch_validation_query() { + assert_validation_error_session( + |mut session| session.fetch("1", "BODY[\n]"), + "FETCH seq query", + "query", + '\n', + ) + } + #[test] fn uid_fetch() { generic_fetch(" UID ", |c, seq, query| c.uid_fetch(seq, query)) @@ -2057,6 +2269,36 @@ mod tests { ); } + #[test] + fn uid_fetch_validation_seq() { + assert_validation_error_session( + |mut session| session.uid_fetch("\r1", "BODY[]"), + "UID FETCH seq query", + "seq", + '\r', + ) + } + + #[test] + fn uid_fetch_validation_query() { + assert_validation_error_session( + |mut session| session.uid_fetch("1", "BODY[\n]"), + "UID FETCH seq query", + "query", + '\n', + ) + } + + #[test] + fn status_validation_mailbox() { + assert_validation_error_session( + |mut session| session.status("IN\nBOX", "(MESSAGES)"), + "STATUS mailbox dataitems", + "mailbox", + '\n', + ) + } + #[test] fn quote_backslash() { assert_eq!("\"test\\\\text\"", quote!(r"test\text")); @@ -2071,15 +2313,15 @@ mod tests { fn validate_random() { assert_eq!( "\"~iCQ_k;>[&\\\"sVCvUW`e<[&\"sVCvUW`e<[&\"sVCvUW`e<) -> fmt::Result { // print character in debug form because invalid ones are often whitespaces - write!(f, "Invalid character in input: {:?}", self.0) + write!( + f, + "Invalid character {:?} in argument '{}' of command '{}'", + self.offending_char, self.argument, self.command_synopsis + ) } } impl StdError for ValidateError { fn description(&self) -> &str { - "Invalid character in input" + "Invalid character in command argument" } fn cause(&self) -> Option<&dyn StdError> { None } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validate_error_display() { + assert_eq!( + ValidateError { + command_synopsis: "COMMAND arg1 arg2".to_owned(), + argument: "arg2".to_string(), + offending_char: '\n' + } + .to_string(), + "Invalid character '\\n' in argument 'arg2' of command 'COMMAND arg1 arg2'" + ); + } +} diff --git a/src/extensions/metadata.rs b/src/extensions/metadata.rs index 619a2143..23534aa4 100644 --- a/src/extensions/metadata.rs +++ b/src/extensions/metadata.rs @@ -23,19 +23,20 @@ use std::sync::mpsc; use crate::error::No; trait CmdListItemFormat { - fn format_as_cmd_list_item(&self) -> String; + fn format_as_cmd_list_item(&self, item_index : usize) -> Result; } impl CmdListItemFormat for Metadata { - fn format_as_cmd_list_item(&self) -> String { - format!( + fn format_as_cmd_list_item(&self, item_index: usize) -> Result { + let synopsis = "SETMETADATA mailbox (entry value ..)"; + Ok(format!( "{} {}", - validate_str(self.entry.as_str()).unwrap(), + validate_str(synopsis, &format!("entry#{}", item_index + 1), self.entry.as_str())?, self.value .as_ref() - .map(|v| validate_str(v.as_str()).unwrap()) - .unwrap_or_else(|| "NIL".to_string()) - ) + .map(|v| validate_str(synopsis, &format!("value#{}", item_index + 1), v.as_str())) + .unwrap_or_else(|| Ok("NIL".to_string()))? + )) } } @@ -168,10 +169,16 @@ impl Session { depth: MetadataDepth, maxsize: Option, ) -> Result<(Vec, Option)> { + let synopsis = if mailbox.is_some() { + "GETMETADATA options mailbox (entry ..)" + } else { + "GETMETADATA options \"\" (entry ..)" + }; let v: Vec = entries .iter() - .map(|e| validate_str(e.as_ref()).unwrap()) - .collect(); + .enumerate() + .map(|(i, e)| validate_str(synopsis, format!("entry#{}", i + 1), e.as_ref())) + .collect::>()?; let s = v.as_slice().join(" "); let mut command = format!("GETMETADATA (DEPTH {}", depth.depth_str()); @@ -183,8 +190,8 @@ impl Session { format!( ") {} ({})", mailbox - .map(|mbox| validate_str(mbox).unwrap()) - .unwrap_or_else(|| "\"\"".to_string()), + .map(|mbox| validate_str(synopsis, "mailbox", mbox)) + .unwrap_or_else(|| Ok("\"\"".to_string()))?, s ) .as_str(), @@ -235,10 +242,11 @@ impl Session { pub fn set_metadata(&mut self, mbox: impl AsRef, annotations: &[Metadata]) -> Result<()> { let v: Vec = annotations .iter() - .map(|metadata| metadata.format_as_cmd_list_item()) - .collect(); + .enumerate() + .map(|(i, metadata)| metadata.format_as_cmd_list_item(i)) + .collect::>()?; let s = v.as_slice().join(" "); - let command = format!("SETMETADATA {} ({})", validate_str(mbox.as_ref())?, s); + let command = format!("SETMETADATA {} ({})", validate_str("SETMETADATA mailbox (entry value ..)", "mailbox", mbox.as_ref())?, s); self.run_command_and_check_ok(command) } } @@ -276,4 +284,175 @@ mod tests { Err(e) => panic!("Unexpected error: {:?}", e), } } + + use crate::client::testutils::assert_validation_error_session; + + #[test] + fn test_getmetadata_validation_entry1() { + assert_validation_error_session( + |mut session| { + session.get_metadata( + None, + &[ + "/shared/vendor\n/vendor.coi", + "/shared/comment", + "/some/other/entry", + ], + MetadataDepth::Infinity, + None, + ) + }, + "GETMETADATA options \"\" (entry ..)", + "entry#1", + '\n', + ) + } + + #[test] + fn test_getmetadata_validation_entry2() { + assert_validation_error_session( + |mut session| { + session.get_metadata( + Some("INBOX"), + &["/shared/vendor/vendor.coi", "/\rshared/comment"], + MetadataDepth::Infinity, + None, + ) + }, + "GETMETADATA options mailbox (entry ..)", + "entry#2", + '\r', + ) + } + + #[test] + fn test_getmetadata_validation_mailbox() { + assert_validation_error_session( + |mut session| { + session.get_metadata( + Some("INB\nOX"), + &["/shared/vendor/vendor.coi", "/shared/comment"], + MetadataDepth::Infinity, + None, + ) + }, + "GETMETADATA options mailbox (entry ..)", + "mailbox", + '\n', + ); + } + + #[test] + fn test_setmetadata_validation_mailbox() { + assert_validation_error_session( + |mut session| { + session.set_metadata( + "INB\nOX", + &[ + Metadata { + entry: "/shared/vendor/vendor.coi".to_string(), + value: None, + }, + Metadata { + entry: "/shared/comment".to_string(), + value: Some("value".to_string()), + }, + ], + ) + }, + "SETMETADATA mailbox (entry value ..)", + "mailbox", + '\n', + ); + } + + #[test] + fn test_setmetadata_validation_entry1() { + assert_validation_error_session( + |mut session| { + session.set_metadata( + "INBOX", + &[ + Metadata { + entry: "/shared/\nvendor/vendor.coi".to_string(), + value: None, + }, + Metadata { + entry: "/shared/comment".to_string(), + value: Some("value".to_string()), + }, + ], + ) + }, + "SETMETADATA mailbox (entry value ..)", + "entry#1", + '\n', + ); + } + + #[test] + fn test_setmetadata_validation_entry2_key() { + assert_validation_error_session( + |mut session| { + session.set_metadata( + "INBOX", + &[ + Metadata { + entry: "/shared/vendor/vendor.coi".to_string(), + value: None, + }, + Metadata { + entry: "/shared\r/comment".to_string(), + value: Some("value".to_string()), + }, + ], + ) + }, + "SETMETADATA mailbox (entry value ..)", + "entry#2", + '\r', + ); + } + + #[test] + fn test_setmetadata_validation_entry2_value() { + assert_validation_error_session( + |mut session| { + session.set_metadata( + "INBOX", + &[ + Metadata { + entry: "/shared/vendor/vendor.coi".to_string(), + value: None, + }, + Metadata { + entry: "/shared/comment".to_string(), + value: Some("va\nlue".to_string()), + }, + ], + ) + }, + "SETMETADATA mailbox (entry value ..)", + "value#2", + '\n', + ); + } + + #[test] + fn test_setmetadata_validation_entry() { + assert_validation_error_session( + |mut session| { + session.set_metadata( + "INBOX", + &[Metadata { + entry: "/shared/\nvendor/vendor.coi".to_string(), + value: None, + }], + ) + }, + "SETMETADATA mailbox (entry value ..)", + "entry#1", + '\n', + ); + } }