Re: [PATCH proxmox] fix #7814: send-mail: normalize LF line endings to CRLF endings

["Shannon Sterz" <s.sterz@proxmox.com>]

Sorry this is also a v2...

On Thu Jul 30, 2026 at 4:01 PM CEST, Shannon Sterz wrote:
> RFC5321 Section 2.3.8 requires that mails do not contain bare CR or LF
> bytes. this patch normalizes unix style LF line endings to CRLF line
> endings. it also removes bare CR bytes.
>
> when text would be base64 encoded anyway, CR bytes will be dropped, to
> keep the behavior for encoded and non-encoded text aligned. however,
> since this code was already extensively tested against multiple MUAs,
> encoded LF line endings are kept as-is.
>
> also adds tests to encode this behavior.
>
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>
> Notes:
>     v1: https://lore.proxmox.com/all/20260714084719.68076-1-s.sterz@proxmox.com/
>
>     changes since v1 (thanks @ Lukas Wagner):
>
>     * fixed a typo (than -> then)
>     * adjusted how the capacity for forwarded messages that need to be
>       normalized is calculated.
>
>  proxmox-sendmail/src/lib.rs | 244 ++++++++++++++++++++++++++++++++----
>  1 file changed, 218 insertions(+), 26 deletions(-)
>
> diff --git a/proxmox-sendmail/src/lib.rs b/proxmox-sendmail/src/lib.rs
> index b3ff56e5..355ed12d 100644
> --- a/proxmox-sendmail/src/lib.rs
> +++ b/proxmox-sendmail/src/lib.rs
> @@ -50,7 +50,10 @@ fn encode_base64_formatted<T: AsRef<[u8]>>(raw: T) -> String {
>  }
>
>  fn encode_filename_formatted(filename: &str) -> String {
> -    let encoded = format!("UTF-8''{}", utf8_percent_encode(filename, RFC5987SET));
> +    let encoded = format!(
> +        "UTF-8''{}",
> +        utf8_percent_encode(&filename.replace('\r', ""), RFC5987SET)
> +    );
>
>      let format_prefix = |l| {
>          if let Some(l) = l {
> @@ -63,6 +66,18 @@ fn encode_filename_formatted(filename: &str) -> String {
>      format_encoded_text(&encoded, format_prefix, ";", 0, true)
>  }
>
> +/// Strip bare CR bytes, then base64 encode. This is useful to make sure we treat encoded (header)
> +/// values the same as non-encoded one. Stripping CR bytes is necessary according to RFC 5321 Section
> +/// Section 2.3.8 (https://datatracker.ietf.org/doc/html/rfc5321#section-2.3.8).
> +fn strip_cr_base64<T: AsRef<[u8]>>(raw: T) -> String {
> +    let normalized: Vec<u8> = raw
> +        .as_ref()
> +        .iter()
> +        .filter_map(|b| (*b != b'\r').then_some(*b))
> +        .collect();
> +    proxmox_base64::encode(&normalized)
> +}
> +
>  /// Helper function used to format text to not exceed line limits imposed by standards such as RFC
>  /// 2045 Section 6.8 that specifies that Base64 content transfer encoded text "must be represented
>  /// in lines of no more than 76 characters each". It also allows taking into account any necessary
> @@ -166,11 +181,7 @@ impl Recipient {
>      fn format_recipient(&self) -> String {
>          if let Some(name) = &self.name {
>              if !name.is_ascii() {
> -                format!(
> -                    "=?utf-8?B?{}?= <{}>",
> -                    proxmox_base64::encode(name),
> -                    self.email
> -                )
> +                format!("=?utf-8?B?{}?= <{}>", strip_cr_base64(name), self.email)
>              } else {
>                  format!("{name} <{}>", self.email)
>              }
> @@ -199,7 +210,7 @@ impl Attachment<'_> {
>          if self.filename.is_ascii() {
>              let _ = write!(attachment, "{}", &self.filename);
>          } else {
> -            let encoded = proxmox_base64::encode(&self.filename);
> +            let encoded = strip_cr_base64(&self.filename);
>              let prefix_fn = |line| {
>                  if Some(0) == line {
>                      "=?utf-8?B?"
> @@ -451,7 +462,10 @@ impl<'a> Mail<'a> {
>
>      /// Forwards an email message to a given list of recipients.
>      ///
> -    /// `message` must be compatible with ``sendmail`` (the message is piped into stdin unmodified).
> +    /// `message` must be compatible with ``sendmail``. The message is piped into `stdin`
> +    /// unmodified. One exception is stripping bare CR bytes and normalizing LF line endings to
> +    /// CRLF line endings in accordance with RFC 5321 Section 2.3.8
> +    /// (https://datatracker.ietf.org/doc/html/rfc5321#section-2.3.8).
>      #[cfg(feature = "mail-forwarder")]
>      pub fn forward(
>          mailto: &[&str],
> @@ -485,11 +499,25 @@ impl<'a> Mail<'a> {
>              .spawn()
>              .with_context(|| "could not spawn sendmail process")?;
>
> +        // this will always have enough capacity, since the worst case is a message consisting only
> +        // of "\n" and that would exactly double in size. messages are also unlikely to be that big
> +        // that the extra allocated space here really matters. so set the capacity to the upper
> +        // bound to avoid more complex capacity calculations that would take more time to run.
> +        let mut normalized_message: Vec<u8> = Vec::with_capacity(message.len() * 2);
> +
> +        for byte in message {
> +            match byte {
> +                b'\r' => continue,
> +                b'\n' => normalized_message.extend_from_slice(&[b'\r', b'\n']),
> +                b => normalized_message.push(*b),
> +            }
> +        }
> +
>          sendmail_process
>              .stdin
>              .take()
>              .unwrap()
> -            .write_all(message)
> +            .write_all(&normalized_message)
>              .with_context(|| "couldn't write to sendmail stdin")?;
>
>          sendmail_process
> @@ -520,7 +548,9 @@ impl<'a> Mail<'a> {
>              write!(mail, "\n--{file_boundary}--")?;
>          }
>
> -        Ok(mail)
> +        // normalize line endings from `\n` to `\r\n` and drop bare `\r` according to RFC 5321
> +        // 2.3.8: https://datatracker.ietf.org/doc/html/rfc5321#section-2.3.8
> +        Ok(mail.replace('\r', "").replace('\n', "\r\n"))
>      }
>
>      fn format_header(
> @@ -561,7 +591,7 @@ impl<'a> Mail<'a> {
>              writeln!(
>                  header,
>                  "Subject: =?utf-8?B?{}?=",
> -                proxmox_base64::encode(&self.subject)
> +                strip_cr_base64(&self.subject)
>              )?;
>          } else {
>              writeln!(header, "Subject: {}", self.subject)?;
> @@ -571,7 +601,7 @@ impl<'a> Mail<'a> {
>              writeln!(
>                  header,
>                  "From: =?utf-8?B?{}?= <{}>",
> -                proxmox_base64::encode(&self.mail_author),
> +                strip_cr_base64(&self.mail_author),
>                  self.mail_from
>              )?;
>          } else {
> @@ -606,17 +636,15 @@ impl<'a> Mail<'a> {
>          header.push_str("Auto-Submitted: auto-generated;\n");
>
>          for (key, value) in &self.headers {
> -            // Strip CR/LF so an interpolated key or value cannot inject extra header lines.
> -            let key = key.replace(['\r', '\n'], "");
> -            let value = value.replace(['\r', '\n'], "");
> +            // Strip LF so an interpolated key or value cannot inject extra header lines. Bare CR
> +            // will be stripped for non-ASCII text before encoding and for ASCII text globally
> +            // before finishing formatting.
> +            let key = key.replace('\n', "");
> +            let value = value.replace('\n', "");
>              if value.is_ascii() {
>                  writeln!(header, "{key}: {value}")?;
>              } else {
> -                writeln!(
> -                    header,
> -                    "{key}: =?utf-8?B?{}?=",
> -                    proxmox_base64::encode(&value)
> -                )?;
> +                writeln!(header, "{key}: =?utf-8?B?{}?=", strip_cr_base64(&value))?;
>              }
>          }
>
> @@ -653,7 +681,7 @@ impl<'a> Mail<'a> {
>              body.push_str(&self.body_txt);
>          } else {
>              body.push_str("Content-Transfer-Encoding: base64\n\n");
> -            body.push_str(&encode_base64_formatted(&self.body_txt));
> +            body.push_str(&encode_base64_formatted(self.body_txt.replace('\r', "")));
>          }
>
>          if let Some(html) = &self.body_html {
> @@ -666,7 +694,7 @@ impl<'a> Mail<'a> {
>                  body.push_str(html);
>              } else {
>                  body.push_str("Content-Transfer-Encoding: base64\n\n");
> -                body.push_str(&encode_base64_formatted(html));
> +                body.push_str(&encode_base64_formatted(html.replace('\r', "")));
>              }
>
>              write!(body, "\n--{html_boundary}--")?;
> @@ -1140,10 +1168,10 @@ Content-Transfer-Encoding: 7bit
>          ];
>
>          let mail = Mail::new(
> -            "Sender Näme",
> -            "from@example.com",
> -            "Subject Linë",
> -            "Lorem Ipsum Dolor Sit\nAmët",
> +            "Sender Nä\rme",
> +            "from@example.\rcom",
> +            "Subject Linë\r",
> +            "Lorem Ipsum Dolor \rSit\nAmët",
>          )
>          .with_recipient_and_name("Receiver Näme", "receiver@example.com")
>          .with_attachment("deadbeef.bin", "application/octet-stream", &bin)
> @@ -1344,4 +1372,168 @@ ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz
>
>          assert_eq!(suffixed, out);
>      }
> +
> +    #[test]
> +    fn line_endings_are_normalized_when_not_encoded() {
> +        let bin: [u8; 63] = [
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0x0d,
> +        ];
> +
> +        let mail = Mail::new(
> +            "Se\rnder Name",
> +            "fro\rm@example.com",
> +            "Sub\rject Line",
> +            "Lor\rem Ipsum Dolor Sit\nAmet",
> +        )
> +        .with_recipient_and_name("R\receiver Name", "receive\rr@example.com")
> +        .with_attachment("deadb\reef.bin", "application/octet-stream", &bin)
> +        .with_html_alt("<html lang=\"de-at\"><head></head><body>\n\t<pre>\n\t\tLorem Ipsum Dolor Si\rt Amet\n\t</pre>\n</body></html>")
> +        .with_header("X-CR-HEADE\r", "value\r");
> +
> +        let body = mail.format_mail(1732806251).expect("could not format mail");
> +
> +        assert!(body.is_ascii());
> +
> +        let expected = r#"Content-Type: multipart/mixed;
> +	boundary="----_=_NextPart_001_1732806251"
> +MIME-Version: 1.0
> +Subject: Subject Line
> +From: Sender Name <from@example.com>
> +To: Receiver Name <receiver@example.com>
> +Date: Thu, 28 Nov 2024 16:04:11 +0100
> +Auto-Submitted: auto-generated;
> +X-CR-HEADE: value
> +
> +This is a multi-part message in MIME format.
> +
> +------_=_NextPart_001_1732806251
> +Content-Type: multipart/alternative; boundary="----_=_NextPart_002_1732806251"
> +MIME-Version: 1.0
> +
> +------_=_NextPart_002_1732806251
> +Content-Type: text/plain;
> +	charset="UTF-8"
> +Content-Transfer-Encoding: 7bit
> +
> +Lorem Ipsum Dolor Sit
> +Amet
> +------_=_NextPart_002_1732806251
> +Content-Type: text/html;
> +	charset="UTF-8"
> +Content-Transfer-Encoding: 7bit
> +
> +<html lang="de-at"><head></head><body>
> +	<pre>
> +		Lorem Ipsum Dolor Sit Amet
> +	</pre>
> +</body></html>
> +------_=_NextPart_002_1732806251--
> +------_=_NextPart_001_1732806251
> +Content-Type: application/octet-stream;
> +	name="deadbeef.bin"
> +Content-Disposition: attachment;
> +	filename*0*=UTF-8''deadbeef.bin
> +Content-Transfer-Encoding: base64
> +
> +3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7v
> +3q2+796tvu8N
> +------_=_NextPart_001_1732806251--"#;
> +
> +        // manually compare here to ensure we split specifically only on `\r\n` line endings
> +
> +        let lines1 = body.split("\r\n");
> +        let lines2 = expected.split("\n");
> +
> +        for ((i, line1), line2) in lines1.enumerate().zip(lines2) {
> +            if !(line1.starts_with("Date:") && line2.starts_with("Date:")) {
> +                assert_eq!(line1, line2, "lines at {i} do not match");
> +            }
> +        }
> +
> +        assert_eq!(body.lines().count(), expected.lines().count());
> +    }
> +
> +    #[test]
> +    fn line_endings_are_normalized_when_encoded() {
> +        let bin: [u8; 63] = [
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
> +            0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0x0d,
> +        ];
> +
> +        let mail = Mail::new(
> +            "Se\rnder Näme",
> +            "fro\rm@example.com",
> +            "Sub\rject Linë",
> +            "Lor\rem Ipsum Dolor Sit\nAmët",
> +        )
> +        .with_recipient_and_name("R\receiver Näme", "receive\rr@example.com")
> +        .with_attachment("🐄💀.b\rin", "ima\rge/bmp", &bin)
> +        .with_html_alt("<html lang=\"de-at\"><head></head><body>\n\t<pre>\n\t\tLorem Ipsum Dölor Si\rt Amet\n\t</pre>\n</body></html>")
> +        .with_header("X-CR-HEADE\r", "valüe\r");
> +
> +        let body = mail.format_mail(1732806251).expect("could not format mail");
> +
> +        assert!(body.is_ascii());
> +
> +        let expected = r#"Content-Type: multipart/mixed;
> +	boundary="----_=_NextPart_001_1732806251"
> +MIME-Version: 1.0
> +Subject: =?utf-8?B?U3ViamVjdCBMaW7Dqw==?=
> +From: =?utf-8?B?U2VuZGVyIE7DpG1l?= <from@example.com>
> +To: =?utf-8?B?UmVjZWl2ZXIgTsOkbWU=?= <receiver@example.com>
> +Date: Thu, 28 Nov 2024 16:04:11 +0100
> +Auto-Submitted: auto-generated;
> +X-CR-HEADE: =?utf-8?B?dmFsw7xl?=
> +
> +This is a multi-part message in MIME format.
> +
> +------_=_NextPart_001_1732806251
> +Content-Type: multipart/alternative; boundary="----_=_NextPart_002_1732806251"
> +MIME-Version: 1.0
> +
> +------_=_NextPart_002_1732806251
> +Content-Type: text/plain;
> +	charset="UTF-8"
> +Content-Transfer-Encoding: base64
> +
> +TG9yZW0gSXBzdW0gRG9sb3IgU2l0CkFtw6t0
> +------_=_NextPart_002_1732806251
> +Content-Type: text/html;
> +	charset="UTF-8"
> +Content-Transfer-Encoding: base64
> +
> +PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4KCTxwcmU+CgkJTG9yZW0g
> +SXBzdW0gRMO2bG9yIFNpdCBBbWV0Cgk8L3ByZT4KPC9ib2R5PjwvaHRtbD4=
> +------_=_NextPart_002_1732806251--
> +------_=_NextPart_001_1732806251
> +Content-Type: image/bmp;
> +	name="=?utf-8?B?8J+QhPCfkoAuYmlu?="
> +Content-Disposition: attachment;
> +	filename*0*=UTF-8''%F0%9F%90%84%F0%9F%92%80.bin
> +Content-Transfer-Encoding: base64
> +
> +3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7v
> +3q2+796tvu8N
> +------_=_NextPart_001_1732806251--"#;
> +
> +        // manually compare here to ensure we split specifically only on `\r\n` line endings
> +
> +        let lines1 = body.split("\r\n");
> +        let lines2 = expected.split("\n");
> +
> +        for ((i, line1), line2) in lines1.enumerate().zip(lines2) {
> +            if !(line1.starts_with("Date:") && line2.starts_with("Date:")) {
> +                assert_eq!(line1, line2, "lines at {i} do not match");
> +            }
> +        }
> +
> +        assert_eq!(body.lines().count(), expected.lines().count());
> +    }
>  }