From 9b785503946faa486d8555536885bbc15067840a Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Sat, 6 Mar 2021 14:53:14 -0800 Subject: [PATCH] Adopt latest imap_proto and expose error status codes --- Cargo.toml | 4 ++ src/client.rs | 51 ++++++++++------- src/error.rs | 31 ++++++++++- src/extensions/metadata.rs | 47 ++++++++-------- src/parse.rs | 22 +++++--- src/types/capabilities.rs | 4 +- src/types/fetch.rs | 109 +++++++++++++++---------------------- src/types/name.rs | 18 ++++-- tests/imap_integration.rs | 16 +++--- 9 files changed, 169 insertions(+), 133 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3b1a975..f97629a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,3 +43,7 @@ required-features = ["default"] [[test]] name = "imap_integration" required-features = ["default"] + +[patch.crates-io] +# https://github.com/djc/tokio-imap/pull/115 +imap-proto = { git = "https://github.com/jonhoo/tokio-imap.git", branch = "moo" } diff --git a/src/client.rs b/src/client.rs index a5c0c01..36535f7 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,5 +1,6 @@ use bufstream::BufStream; use chrono::{DateTime, FixedOffset}; +use imap_proto::Response; #[cfg(feature = "tls")] use native_tls::{TlsConnector, TlsStream}; use std::collections::HashSet; @@ -10,7 +11,7 @@ use std::str; use std::sync::mpsc; use super::authenticator::Authenticator; -use super::error::{Error, No, ParseError, Result, ValidateError}; +use super::error::{Bad, Error, No, ParseError, Result, ValidateError}; use super::extensions; use super::parse::*; use super::types::*; @@ -847,11 +848,11 @@ impl Session { /// either does not have [`Flag::Deleted`] set or has a [`Uid`] that is not included in the /// specified sequence set, it is not affected. /// - /// This command is particularly useful for disconnected use clients. By using [`uid_expunge`] - /// instead of [`expunge`] when resynchronizing with the server, the client can ensure that it - /// does not inadvertantly remove any messages that have been marked as [`Flag::Deleted`] by - /// other clients between the time that the client was last connected and the time the client - /// resynchronizes. + /// This command is particularly useful for disconnected use clients. By using `uid_expunge` + /// instead of [`expunge`](Session::expunge) when resynchronizing with the server, the client + /// can ensure that it does not inadvertantly remove any messages that have been marked as + /// [`Flag::Deleted`] by other clients between the time that the client was last connected and + /// the time the client resynchronizes. /// /// This command requires that the server supports [RFC /// 4315](https://tools.ietf.org/html/rfc4315) as indicated by the `UIDPLUS` capability (see @@ -1391,7 +1392,7 @@ impl Connection { }; let break_with = { - use imap_proto::{Response, Status}; + use imap_proto::Status; let line = &data[line_start..]; match imap_proto::parser::parse_response(line) { @@ -1401,16 +1402,19 @@ impl Connection { tag, status, information, + code, .. }, )) => { assert_eq!(tag.as_bytes(), match_tag.as_bytes()); Some(match status { - Status::Bad | Status::No => { - Err((status, information.map(ToString::to_string))) - } + Status::Bad | Status::No => Err(( + status, + information.map(|v| v.into_owned()), + code.map(|v| v.into_owned()), + )), Status::Ok => Ok(()), - status => Err((status, None)), + status => Err((status, None, code.map(|v| v.into_owned()))), }) } Ok((..)) => None, @@ -1418,7 +1422,7 @@ impl Connection { continue_from = Some(line_start); None } - _ => Some(Err((Status::Bye, None))), + _ => Some(Err((Status::Bye, None, None))), } }; @@ -1426,16 +1430,19 @@ impl Connection { Some(Ok(_)) => { break Ok(line_start); } - Some(Err((status, expl))) => { + Some(Err((status, expl, code))) => { use imap_proto::Status; match status { Status::Bad => { - break Err(Error::Bad( - expl.unwrap_or_else(|| "no explanation given".to_string()), - )); + break Err(Error::Bad(Bad { + code, + information: expl + .unwrap_or_else(|| "no explanation given".to_string()), + })); } Status::No => { break Err(Error::No(No { + code, information: expl .unwrap_or_else(|| "no explanation given".to_string()), })); @@ -1487,6 +1494,7 @@ mod tests { use super::super::mock_stream::MockStream; use super::*; use imap_proto::types::*; + use std::borrow::Cow; macro_rules! mock_session { ($s:expr) => { @@ -1499,7 +1507,8 @@ mod tests { let response = "a0 OK Logged in.\r\n"; let mock_stream = MockStream::new(response.as_bytes().to_vec()); let mut client = Client::new(mock_stream); - let actual_response = client.read_response().unwrap(); + let (mut actual_response, i) = client.read_response().unwrap(); + actual_response.truncate(i); assert_eq!(Vec::::new(), actual_response); } @@ -1589,7 +1598,7 @@ mod tests { let client = Client::new(mock_stream); enum Authenticate { Auth, - }; + } impl Authenticator for Authenticate { type Response = Vec; fn process(&self, challenge: &[u8]) -> Self::Response { @@ -1846,9 +1855,9 @@ mod tests { .to_vec(); let expected_capabilities = vec![ Capability::Imap4rev1, - Capability::Atom("STARTTLS"), - Capability::Auth("GSSAPI"), - Capability::Atom("LOGINDISABLED"), + Capability::Atom(Cow::Borrowed("STARTTLS")), + Capability::Auth(Cow::Borrowed("GSSAPI")), + Capability::Atom(Cow::Borrowed("LOGINDISABLED")), ]; let mock_stream = MockStream::new(response); let mut session = mock_session!(mock_stream); diff --git a/src/error.rs b/src/error.rs index 81ac38f..94d21f2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,7 +10,7 @@ use std::str::Utf8Error; use base64::DecodeError; use bufstream::IntoInnerError as BufError; -use imap_proto::Response; +use imap_proto::{types::ResponseCode, Response}; #[cfg(feature = "tls")] use native_tls::Error as TlsError; #[cfg(feature = "tls")] @@ -19,11 +19,36 @@ use native_tls::HandshakeError as TlsHandshakeError; /// A convenience wrapper around `Result` for `imap::Error`. pub type Result = result::Result; -/// A `NO` response from the server, which may contain additional metadata about the error. +/// A BAD response from the server, which indicates an error message from the server. +#[derive(Debug)] +#[non_exhaustive] +pub struct Bad { + /// Human-redable message included with the Bad response. + pub information: String, + /// A more specific error status code included with the Bad response. + pub code: Option>, +} + +impl fmt::Display for Bad { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.information) + } +} + +/// A NO response from the server, which indicates an operational error message from the server. #[derive(Debug)] #[non_exhaustive] pub struct No { + /// Human-redable message included with the NO response. pub information: String, + /// A more specific error status code included with the NO response. + pub code: Option>, +} + +impl fmt::Display for No { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.information) + } } /// A set of errors that can occur in the IMAP client @@ -39,7 +64,7 @@ pub enum Error { #[cfg(feature = "tls")] Tls(TlsError), /// A BAD response from the IMAP server. - Bad(String), + Bad(Bad), /// A NO response from the IMAP server. No(No), /// The connection was terminated unexpectedly. diff --git a/src/extensions/metadata.rs b/src/extensions/metadata.rs index 3e29feb..5683a11 100644 --- a/src/extensions/metadata.rs +++ b/src/extensions/metadata.rs @@ -3,8 +3,9 @@ //! //! Mailboxes or the server as a whole may have zero or more annotations associated with them. An //! annotation contains a uniquely named entry, which has a value. Annotations can be added to -//! mailboxes when a mailbox name is provided as the first argument to [`set_metadata`], or to the -//! server as a whole when the first argument is `None`. +//! mailboxes when a mailbox name is provided as the first argument to +//! [`set_metadata`](Session::set_metadata), or to the server as a whole when the first argument is +//! `None`. //! //! For example, a general comment being added to a mailbox may have an entry name of "/comment" //! and a value of "Really useful mailbox". @@ -13,10 +14,14 @@ use crate::client::*; use crate::error::{Error, ParseError, Result}; use crate::parse::handle_unilateral; use crate::types::*; -use imap_proto::types::{MailboxDatum, Metadata, Response}; +use imap_proto::types::{MailboxDatum, Metadata, Response, ResponseCode}; use std::io::{Read, Write}; use std::sync::mpsc; +// for intra-doc links +#[allow(unused_imports)] +use crate::error::No; + trait CmdListItemFormat { fn format_as_cmd_list_item(&self) -> String; } @@ -162,7 +167,7 @@ impl Session { entries: &[impl AsRef], depth: MetadataDepth, maxsize: Option, - ) -> Result<(Vec, Option)> { + ) -> Result<(Vec, Option)> { let v: Vec = entries .iter() .map(|e| validate_str(e.as_ref()).unwrap()) @@ -195,8 +200,8 @@ impl Session { { match code { None => None, - // TODO: https://github.com/djc/tokio-imap/issues/113 - Some(_) => {} + Some(ResponseCode::MetadataLongEntries(v)) => Some(v), + Some(_) => None, } } else { unreachable!("already parsed as Done by Client::run"); @@ -213,22 +218,18 @@ impl Session { /// provided, on the specified existing mailboxes or on the server (if the mailbox argument is /// `None`). Clients can use `None` for the value of entries it wants to remove. /// - /// If the server is unable to set an annotation because the size of its - /// value is too large, this command will fail with a [`Error::No`]. - // TODO: https://github.com/djc/tokio-imap/issues/113 - // with a "[METADATA MAXSIZE NNN]" response code when NNN is the maximum octet count that it is - // willing to accept. + /// If the server is unable to set an annotation because the size of its value is too large, + /// this command will fail with a [`Error::No`] and its [status code](No::code) will be + /// [`ResponseCode::MetadataMaxSize`] where the contained value is the maximum octet count that + /// the server is willing to accept. /// - /// If the server is unable to set a new annotation because the maximum - /// number of allowed annotations has already been reached, this command will also fail with an - /// [`Error::No`]. - // TODO: https://github.com/djc/tokio-imap/issues/113 - // with a "[METADATA TOOMANY]" response code. + /// If the server is unable to set a new annotation because the maximum number of allowed + /// annotations has already been reached, this command will fail with an [`Error::No`] and its + /// [status code](No::code) will be [`ResponseCode::MetadataTooMany`]. /// /// If the server is unable to set a new annotation because it does not support private - /// annotations on one of the specified mailboxes, you guess it, you'll get an [`Error::No`]. - // TODO: https://github.com/djc/tokio-imap/issues/113 - // with a "[METADATA NOPRIVATE]" response code. + /// annotations on one of the specified mailboxes, you guess it, you'll get an [`Error::No`] with + /// a [status code](No::code) of [`ResponseCode::MetadataNoPrivate`]. /// /// When any one annotation fails to be set and [`Error::No`] is returned, the server will not /// change the values for other annotations specified. @@ -257,16 +258,16 @@ mod tests { let mock_stream = MockStream::new(response.as_bytes().to_vec()); let client = Client::new(mock_stream); let mut session = client.login("testuser", "pass").unwrap(); - let r = get_metadata( - &mut session, - "", + let r = session.get_metadata( + None, &["/shared/vendor/vendor.coi", "/shared/comment"], MetadataDepth::Infinity, Option::None, ); match r { - Ok(v) => { + Ok((v, missed)) => { + assert_eq!(missed, None); assert_eq!(v.len(), 3); assert_eq!(v[0].entry, "/shared/vendor/vendor.coi/a"); assert_eq!(v[0].value.as_ref().expect("None is not expected"), "AAA"); diff --git a/src/parse.rs b/src/parse.rs index 5a4103d..ae79fe3 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -108,7 +108,9 @@ pub fn parse_fetches( use imap_proto::AttributeValue; match attr { AttributeValue::Flags(flags) => { - fetch.flags.extend(flags.iter().cloned().map(Flag::from)); + fetch + .flags + .extend(flags.iter().map(|f| Flag::from(f.to_string()))); } AttributeValue::Uid(uid) => fetch.uid = Some(*uid), AttributeValue::Rfc822Size(sz) => fetch.size = Some(*sz), @@ -399,14 +401,15 @@ pub(crate) fn handle_unilateral<'a>( mod tests { use super::*; use imap_proto::types::*; + use std::borrow::Cow; #[test] fn parse_capability_test() { let expected_capabilities = vec![ Capability::Imap4rev1, - Capability::Atom("STARTTLS"), - Capability::Auth("GSSAPI"), - Capability::Atom("LOGINDISABLED"), + Capability::Atom(Cow::Borrowed("STARTTLS")), + Capability::Auth(Cow::Borrowed("GSSAPI")), + Capability::Atom(Cow::Borrowed("LOGINDISABLED")), ]; let lines = b"* CAPABILITY IMAP4rev1 STARTTLS AUTH=GSSAPI LOGINDISABLED\r\n"; let (mut send, recv) = mpsc::channel(); @@ -422,7 +425,10 @@ mod tests { #[test] fn parse_capability_case_insensitive_test() { // Test that "IMAP4REV1" (instead of "IMAP4rev1") is accepted - let expected_capabilities = vec![Capability::Imap4rev1, Capability::Atom("STARTTLS")]; + let expected_capabilities = vec![ + Capability::Imap4rev1, + Capability::Atom(Cow::Borrowed("STARTTLS")), + ]; let lines = b"* CAPABILITY IMAP4REV1 STARTTLS\r\n"; let (mut send, recv) = mpsc::channel(); let capabilities = parse_capabilities(lines.to_vec(), &mut send).unwrap(); @@ -525,9 +531,9 @@ mod tests { fn parse_capabilities_w_unilateral() { let expected_capabilities = vec![ Capability::Imap4rev1, - Capability::Atom("STARTTLS"), - Capability::Auth("GSSAPI"), - Capability::Atom("LOGINDISABLED"), + Capability::Atom(Cow::Borrowed("STARTTLS")), + Capability::Auth(Cow::Borrowed("GSSAPI")), + Capability::Atom(Cow::Borrowed("LOGINDISABLED")), ]; let lines = b"\ * CAPABILITY IMAP4rev1 STARTTLS AUTH=GSSAPI LOGINDISABLED\r\n\ diff --git a/src/types/capabilities.rs b/src/types/capabilities.rs index 428ada9..13238d9 100644 --- a/src/types/capabilities.rs +++ b/src/types/capabilities.rs @@ -51,10 +51,10 @@ impl Capabilities { if s.len() > AUTH_CAPABILITY_PREFIX.len() { let (pre, val) = s.split_at(AUTH_CAPABILITY_PREFIX.len()); if pre.eq_ignore_ascii_case(AUTH_CAPABILITY_PREFIX) { - return self.has(&Capability::Auth(val)); + return self.has(&Capability::Auth(val.into())); } } - self.has(&Capability::Atom(s)) + self.has(&Capability::Atom(s.into())) } /// Iterate over all the server's capabilities diff --git a/src/types/fetch.rs b/src/types/fetch.rs index a5821c5..6a64ec0 100644 --- a/src/types/fetch.rs +++ b/src/types/fetch.rs @@ -40,36 +40,30 @@ impl Fetch { /// The bytes that make up the header of this message, if `BODY[HEADER]`, `BODY.PEEK[HEADER]`, /// or `RFC822.HEADER` was included in the `query` argument to `FETCH`. pub fn header(&self) -> Option<&[u8]> { - self.fetch - .iter() - .filter_map(|av| match av { - AttributeValue::BodySection { - section: Some(SectionPath::Full(MessageSection::Header)), - data: Some(hdr), - .. - } - | AttributeValue::Rfc822Header(Some(hdr)) => Some(*hdr), - _ => None, - }) - .next() + self.fetch.iter().find_map(|av| match av { + AttributeValue::BodySection { + section: Some(SectionPath::Full(MessageSection::Header)), + data: Some(hdr), + .. + } + | AttributeValue::Rfc822Header(Some(hdr)) => Some(&**hdr), + _ => None, + }) } /// The bytes that make up this message, included if `BODY[]` or `RFC822` was included in the /// `query` argument to `FETCH`. The bytes SHOULD be interpreted by the client according to the /// content transfer encoding, body type, and subtype. pub fn body(&self) -> Option<&[u8]> { - self.fetch - .iter() - .filter_map(|av| match av { - AttributeValue::BodySection { - section: None, - data: Some(body), - .. - } - | AttributeValue::Rfc822(Some(body)) => Some(*body), - _ => None, - }) - .next() + self.fetch.iter().find_map(|av| match av { + AttributeValue::BodySection { + section: None, + data: Some(body), + .. + } + | AttributeValue::Rfc822(Some(body)) => Some(&**body), + _ => None, + }) } /// The bytes that make up the text of this message, included if `BODY[TEXT]`, `RFC822.TEXT`, @@ -77,18 +71,15 @@ impl Fetch { /// interpreted by the client according to the content transfer encoding, body type, and /// subtype. pub fn text(&self) -> Option<&[u8]> { - self.fetch - .iter() - .filter_map(|av| match av { - AttributeValue::BodySection { - section: Some(SectionPath::Full(MessageSection::Text)), - data: Some(body), - .. - } - | AttributeValue::Rfc822Text(Some(body)) => Some(*body), - _ => None, - }) - .next() + self.fetch.iter().find_map(|av| match av { + AttributeValue::BodySection { + section: Some(SectionPath::Full(MessageSection::Text)), + data: Some(body), + .. + } + | AttributeValue::Rfc822Text(Some(body)) => Some(&**body), + _ => None, + }) } /// The envelope of this message, if `ENVELOPE` was included in the `query` argument to @@ -99,13 +90,10 @@ impl Fetch { /// The full description of the format of the envelope is given in [RFC 3501 section /// 7.4.2](https://tools.ietf.org/html/rfc3501#section-7.4.2). pub fn envelope(&self) -> Option<&Envelope<'_>> { - self.fetch - .iter() - .filter_map(|av| match av { - AttributeValue::Envelope(env) => Some(&**env), - _ => None, - }) - .next() + self.fetch.iter().find_map(|av| match av { + AttributeValue::Envelope(env) => Some(&**env), + _ => None, + }) } /// Extract the bytes that makes up the given `BOD[
]` of a `FETCH` response. @@ -113,17 +101,14 @@ impl Fetch { /// See [section 7.4.2 of RFC 3501](https://tools.ietf.org/html/rfc3501#section-7.4.2) for /// details. pub fn section(&self, path: &SectionPath) -> Option<&[u8]> { - self.fetch - .iter() - .filter_map(|av| match av { - AttributeValue::BodySection { - section: Some(sp), - data: Some(data), - .. - } if sp == path => Some(*data), - _ => None, - }) - .next() + self.fetch.iter().find_map(|av| match av { + AttributeValue::BodySection { + section: Some(sp), + data: Some(data), + .. + } if sp == path => Some(&**data), + _ => None, + }) } /// Extract the `INTERNALDATE` of a `FETCH` response @@ -133,11 +118,10 @@ impl Fetch { pub fn internal_date(&self) -> Option> { self.fetch .iter() - .filter_map(|av| match av { - AttributeValue::InternalDate(date_time) => Some(*date_time), + .find_map(|av| match av { + AttributeValue::InternalDate(date_time) => Some(&**date_time), _ => None, }) - .next() .and_then( |date_time| match DateTime::parse_from_str(date_time, DATE_TIME_FORMAT) { Ok(date_time) => Some(date_time), @@ -151,12 +135,9 @@ impl Fetch { /// See [section 2.3.6 of RFC 3501](https://tools.ietf.org/html/rfc3501#section-2.3.6) for /// details. pub fn bodystructure<'a>(&self) -> Option<&BodyStructure<'a>> { - self.fetch - .iter() - .filter_map(|av| match av { - AttributeValue::BodyStructure(bs) => Some(bs), - _ => None, - }) - .next() + self.fetch.iter().find_map(|av| match av { + AttributeValue::BodyStructure(bs) => Some(bs), + _ => None, + }) } } diff --git a/src/types/name.rs b/src/types/name.rs index 88f31b7..9196738 100644 --- a/src/types/name.rs +++ b/src/types/name.rs @@ -6,8 +6,8 @@ pub struct Name { // Note that none of these fields are *actually* 'static. // Rather, they are tied to the lifetime of the `ZeroCopy` that contains this `Name`. pub(crate) attributes: Vec>, - pub(crate) delimiter: Option<&'static str>, - pub(crate) name: &'static str, + pub(crate) delimiter: Option>, + pub(crate) name: Cow<'static, str>, } /// An attribute set for an IMAP name. @@ -56,6 +56,16 @@ impl<'a> From for NameAttribute<'a> { } } +impl<'a> From> for NameAttribute<'a> { + fn from(s: Cow<'a, str>) -> Self { + if let Some(f) = NameAttribute::system(&*s) { + f + } else { + NameAttribute::Custom(s) + } + } +} + impl<'a> From<&'a str> for NameAttribute<'a> { fn from(s: &'a str) -> Self { if let Some(f) = NameAttribute::system(s) { @@ -77,7 +87,7 @@ impl Name { /// of naming hierarchy. All children of a top-level hierarchy node use the same /// separator character. `None` means that no hierarchy exists; the name is a "flat" name. pub fn delimiter(&self) -> Option<&str> { - self.delimiter + self.delimiter.as_deref() } /// The name represents an unambiguous left-to-right hierarchy, and are valid for use as a @@ -85,6 +95,6 @@ impl Name { /// the name is also valid as an argument for commands, such as `SELECT`, that accept mailbox /// names. pub fn name(&self) -> &str { - self.name + &*self.name } } diff --git a/tests/imap_integration.rs b/tests/imap_integration.rs index 997b2f5..25770aa 100644 --- a/tests/imap_integration.rs +++ b/tests/imap_integration.rs @@ -141,17 +141,17 @@ fn inbox() { assert_ne!(fetch.uid, None); assert_eq!(fetch.size, Some(138)); let e = fetch.envelope().unwrap(); - assert_eq!(e.subject, Some(&b"My first e-mail"[..])); + assert_eq!(e.subject, Some(b"My first e-mail"[..].into())); assert_ne!(e.from, None); assert_eq!(e.from.as_ref().unwrap().len(), 1); let from = &e.from.as_ref().unwrap()[0]; - assert_eq!(from.mailbox, Some(&b"sender"[..])); - assert_eq!(from.host, Some(&b"localhost"[..])); + assert_eq!(from.mailbox, Some(b"sender"[..].into())); + assert_eq!(from.host, Some(b"localhost"[..].into())); assert_ne!(e.to, None); assert_eq!(e.to.as_ref().unwrap().len(), 1); let to = &e.to.as_ref().unwrap()[0]; - assert_eq!(to.mailbox, Some(&b"inbox"[..])); - assert_eq!(to.host, Some(&b"localhost"[..])); + assert_eq!(to.mailbox, Some(b"inbox"[..].into())); + assert_eq!(to.host, Some(b"localhost"[..].into())); let date_opt = fetch.internal_date(); assert!(date_opt.is_some()); @@ -209,7 +209,7 @@ fn inbox_uid() { let fetch = &fetch[0]; assert_eq!(fetch.uid, Some(uid)); let e = fetch.envelope().unwrap(); - assert_eq!(e.subject, Some(&b"My first e-mail"[..])); + assert_eq!(e.subject, Some(b"My first e-mail"[..].into())); let date_opt = fetch.internal_date(); assert!(date_opt.is_some()); @@ -269,7 +269,7 @@ fn append() { let fetch = &fetch[0]; assert_eq!(fetch.uid, Some(uid)); let e = fetch.envelope().unwrap(); - assert_eq!(e.subject, Some(&b"My second e-mail"[..])); + assert_eq!(e.subject, Some(b"My second e-mail"[..].into())); // and let's delete it to clean up c.uid_store(format!("{}", uid), "+FLAGS (\\Deleted)") @@ -320,7 +320,7 @@ fn append_with_flags() { let fetch = &fetch[0]; assert_eq!(fetch.uid, Some(uid)); let e = fetch.envelope().unwrap(); - assert_eq!(e.subject, Some(&b"My third e-mail"[..])); + assert_eq!(e.subject, Some(b"My third e-mail"[..].into())); // check the flags let setflags = fetch.flags();