From 441b8aadea66f4e29fd93c21196ef23864be2dd2 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 01:23:12 +0100 Subject: [PATCH 1/8] Tidy MailHeader::get_key --- src/lib.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a10b03a..8c47090 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -142,13 +142,10 @@ fn test_find_from_u8() { impl<'a> MailHeader<'a> { /// Get the name of the header. Note that header names are case-insensitive. pub fn get_key(&self) -> Result { - Ok( - try!(encoding::all::ISO_8859_1.decode( - self.key, - encoding::DecoderTrap::Strict, - )).trim() - .to_string(), - ) + encoding::all::ISO_8859_1 + .decode(self.key, encoding::DecoderTrap::Strict) + .map(|s| s.trim().to_string()) + .map_err(MailParseError::EncodingError) } fn decode_word(&self, encoded: &str) -> Option { From c808d38c0076401f93e1b6826b89df11018d8899 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 01:36:41 +0100 Subject: [PATCH 2/8] Tidy is_boundary() --- src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8c47090..8c56322 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,11 +95,9 @@ pub struct MailHeader<'a> { } fn is_boundary(line: &str, ix: Option) -> bool { - ix.map(|v| { - line.chars().nth(v).map_or(true, |c| { - c.is_whitespace() || c == '"' || c == '(' || c == ')' || c == '<' || c == '>' - }) - }).unwrap_or(true) + ix.and_then(|v| line.chars().nth(v)) + .map(|c| c.is_whitespace() || c == '"' || c == '(' || c == ')' || c == '<' || c == '>') + .unwrap_or(true) } fn find_from(line: &str, ix_start: usize, key: &str) -> Option { From 6019e3b152b84c850e2c57456dfe70bc04cf11e0 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 01:46:18 +0100 Subject: [PATCH 3/8] Convert try! to ? --- src/lib.rs | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8c56322..f468513 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -194,10 +194,7 @@ impl<'a> MailHeader<'a> { /// ``` pub fn get_value(&self) -> Result { let mut result = String::new(); - let chars = try!(encoding::all::ISO_8859_1.decode( - self.value, - encoding::DecoderTrap::Strict, - )); + let chars = encoding::all::ISO_8859_1.decode(self.value, encoding::DecoderTrap::Strict)?; let mut lines = chars.lines(); let mut add_space = false; while let Some(line) = lines.next().map(str::trim_left) { @@ -406,7 +403,7 @@ pub trait MailHeaderMap { impl<'a> MailHeaderMap for Vec> { fn get_first_value(&self, key: &str) -> Result, MailParseError> { for x in self { - if try!(x.get_key()).eq_ignore_ascii_case(key) { + if x.get_key()?.eq_ignore_ascii_case(key) { return x.get_value().map(Some); } } @@ -416,8 +413,8 @@ impl<'a> MailHeaderMap for Vec> { fn get_all_values(&self, key: &str) -> Result, MailParseError> { let mut values: Vec = Vec::new(); for x in self { - if try!(x.get_key()).eq_ignore_ascii_case(key) { - values.push(try!(x.get_value())); + if x.get_key()?.eq_ignore_ascii_case(key) { + values.push(x.get_value()?); } } Ok(values) @@ -467,7 +464,7 @@ pub fn parse_headers(raw_data: &[u8]) -> Result<(Vec, usize), MailPa )); } } - let (header, ix_next) = try!(parse_header(&raw_data[ix..])); + let (header, ix_next) = parse_header(&raw_data[ix..])?; headers.push(header); ix += ix_next; } @@ -662,10 +659,7 @@ impl<'a> ParsedMail<'a> { let decoded = self.get_body_raw()?; let charset_conv = encoding::label::encoding_from_whatwg_label(&self.ctype.charset) .unwrap_or(encoding::all::ASCII); - let str_body = try!(charset_conv.decode( - &decoded, - encoding::DecoderTrap::Replace, - )); + let str_body = charset_conv.decode(&decoded, encoding::DecoderTrap::Replace)?; Ok(str_body) } @@ -684,7 +678,7 @@ impl<'a> ParsedMail<'a> { /// assert_eq!(p.get_body_raw().unwrap(), b"This is the body"); /// ``` pub fn get_body_raw(&self) -> Result, MailParseError> { - let transfer_coding = try!(self.headers.get_first_value("Content-Transfer-Encoding")) + let transfer_coding = self.headers.get_first_value("Content-Transfer-Encoding")? .map(|s| s.to_lowercase()); let decoded = match transfer_coding.unwrap_or_default().as_ref() { "base64" => { @@ -695,13 +689,13 @@ impl<'a> ParsedMail<'a> { v => Some(v), }) .collect::>(); - try!(base64::decode(&cleaned)) + base64::decode(&cleaned)? } "quoted-printable" => { - try!(quoted_printable::decode( + quoted_printable::decode( self.body, quoted_printable::ParseMode::Robust, - )) + )? } _ => Vec::::from(self.body), }; @@ -760,7 +754,7 @@ impl<'a> ParsedMail<'a> { /// assert_eq!(dateparse(parsed.headers.get_first_value("Date").unwrap().unwrap().as_str()).unwrap(), 1475417182); /// ``` pub fn parse_mail(raw_data: &[u8]) -> Result { - let (headers, ix_body) = try!(parse_headers(raw_data)); + let (headers, ix_body) = parse_headers(raw_data)?; let ctype = headers .get_first_value("Content-Type")? .map(|s| parse_content_type(&s)) @@ -786,9 +780,7 @@ pub fn parse_mail(raw_data: &[u8]) -> Result { let ix_part_end = find_from_u8(raw_data, ix_part_start, boundary.as_bytes()) .unwrap_or_else(|| raw_data.len()); - result.subparts.push(try!(parse_mail( - &raw_data[ix_part_start..ix_part_end], - ))); + result.subparts.push(parse_mail(&raw_data[ix_part_start..ix_part_end])?); ix_boundary_end = ix_part_end + boundary.len(); if ix_boundary_end + 2 > raw_data.len() || (raw_data[ix_boundary_end] == b'-' && raw_data[ix_boundary_end + 1] == b'-') From c3c1ade07d48f453f541dea59f4e3b1c9c4f0bba Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 01:53:59 +0100 Subject: [PATCH 4/8] Tidy parse_param_content --- src/lib.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f468513..6a70a69 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -813,17 +813,16 @@ fn parse_param_content(content: &str) -> ParamContent { // There must be at least one token produced by split, even if it's empty. let value = tokens.next().unwrap().trim(); let map = tokens - .filter_map(|kv| if let Some(idx) = kv.find('=') { - let key = kv[0..idx].trim().to_lowercase(); - let mut value = kv[idx + 1..].trim(); - if value.starts_with('"') && value.ends_with('"') { - value = &value[1..value.len() - 1]; - } - Some((key, value.to_string())) - } else { - None - }) - .collect(); + .filter_map(|kv| { + kv.find('=').map(|idx| { + let key = kv[0..idx].trim().to_lowercase(); + let mut value = kv[idx + 1..].trim(); + if value.starts_with('"') && value.ends_with('"') { + value = &value[1..value.len() - 1]; + } + (key, value.to_string()) + }) + }).collect(); ParamContent { value: value.into(), From 7eda2c08ebcfca623930779da263d6a0a093bfb8 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 02:07:09 +0100 Subject: [PATCH 5/8] Tidy get_body_raw() with is_ascii_whitespace() Semantics aren't *identical*, since this adds vertical tab to the filter. --- src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6a70a69..ad8daae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -682,12 +682,11 @@ impl<'a> ParsedMail<'a> { .map(|s| s.to_lowercase()); let decoded = match transfer_coding.unwrap_or_default().as_ref() { "base64" => { - let cleaned = self.body + let cleaned = self + .body .iter() - .filter_map(|&c| match c { - b' ' | b'\t' | b'\r' | b'\n' => None, - v => Some(v), - }) + .filter(|c| !c.is_ascii_whitespace()) + .cloned() .collect::>(); base64::decode(&cleaned)? } From 24fe8b3de2eb426c8d3b2410c5fc8e3a33bcea93 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 02:14:50 +0100 Subject: [PATCH 6/8] Tidy get_body_raw() This saves an allocation of the empty string in the no-Content-Transfer-Encoding case. --- src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ad8daae..e7a74a9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -678,10 +678,13 @@ impl<'a> ParsedMail<'a> { /// assert_eq!(p.get_body_raw().unwrap(), b"This is the body"); /// ``` pub fn get_body_raw(&self) -> Result, MailParseError> { - let transfer_coding = self.headers.get_first_value("Content-Transfer-Encoding")? + let transfer_coding = self + .headers + .get_first_value("Content-Transfer-Encoding")? .map(|s| s.to_lowercase()); - let decoded = match transfer_coding.unwrap_or_default().as_ref() { - "base64" => { + + let decoded = match transfer_coding { + Some(ref enc) if enc == "base64" => { let cleaned = self .body .iter() @@ -690,11 +693,8 @@ impl<'a> ParsedMail<'a> { .collect::>(); base64::decode(&cleaned)? } - "quoted-printable" => { - quoted_printable::decode( - self.body, - quoted_printable::ParseMode::Robust, - )? + Some(ref enc) if enc == "quoted-printable" => { + quoted_printable::decode(self.body, quoted_printable::ParseMode::Robust)? } _ => Vec::::from(self.body), }; From 6fa3898524ac4ecc10012aef92c6f53da21572a2 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 02:19:40 +0100 Subject: [PATCH 7/8] Replace try_none! with ? --- src/lib.rs | 12 +++++------- src/macros.rs | 8 -------- 2 files changed, 5 insertions(+), 15 deletions(-) delete mode 100644 src/macros.rs diff --git a/src/lib.rs b/src/lib.rs index e7a74a9..450c45a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,8 +9,6 @@ use std::collections::BTreeMap; use encoding::Encoding; -#[macro_use] -mod macros; mod dateparse; pub use dateparse::dateparse; @@ -147,15 +145,15 @@ impl<'a> MailHeader<'a> { } fn decode_word(&self, encoded: &str) -> Option { - let ix_delim1 = try_none!(encoded.find('?')); - let ix_delim2 = try_none!(find_from(encoded, ix_delim1 + 1, "?")); + let ix_delim1 = encoded.find('?')?; + let ix_delim2 = find_from(encoded, ix_delim1 + 1, "?")?; let charset = &encoded[0..ix_delim1]; let transfer_coding = &encoded[ix_delim1 + 1..ix_delim2]; let input = &encoded[ix_delim2 + 1..]; let decoded = match transfer_coding { - "B" | "b" => try_none!(base64::decode(input.as_bytes()).ok()), + "B" | "b" => base64::decode(input.as_bytes()).ok()?, "Q" | "q" => { // The quoted_printable module does a trim_right on the input, so if // that affects the output we should save and restore the trailing @@ -169,11 +167,11 @@ impl<'a> MailHeader<'a> { to_decode[trimmed.len()..].as_bytes(), ); } - try_none!(d.ok()) + d.ok()? } _ => return None, }; - let charset_conv = try_none!(encoding::label::encoding_from_whatwg_label(charset)); + let charset_conv = encoding::label::encoding_from_whatwg_label(charset)?; charset_conv .decode(&decoded, encoding::DecoderTrap::Replace) .ok() diff --git a/src/macros.rs b/src/macros.rs deleted file mode 100644 index 980f0eb..0000000 --- a/src/macros.rs +++ /dev/null @@ -1,8 +0,0 @@ -macro_rules! try_none { - ( $x:expr ) => {{ - match $x { - Some(v) => v, - None => return None, - } - }} -} From 0a7f56376e0e7f8441a7be890f16052295642bd1 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 7 Sep 2018 02:33:40 +0100 Subject: [PATCH 8/8] Prefer map_err(|e| e.into()) over explicit error --- src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 450c45a..96c01ca 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -141,7 +141,7 @@ impl<'a> MailHeader<'a> { encoding::all::ISO_8859_1 .decode(self.key, encoding::DecoderTrap::Strict) .map(|s| s.trim().to_string()) - .map_err(MailParseError::EncodingError) + .map_err(|e| e.into()) } fn decode_word(&self, encoded: &str) -> Option { @@ -657,8 +657,8 @@ impl<'a> ParsedMail<'a> { let decoded = self.get_body_raw()?; let charset_conv = encoding::label::encoding_from_whatwg_label(&self.ctype.charset) .unwrap_or(encoding::all::ASCII); - let str_body = charset_conv.decode(&decoded, encoding::DecoderTrap::Replace)?; - Ok(str_body) + charset_conv.decode(&decoded, encoding::DecoderTrap::Replace) + .map_err(|e| e.into()) } /// Get the body of the message as a Rust Vec. This function tries to