From 01b595d76fe888fff8dcc8cfb3941deb3177a781 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Thu, 9 Apr 2026 11:18:53 +0100 Subject: [PATCH] mailparsing: improve conformance with internationalized email and authres Also fixes up a minor round-trip issue with encoding some of the fields --- crates/mailparsing/src/rfc5322_parser.rs | 81 +++++++++++++++----- crates/message/test_binary_header_values.lua | 29 ++++--- 2 files changed, 80 insertions(+), 30 deletions(-) diff --git a/crates/mailparsing/src/rfc5322_parser.rs b/crates/mailparsing/src/rfc5322_parser.rs index f2233bf86..1c77e1e98 100644 --- a/crates/mailparsing/src/rfc5322_parser.rs +++ b/crates/mailparsing/src/rfc5322_parser.rs @@ -67,7 +67,7 @@ fn not_angle(c: u8) -> bool { // char = { '\u{01}'..'\u{7f}' } fn is_char(c: u8) -> bool { match c { - 0x01..=0xff => true, + 0x01..=0x7f => true, _ => false, } } @@ -1352,13 +1352,14 @@ fn methodspec(input: Span) -> IResult, String)> { } // Taken from https://datatracker.ietf.org/doc/html/rfc8601 which says -// that this is the same as the SMTP Keyword token. -// Only matches ASCII alphanumeric, '+' and '-'. +// that this is the same as the SMTP Keyword token (RFC 5321 section 4.1.2). +// Keyword = Ldh-str = *( ALPHA / DIGIT / "-" ) Let-dig +// Only matches ASCII alphanumeric and '-'. fn keyword(input: Span) -> IResult { context( "keyword", map( - take_while1(|c: u8| c.is_ascii_alphanumeric() || c == b'+' || c == b'-'), + take_while1(|c: u8| c.is_ascii_alphanumeric() || c == b'-'), // SAFETY: predicate only matches ASCII bytes |s: Span| String::from_utf8((*s).into()).expect("keyword is ASCII-only"), ), @@ -1393,6 +1394,10 @@ fn propspec(input: Span) -> IResult { "propspec", map( ( + // RFC 8601 resinfo ABNF says CFWS is required before each + // propspec, but we use opt(cfws) here because other parsers + // (notably quoted_string) may have already consumed the + // whitespace. opt(cfws), keyword, opt(cfws), @@ -1402,6 +1407,10 @@ fn propspec(input: Span) -> IResult { opt(cfws), tag("="), opt(cfws), + // pvalue = [CFWS] ( value / [ [CFWS] "@" ] domain ) [CFWS] + // Try @domain and local@domain first (distinctive @ marker), + // then quoted_string (distinctive " marker), then domain + // (handles dotted names), then mime_token last (single tokens). alt(( map(preceded(tag("@"), domain), |d| { let mut at_dom = BString::from("@"); @@ -1414,9 +1423,9 @@ fn propspec(input: Span) -> IResult { result.push_str(d); result }), + quoted_string, domain, - // value must be last in this alternation - value, + map(mime_token, |s: Span| (*s).into()), )), opt(cfws), ), @@ -1447,8 +1456,16 @@ fn is_mime_token(c: u8) -> bool { } // mime_token = { (!(" " | ctl | tspecials) ~ char)+ } +// Also accepts validated UTF-8 multi-byte sequences per RFC 6532. fn mime_token(input: Span) -> IResult { - context("mime_token", take_while1(is_mime_token)).parse(input) + context( + "mime_token", + recognize(many1(alt(( + take_while1(is_mime_token), + utf8_non_ascii, + )))), + ) + .parse(input) } // RFC2045 modified by RFC2231 MIME header fields @@ -1783,10 +1800,10 @@ impl EncodeHeaderValue for ARCAuthenticationResults { fn encode_value(&self) -> SharedString<'static> { let mut result = format!("i={}; ", self.instance).into_bytes(); - match self.version { - Some(v) => result.push_str(&format!("{} {v}", self.serv_id)), - None => result.push_str(&self.serv_id), - }; + emit_value_token(&self.serv_id, &mut result); + if let Some(v) = self.version { + result.push_str(&format!(" {v}")); + } if self.results.is_empty() { result.push_str("; none"); @@ -1828,6 +1845,7 @@ pub struct AuthenticationResults { /// Emits a value that was parsed by `value`, into target fn emit_value_token(value: &[u8], target: &mut Vec) { + // Allow '@' bare since the pvalue parser handles @domain and local@domain let use_quoted_string = !value.iter().all(|&c| is_mime_token(c) || c == b'@'); if use_quoted_string { target.push(b'"'); @@ -1845,10 +1863,11 @@ fn emit_value_token(value: &[u8], target: &mut Vec) { impl EncodeHeaderValue for AuthenticationResults { fn encode_value(&self) -> SharedString<'static> { - let mut result = match self.version { - Some(v) => format!("{} {v}", self.serv_id).into_bytes(), - None => self.serv_id.to_vec(), - }; + let mut result = Vec::new(); + emit_value_token(&self.serv_id, &mut result); + if let Some(v) = self.version { + result.push_str(&format!(" {v}")); + } if self.results.is_empty() { result.push_str("; none"); } else { @@ -4019,13 +4038,38 @@ ARCAuthenticationResults { k9::snapshot!( encoded, r#" -mx.ex\x80mple.com;\r -\tspf=pass reason=good\xffsig\r -\tsmtp.mailfrom=user@\xfehost +"mx.ex\x80mple.com";\r +\tspf=pass reason="good\xffsig"\r +\tsmtp.mailfrom="user@\xfehost" "# ); } + #[test] + fn authentication_results_serv_id_quoting() { + // A serv_id containing characters that need quoting is properly quoted + let ar = AuthenticationResults { + serv_id: BString::from("mx example.com"), + version: None, + results: vec![], + }; + let encoded = ar.encode_value(); + k9::snapshot!(encoded, r#""mx example.com"; none"#); + + // Normal domain-like serv_id is emitted bare + let ar2 = AuthenticationResults { + serv_id: BString::from("mx.example.com"), + version: Some(1), + results: vec![], + }; + let encoded2 = ar2.encode_value(); + k9::snapshot!(&encoded2, "mx.example.com 1; none"); + // Bare serv_id roundtrips + let parsed = Parser::parse_authentication_results_header(encoded2.as_bytes()).unwrap(); + k9::assert_equal!(parsed.serv_id, ar2.serv_id); + k9::assert_equal!(parsed.version, Some(1)); + } + #[test] fn arc_authentication_results_serialize_as_strings() { let arc = ARCAuthenticationResults { @@ -4040,4 +4084,5 @@ mx.ex\x80mple.com;\r r#"{"instance":1,"serv_id":"mx.example.com","version":null,"results":[]}"# ); } + } diff --git a/crates/message/test_binary_header_values.lua b/crates/message/test_binary_header_values.lua index b6921bb1d..28d55fbc2 100644 --- a/crates/message/test_binary_header_values.lua +++ b/crates/message/test_binary_header_values.lua @@ -62,25 +62,30 @@ local msg4 = new_msg('X-AllBytes: ' .. all_bytes .. '\r\n\r\nBody') local result = msg4:get_first_named_header_value 'X-AllBytes' utils.assert_eq(result, all_bytes) --- Authentication-Results header with binary (non-UTF-8) content in serv_id --- and reason fields. Binary in propspec values is tested with a leading --- non-ASCII byte to avoid the parser's domain-before-value alternation. -local ar_header = 'mx.ex\x80mple.com;' +-- Authentication-Results header with binary (non-UTF-8) content. +-- The raw value accessor preserves binary bytes via the fallback path. +local ar_header_bin = 'mx.ex\x80mple.com;' .. ' spf=pass reason=good\xffsig' .. ' smtp.mailfrom=\xfevalue' local msg5 = - new_msg('Authentication-Results: ' .. ar_header .. '\r\n\r\nBody') + new_msg('Authentication-Results: ' .. ar_header_bin .. '\r\n\r\nBody') local ar_value = msg5:get_first_named_header_value 'Authentication-Results' -utils.assert_eq(ar_value, ar_header) +utils.assert_eq(ar_value, ar_header_bin) --- Use the structured authentication_results accessor to verify binary --- is preserved in individual parsed fields. -local hdr = msg5:parse_mime().headers:get_first_named 'Authentication-Results' +-- The structured authentication_results accessor uses the strict RFC 8601 +-- parser which correctly requires valid characters. Use UTF-8 non-ASCII +-- (internationalized) content to verify it parses and preserves them. +local ar_header_utf8 = 'mx.ex\xc3\xbcmple.com;' + .. ' spf=pass reason=good\xc3\xa9sig' + .. ' smtp.mailfrom=t\xc3\xa9st' +local msg6 = + new_msg('Authentication-Results: ' .. ar_header_utf8 .. '\r\n\r\nBody') +local hdr = msg6:parse_mime().headers:get_first_named 'Authentication-Results' local ar = hdr.authentication_results -utils.assert_eq(ar.serv_id, 'mx.ex\x80mple.com') +utils.assert_eq(ar.serv_id, 'mx.ex\xc3\xbcmple.com') utils.assert_eq(#ar.results, 1) local r = ar.results[1] utils.assert_eq(r.method, 'spf') utils.assert_eq(r.result, 'pass') -utils.assert_eq(r.reason, 'good\xffsig') -utils.assert_eq(r.props['smtp.mailfrom'], '\xfevalue') +utils.assert_eq(r.reason, 'good\xc3\xa9sig') +utils.assert_eq(r.props['smtp.mailfrom'], 't\xc3\xa9st')