mailparsing: remove implicit lossy MimePart::to_message_string()

This was causing some problems with legacy shift_jis content,
so replace it with a binary preserving equivalent, and fixup
the call sites to use the renamed `to_message_bytes` method
instead.

crates/integration-tests/src/kumod.rs

@@ -158,7 +158,7 @@ impl MailGenParams<'_> {
         message.prepend("X-Test1", "Test1");
         message.prepend("X-Another", "Another");
         message.set_stable_content(true);
-        Ok(message.build()?.to_message_string())
+        Ok(String::from_utf8(message.build()?.to_message_bytes())?)
     }
 }
 

crates/kumo-log-types/src/rfc3464.rs

@@ -783,7 +783,7 @@ mod test {
         let report_msg = Report::generate(&params, Some(&original_msg), &log)
             .unwrap()
             .unwrap();
-        let report_eml = report_msg.to_message_string();
+        let report_eml = BString::from(report_msg.to_message_bytes());
         k9::snapshot!(
             &report_eml,
             r#"
@@ -914,7 +914,7 @@ Subject: Hello!
         let report_msg = Report::generate(&params, Some(&original_msg), &log)
             .unwrap()
             .unwrap();
-        let report_eml = report_msg.to_message_string();
+        let report_eml = BString::from(report_msg.to_message_bytes());
         k9::snapshot!(
             &report_eml,
             r#"
@@ -1047,7 +1047,7 @@ Subject: Hello!
         let report_msg = Report::generate(&params, Some(&original_msg), &log)
             .unwrap()
             .unwrap();
-        let report_eml = report_msg.to_message_string();
+        let report_eml = BString::from(report_msg.to_message_bytes());
         k9::snapshot!(
             &report_eml,
             r#"
@@ -1185,7 +1185,7 @@ hello there
         let report_msg = Report::generate(&params, Some(&original_msg), &log)
             .unwrap()
             .unwrap();
-        let report_eml = report_msg.to_message_string();
+        let report_eml = BString::from(report_msg.to_message_bytes());
         k9::snapshot!(
             &report_eml,
             r#"

crates/kumod/src/http_server/inject_v1.rs

@@ -508,7 +508,7 @@ impl<'a> Compiled<'a> {
                     msg.headers_mut().set_mime_version("1.0")?;
                 }
 
-                Ok(msg.to_message_string())
+                Ok(String::from_utf8(msg.to_message_bytes())?)
             }
             Content::Builder {
                 text_body,
@@ -563,7 +563,7 @@ impl<'a> Compiled<'a> {
                     builder.attach_part(part.clone());
                 }
 
-                Ok(builder.build()?.to_message_string())
+                Ok(String::from_utf8(builder.build()?.to_message_bytes())?)
             }
         }
     }

crates/kumod/src/mod_kumo.rs

@@ -284,14 +284,14 @@ pub fn register(lua: &Lua) -> anyhow::Result<()> {
             Some(report) => {
                 let recip = EnvelopeAddress::parse(&log_record.sender)
                     .context("log_record is somehow an invalid EnvelopeAddress")?;
-                let body = report.to_message_string();
+                let body = report.to_message_bytes();
 
                 let msg = Message::new_dirty(
                     SpoolId::new(),
                     EnvelopeAddress::null_sender(),
                     vec![recip],
                     serde_json::json!({}),
-                    Arc::new(body.as_bytes().to_vec().into_boxed_slice()),
+                    Arc::new(body.into_boxed_slice()),
                 )?;
                 Ok(Some(msg))
             }

crates/mailparsing/src/builder.rs

@@ -191,6 +191,7 @@ impl<'a> std::ops::DerefMut for MessageBuilder<'a> {
 #[cfg(test)]
 mod test {
     use super::*;
+    use bstr::BString;
 
     #[test]
     fn basic() {
@@ -201,7 +202,7 @@ mod test {
         b.text_html("<b>this is html 🚀</b>");
         let msg = b.build().unwrap();
         k9::snapshot!(
-            msg.to_message_string(),
+            BString::from(msg.to_message_bytes()),
             r#"
 Content-Type: multipart/alternative;\r
 \tboundary="ma-boundary"\r
@@ -251,7 +252,7 @@ Hello World in AMP!
         );
         let msg = b.build().unwrap();
         k9::snapshot!(
-            msg.to_message_string(),
+            BString::from(msg.to_message_bytes()),
             r#"
 Content-Type: multipart/alternative;\r
 \tboundary="ma-boundary"\r
@@ -311,7 +312,7 @@ Content-Transfer-Encoding: quoted-printable\r
         .unwrap();
         let msg = b.build().unwrap();
         k9::snapshot!(
-            msg.to_message_string(),
+            BString::from(msg.to_message_bytes()),
             r#"
 Content-Type: multipart/mixed;\r
 \tboundary="mm-boundary"\r

crates/mailparsing/src/mimepart.rs

@@ -647,10 +647,10 @@ impl<'a> MimePart<'a> {
 
     /// Convenience method wrapping write_message that returns
     /// the formatted message as a standalone string
-    pub fn to_message_string(&self) -> String {
+    pub fn to_message_bytes(&self) -> Vec<u8> {
         let mut out = vec![];
         self.write_message(&mut out).unwrap();
-        String::from_utf8_lossy(&out).to_string()
+        out
     }
 
     pub fn replace_text_body(
@@ -1306,7 +1306,7 @@ mod test {
         );
 
         let part = MimePart::parse(message).unwrap();
-        k9::assert_equal!(message, part.to_message_string());
+        k9::assert_equal!(message.as_bytes(), part.to_message_bytes());
         assert_eq!(part.raw_body(), "I am the body");
         k9::snapshot!(
             part.body(),
@@ -1320,7 +1320,7 @@ Ok(
         );
 
         k9::snapshot!(
-            part.rebuild(None).unwrap().to_message_string(),
+            BString::from(part.rebuild(None).unwrap().to_message_bytes()),
             r#"
 Content-Type: text/plain;\r
 \tcharset="us-ascii"\r
@@ -1367,7 +1367,7 @@ I am the body\r
         );
 
         let part = MimePart::parse(message).unwrap();
-        k9::assert_equal!(message, part.to_message_string());
+        k9::assert_equal!(message.as_bytes(), part.to_message_bytes());
         assert_eq!(part.raw_body(), "aGVsbG8K\n");
         k9::snapshot!(
             part.body(),
@@ -1382,7 +1382,7 @@ Ok(
         );
 
         k9::snapshot!(
-            part.rebuild(None).unwrap().to_message_string(),
+            BString::from(part.rebuild(None).unwrap().to_message_bytes()),
             r#"
 Content-Type: text/plain;\r
 \tcharset="us-ascii"\r
@@ -1422,7 +1422,7 @@ hello=0A\r
 
         let part = MimePart::parse(message).unwrap();
 
-        k9::assert_equal!(message, part.to_message_string());
+        k9::assert_equal!(message.as_bytes(), part.to_message_bytes());
 
         let children = part.child_parts();
         k9::assert_equal!(children.len(), 2);
@@ -1475,7 +1475,7 @@ Ok(
         );
 
         let mut part = MimePart::parse(message).unwrap();
-        k9::assert_equal!(message, part.to_message_string());
+        k9::assert_equal!(message.as_bytes(), part.to_message_bytes());
         fn munge(part: &mut MimePart) {
             let headers = part.headers_mut();
             headers.push(Header::with_name_value("X-Woot", "Hello"));
@@ -1484,7 +1484,7 @@ Ok(
         }
         munge(&mut part);
 
-        let re_encoded = part.to_message_string();
+        let re_encoded = BString::from(part.to_message_bytes());
         k9::snapshot!(
             re_encoded,
             r#"
@@ -1520,7 +1520,7 @@ After the final boundary stuff gets ignored.\r
 
         eprintln!("part with html removed is:\n{part:#?}");
 
-        let re_encoded = part.to_message_string();
+        let re_encoded = BString::from(part.to_message_bytes());
         k9::snapshot!(
             re_encoded,
             r#"
@@ -1546,7 +1546,7 @@ After the final boundary stuff gets ignored.\r
     #[test]
     fn replace_text_body() {
         let mut part = MimePart::new_text_plain("Hello 👻\r\n").unwrap();
-        let encoded = part.to_message_string();
+        let encoded = BString::from(part.to_message_bytes());
         k9::snapshot!(
             &encoded,
             r#"
@@ -1561,7 +1561,7 @@ SGVsbG8g8J+Ruw0K\r
 
         part.replace_text_body("text/plain", "Hello 🚀\r\n")
             .unwrap();
-        let encoded = part.to_message_string();
+        let encoded = BString::from(part.to_message_bytes());
         k9::snapshot!(
             &encoded,
             r#"
@@ -1581,7 +1581,7 @@ SGVsbG8g8J+agA0K\r
 
         let part = MimePart::new_text_plain(input_text).unwrap();
 
-        let encoded = part.to_message_string();
+        let encoded = BString::from(part.to_message_bytes());
         k9::snapshot!(
             &encoded,
             r#"
@@ -1598,7 +1598,7 @@ t's see how that turns out!\r
         );
 
         let parsed_part = MimePart::parse(encoded.clone()).unwrap();
-        k9::assert_equal!(encoded.as_str(), parsed_part.to_message_string().as_str());
+        k9::assert_equal!(encoded, parsed_part.to_message_bytes());
         k9::assert_equal!(part.body().unwrap(), DecodedBody::Text(input_text.into()));
         k9::snapshot!(
             parsed_part.simplified_structure_pointers(),
@@ -1644,7 +1644,7 @@ Ok(
         )
         .unwrap();
         k9::snapshot!(
-            msg.to_message_string(),
+            BString::from(msg.to_message_bytes()),
             r#"
 Content-Type: multipart/mixed;\r
 \tboundary="my-boundary"\r
@@ -1852,7 +1852,7 @@ Ok(
         let rebuilt = part.rebuild(None).unwrap();
 
         k9::snapshot!(
-            rebuilt.to_message_string(),
+            BString::from(rebuilt.to_message_bytes()),
             r#"
 Content-Type: multipart/mixed;\r
 \tboundary="8a54d64d7ad7c04a084478052b36cbe1609b33bf3a41203aaee8dd642cd3"\r
@@ -1912,8 +1912,8 @@ Hello";
             "Message has conformance issues: MISSING_MESSAGE_ID_HEADER"
         );
 
-        let rebuilt = msg
-            .check_fix_conformance(
+        let rebuilt = BString::from(
+            msg.check_fix_conformance(
                 MessageConformance::MISSING_MESSAGE_ID_HEADER,
                 MessageConformance::MISSING_MESSAGE_ID_HEADER,
                 CheckFixSettings {
@@ -1923,7 +1923,8 @@ Hello";
             )
             .unwrap()
             .unwrap()
-            .to_message_string();
+            .to_message_bytes(),
+        );
 
         k9::snapshot!(
             rebuilt,
@@ -1947,8 +1948,8 @@ Hello this is a really long line Hello this is a really long line \
 Hello this is a really long line Hello this is a really long line
 ";
         let msg = MimePart::parse(DOUBLE_ANGLE_AND_LONG_LINE).unwrap();
-        let rebuilt = msg
-            .check_fix_conformance(
+        let rebuilt = BString::from(
+            msg.check_fix_conformance(
                 MessageConformance::MISSING_COLON_VALUE,
                 MessageConformance::MISSING_MESSAGE_ID_HEADER | MessageConformance::LINE_TOO_LONG,
                 CheckFixSettings {
@@ -1958,7 +1959,8 @@ Hello this is a really long line Hello this is a really long line
             )
             .unwrap()
             .unwrap()
-            .to_message_string();
+            .to_message_bytes(),
+        );
 
         k9::snapshot!(
             rebuilt,
@@ -1987,15 +1989,16 @@ y long line=0A\r
         "X-Hello: there\r\nX-Header: value\r\nSubject: Hello\r\nX-Header: another value\r\nFrom :Someone@somewhere\r\n\r\nBody";
 
         let msg = MimePart::parse(MULTI_HEADER_CONTENT).unwrap();
-        let rebuilt = msg
-            .check_fix_conformance(
+        let rebuilt = BString::from(
+            msg.check_fix_conformance(
                 MessageConformance::default(),
                 MessageConformance::MISSING_MIME_VERSION,
                 CheckFixSettings::default(),
             )
             .unwrap()
             .unwrap()
-            .to_message_string();
+            .to_message_bytes(),
+        );
         k9::snapshot!(
             rebuilt,
             r#"
@@ -2011,15 +2014,16 @@ Body
         );
 
         let msg = MimePart::parse(MULTI_HEADER_CONTENT).unwrap();
-        let rebuilt = msg
-            .check_fix_conformance(
+        let rebuilt = BString::from(
+            msg.check_fix_conformance(
                 MessageConformance::default(),
                 MessageConformance::MISSING_MIME_VERSION | MessageConformance::NAME_ENDS_WITH_SPACE,
                 CheckFixSettings::default(),
             )
             .unwrap()
             .unwrap()
-            .to_message_string();
+            .to_message_bytes(),
+        );
         k9::snapshot!(
             rebuilt,
             r#"

crates/mailparsing/src/rfc5322_parser.rs

@@ -2767,7 +2767,7 @@ Some(
         );
 
         k9::snapshot!(
-            msg.rebuild(None).unwrap().to_message_string(),
+            BString::from(msg.rebuild(None).unwrap().to_message_bytes()),
             r#"
 Content-Type: text/plain;\r
 \tcharset="us-ascii"\r

crates/mailparsing/src/strings.rs

@@ -200,3 +200,13 @@ impl<'a> IntoSharedString<'a> for &'a [u8] {
         }
     }
 }
+
+impl<'a> IntoSharedString<'a> for BString {
+    fn into_shared_string(self) -> (SharedString<'a>, MessageConformance) {
+        let bytes: Vec<u8> = self.into();
+        match std::str::from_utf8(&bytes) {
+            Ok(_) => (bytes.into(), MessageConformance::default()),
+            Err(_) => (bytes.into(), MessageConformance::NEEDS_TRANSFER_ENCODING),
+        }
+    }
+}

crates/message/src/message.rs

@@ -1238,8 +1238,8 @@ impl Message {
                     text.push_str(content);
                     p.replace_text_body("text/plain", &*text)?;
 
-                    let new_data = msg.to_message_string();
-                    self.assign_data(new_data.into_bytes());
+                    let new_data = msg.to_message_bytes();
+                    self.assign_data(new_data);
                     Ok(true)
                 }
                 DecodedBody::Binary(_) => {
@@ -1274,8 +1274,8 @@ impl Message {
 
                     p.replace_text_body("text/html", &*text)?;
 
-                    let new_data = msg.to_message_string();
-                    self.assign_data(new_data.into_bytes());
+                    let new_data = msg.to_message_bytes();
+                    self.assign_data(new_data);
                     Ok(true)
                 }
                 DecodedBody::Binary(_) => {
@@ -1316,8 +1316,8 @@ impl Message {
         let opt_msg = msg.check_fix_conformance(check, fix, settings)?;
 
         if let Some(msg) = opt_msg {
-            let new_data = msg.to_message_string();
-            self.assign_data(new_data.into_bytes());
+            let new_data = msg.to_message_bytes();
+            self.assign_data(new_data);
         }
 
         Ok(())

crates/message/test_legacy_jp_encodings.lua

@@ -10,45 +10,62 @@ local JP_UTF8 =
 local SHIFT_JIS = kumo.encode.charset_encode('shift-jis', JP_UTF8)
 utils.assert_eq(kumo.encode.charset_decode('shift-jis', SHIFT_JIS), JP_UTF8)
 
-local BODY = 'From: someone@example.com\r\nSubject: Testing\r\n\r\n'
+local BODY = 'From: someone@example.com\r\nContent-Type: text/plain; charset="shift-jis"\r\nSubject: Testing\r\n\r\n'
   .. SHIFT_JIS
-local msg = new_msg(BODY)
 
-local mime_before = msg:parse_mime()
--- We might hope that we get the SHIFT_JIS bytes back out for the body,
--- but we'll try to interpret it as UTF-8 for internationalize email's sake,
--- and this input just happens to look like plausible UTF-8, even though
--- it is complete nonsense
-utils.assert_ne(mime_before.body, SHIFT_JIS)
+local function test_basics()
+  local msg = new_msg(BODY)
 
--- Interesting to note that the charset-normalizer based utf-8 decoder
--- won't allow converting these shift-jis bytes to utf-8, but Rust's
--- own built in str::from_utf8 will.
--- utils.assert_eq(mime_before.body, kumo.encode.charset_decode('utf-8', SHIFT_JIS))
+  local mime_before = msg:parse_mime()
+  -- We might hope that we get the SHIFT_JIS bytes back out for the body,
+  -- but we'll try to interpret it as UTF-8 for internationalize email's sake,
+  -- and this input just happens to look like plausible UTF-8, even though
+  -- it is complete nonsense
+  utils.assert_ne(mime_before.body, SHIFT_JIS)
 
--- Can we dkim-sign this binary content?
-local signer = kumo.dkim.rsa_sha256_signer {
-  domain = 'example.com',
-  selector = 'default',
-  headers = { 'From', 'Subject' },
-  key = 'example-private-dkim-key.pem',
-}
-msg:dkim_sign(signer)
--- kumo.log_info(msg:get_data())
+  -- Interesting to note that the charset-normalizer based utf-8 decoder
+  -- won't allow converting these shift-jis bytes to utf-8, but Rust's
+  -- own built in str::from_utf8 will.
+  -- utils.assert_eq(mime_before.body, kumo.encode.charset_decode('utf-8', SHIFT_JIS))
 
--- Use encoding detection to fix the encoding up. Note that this
--- definitely breaks the dkim signature, but we don't care in the
--- context of this test; the above is testing whether can sign
--- the wonky encoding, the below is testing whether we can fix
--- it.  In practice, a given node will be doing either one or
--- the other.
-msg:check_fix_conformance('', 'NEEDS_TRANSFER_ENCODING', {
-  detect_encoding = true,
-  include_encodings = {
-    'shift-jis',
-  },
-  exclude_encodings = {},
-})
+  -- Can we dkim-sign this binary content?
+  local signer = kumo.dkim.rsa_sha256_signer {
+    domain = 'example.com',
+    selector = 'default',
+    headers = { 'From', 'Subject' },
+    key = 'example-private-dkim-key.pem',
+  }
+  msg:dkim_sign(signer)
+  -- kumo.log_info(msg:get_data())
 
-local mime = msg:parse_mime()
-utils.assert_eq(mime.body, JP_UTF8)
+  -- Use encoding detection to fix the encoding up. Note that this
+  -- definitely breaks the dkim signature, but we don't care in the
+  -- context of this test; the above is testing whether can sign
+  -- the wonky encoding, the below is testing whether we can fix
+  -- it.  In practice, a given node will be doing either one or
+  -- the other.
+  msg:check_fix_conformance('', 'NEEDS_TRANSFER_ENCODING', {
+    detect_encoding = true,
+    include_encodings = {
+      'shift-jis',
+    },
+    exclude_encodings = {},
+  })
+
+  local mime = msg:parse_mime()
+  utils.assert_eq(mime.body, JP_UTF8)
+end
+
+local function test_fix_missing_headers()
+  local msg = new_msg(BODY)
+  local res = msg:check_fix_conformance(
+    '',
+    'MISSING_DATE_HEADER|MISSING_MESSAGE_ID_HEADER|MISSING_MIME_VERSION'
+  )
+
+  local mime = msg:parse_mime()
+  utils.assert_ne(mime.body, SHIFT_JIS)
+end
+
+test_basics()
+test_fix_missing_headers()

crates/mod-mimepart/src/mimepart.rs

@@ -169,9 +169,9 @@ impl UserData for PartRef {
             Ok(result)
         });
 
-        methods.add_meta_method(MetaMethod::ToString, move |_lua, this, ()| {
+        methods.add_meta_method(MetaMethod::ToString, move |lua, this, ()| {
             let root = this.root_part.lock();
-            Ok(root.to_message_string())
+            lua.create_string(root.to_message_bytes())
         });
 
         methods.add_method("rebuild", move |_lua, this, ()| {