From 876e1bc510e62ecf6d18b5247039c52c894d9b52 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Mon, 24 Aug 2020 11:46:07 +0300 Subject: [PATCH] melib/imap: turn ImapResponse From to TryFrom --- melib/src/backends/imap.rs | 9 +++--- melib/src/backends/imap/connection.rs | 31 +++++++++++--------- melib/src/backends/imap/protocol_parser.rs | 34 ++++++++++++++++------ 3 files changed, 47 insertions(+), 27 deletions(-) diff --git a/melib/src/backends/imap.rs b/melib/src/backends/imap.rs index 68469f96..e5bbb101 100644 --- a/melib/src/backends/imap.rs +++ b/melib/src/backends/imap.rs @@ -49,6 +49,7 @@ use futures::lock::Mutex as FutureMutex; use futures::stream::Stream; use std::collections::{hash_map::DefaultHasher, BTreeMap}; use std::collections::{BTreeSet, HashMap, HashSet}; +use std::convert::TryFrom; use std::hash::Hasher; use std::pin::Pin; use std::str::FromStr; @@ -788,7 +789,7 @@ impl MailBackend for ImapType { .read_response(&mut response, RequiredResponses::empty()) .await?; } - let ret: Result<()> = ImapResponse::from(&response).into(); + let ret: Result<()> = ImapResponse::try_from(response.as_str())?.into(); ret?; let new_hash = get_path_hash!(path.as_str()); uid_store.mailboxes.lock().await.clear(); @@ -845,7 +846,7 @@ impl MailBackend for ImapType { .read_response(&mut response, RequiredResponses::empty()) .await?; } - let ret: Result<()> = ImapResponse::from(&response).into(); + let ret: Result<()> = ImapResponse::try_from(response.as_str())?.into(); ret?; uid_store.mailboxes.lock().await.clear(); new_mailbox_fut?.await.map_err(|err| format!("Mailbox delete was succesful (returned `{}`) but listing mailboxes afterwards returned `{}`", response, err).into()) @@ -884,7 +885,7 @@ impl MailBackend for ImapType { .await?; } - let ret: Result<()> = ImapResponse::from(&response).into(); + let ret: Result<()> = ImapResponse::try_from(response.as_str())?.into(); if ret.is_ok() { uid_store .mailboxes @@ -936,7 +937,7 @@ impl MailBackend for ImapType { .await?; } let new_hash = get_path_hash!(new_path.as_str()); - let ret: Result<()> = ImapResponse::from(&response).into(); + let ret: Result<()> = ImapResponse::try_from(response.as_str())?.into(); ret?; uid_store.mailboxes.lock().await.clear(); new_mailbox_fut?.await.map_err(|err| format!("Mailbox rename was succesful (returned `{}`) but listing mailboxes afterwards returned `{}`", response, err))?; diff --git a/melib/src/backends/imap/connection.rs b/melib/src/backends/imap/connection.rs index db034f4e..0ba2e205 100644 --- a/melib/src/backends/imap/connection.rs +++ b/melib/src/backends/imap/connection.rs @@ -29,6 +29,7 @@ use futures::io::{AsyncReadExt, AsyncWriteExt}; use native_tls::TlsConnector; pub use smol::Async as AsyncWrapper; use std::collections::HashSet; +use std::convert::TryFrom; use std::future::Future; use std::iter::FromIterator; use std::pin::Pin; @@ -534,7 +535,7 @@ impl ImapConnection { self.send_command(b"COMPRESS DEFLATE").await?; self.read_response(&mut ret, RequiredResponses::empty()) .await?; - match ImapResponse::from(&ret) { + match ImapResponse::try_from(ret.as_str())? { ImapResponse::No(code) | ImapResponse::Bad(code) | ImapResponse::Preauth(code) @@ -578,7 +579,7 @@ impl ImapConnection { match self.server_conf.protocol { ImapProtocol::IMAP { .. } => { - let r: ImapResponse = ImapResponse::from(&response); + let r: ImapResponse = ImapResponse::try_from(response.as_str())?; match r { ImapResponse::Bye(ref response_code) => { self.stream = Err(MeliError::new(format!( @@ -586,31 +587,33 @@ impl ImapConnection { response_code ))); ret.push_str(&response); + return r.into(); } ImapResponse::No(ref response_code) => { //FIXME return error debug!("Received NO response: {:?} {:?}", response_code, response); ret.push_str(&response); + return r.into(); } ImapResponse::Bad(ref response_code) => { //FIXME return error debug!("Received BAD response: {:?} {:?}", response_code, response); ret.push_str(&response); + return r.into(); } - _ => { - /*debug!( - "check every line for required_responses: {:#?}", - &required_responses - );*/ - for l in response.split_rn() { - /*debug!("check line: {}", &l);*/ - if required_responses.check(l) || !self.process_untagged(l).await? { - ret.push_str(l); - } - } + _ => {} + } + /*debug!( + "check every line for required_responses: {:#?}", + &required_responses + );*/ + for l in response.split_rn() { + /*debug!("check line: {}", &l);*/ + if required_responses.check(l) || !self.process_untagged(l).await? { + ret.push_str(l); } } - r.into() + Ok(()) } ImapProtocol::ManageSieve => { ret.push_str(&response); diff --git a/melib/src/backends/imap/protocol_parser.rs b/melib/src/backends/imap/protocol_parser.rs index 760397ae..af7fca19 100644 --- a/melib/src/backends/imap/protocol_parser.rs +++ b/melib/src/backends/imap/protocol_parser.rs @@ -32,6 +32,7 @@ use nom::{ multi::{fold_many1, length_data, many0, separated_list, separated_nonempty_list}, sequence::{delimited, preceded}, }; +use std::convert::TryFrom; use std::str::FromStr; bitflags! { @@ -252,17 +253,29 @@ pub enum ImapResponse { Bye(ResponseCode), } -impl> From for ImapResponse { - fn from(val: T) -> ImapResponse { - let val: &str = val.as_ref().split_rn().last().unwrap_or(val.as_ref()); +impl TryFrom<&'_ str> for ImapResponse { + type Error = MeliError; + fn try_from(val: &'_ str) -> Result { + let val: &str = val.split_rn().last().unwrap_or(val.as_ref()); debug!(&val); - let mut val = val[val.as_bytes().find(b" ").unwrap() + 1..].trim(); + let mut val = val[val.as_bytes().find(b" ").ok_or_else(|| { + MeliError::new(format!( + "Expected tagged IMAP response (OK,NO,BAD, etc) but found {:?}", + val + )) + })? + 1..] + .trim(); // M12 NO [CANNOT] Invalid mailbox name: Name must not have \'/\' characters (0.000 + 0.098 + 0.097 secs).\r\n if val.ends_with(" secs).") { - val = &val[..val.as_bytes().rfind(b"(").unwrap()]; + val = &val[..val.as_bytes().rfind(b"(").ok_or_else(|| { + MeliError::new(format!( + "Expected tagged IMAP response (OK,NO,BAD, etc) but found {:?}", + val + )) + })?]; } - if val.starts_with("OK") { + Ok(if val.starts_with("OK") { Self::Ok(ResponseCode::from(&val["OK ".len()..])) } else if val.starts_with("NO") { Self::No(ResponseCode::from(&val["NO ".len()..])) @@ -273,8 +286,11 @@ impl> From for ImapResponse { } else if val.starts_with("BYE") { Self::Bye(ResponseCode::from(&val["BYE ".len()..])) } else { - panic!("Unknown IMAP response: `{}`", val); - } + return Err(MeliError::new(format!( + "Expected tagged IMAP response (OK,NO,BAD, etc) but found {:?}", + val + ))); + }) } } @@ -295,7 +311,7 @@ impl Into> for ImapResponse { #[test] fn test_imap_response() { - assert_eq!(ImapResponse::from("M12 NO [CANNOT] Invalid mailbox name: Name must not have \'/\' characters (0.000 + 0.098 + 0.097 secs).\r\n"), ImapResponse::No(ResponseCode::Alert("Invalid mailbox name: Name must not have '/' characters".to_string()))); + assert_eq!(ImapResponse::try_from("M12 NO [CANNOT] Invalid mailbox name: Name must not have \'/\' characters (0.000 + 0.098 + 0.097 secs).\r\n").unwrap(), ImapResponse::No(ResponseCode::Alert("Invalid mailbox name: Name must not have '/' characters".to_string()))); } impl<'a> std::iter::DoubleEndedIterator for ImapLineIterator<'a> {