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

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

thanks for the update Shannon!

in addition to the previous tests, i tested the new base64 CRLF
normalization and filename sanitization, which passed too.

looks good to me! :)

Tested-by: Samuel Rufinatscha
Reviewed-by: Samuel Rufinatscha

On 10/20/26 4:35 PM, 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, CR bytes will be dropped, to keep
> the behavior for encoded and non-encoded text aligned. to comply with
> rfc 2045 section 6.8 and rfc 2047 section 4.1 line endings are also
> normalized before encoding them in headers and body text [1,2].
> 
> for filenames we now strip all control characters other than space
> (0x20). this should improve compliance with rfc 6266 section 4.3 [3].
> while that section specifies the recipient should do such stripping,
> doing so before sending minimizes compatibility issues and avoids
> transfering useless information.
> 
> also adds tests to encode this behavior.
> 
> [1]: https://datatracker.ietf.org/doc/html/rfc2045#section-6.8
> [2]: https://datatracker.ietf.org/doc/html/rfc2047#section-4.1
> [3]: https://datatracker.ietf.org/doc/html/rfc6266/#section-4.3
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
> 
> Notes:
>      v2: https://lore.proxmox.com/all/20260730140126.444424-1-s.sterz@proxmox.com/
>      
>      changes since v2 (thanks @ Samuel Rufinatscha):
>      
>      * normalize line endings to CRLF and preserve existing CRLF line
>        endings before base64 encoding according to rfc 2045 and 2047.
>      * remove all control characters from filenames not just CR.
>      * improve documentation.
>      
>      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 | 280 ++++++++++++++++++++++++++++++++----
>   1 file changed, 256 insertions(+), 24 deletions(-)
> 
> diff --git a/proxmox-sendmail/src/lib.rs b/proxmox-sendmail/src/lib.rs
> index b3ff56e5..45868c0f 100644
> --- a/proxmox-sendmail/src/lib.rs
> +++ b/proxmox-sendmail/src/lib.rs
> @@ -63,6 +63,30 @@ fn encode_filename_formatted(filename: &str) -> String {
>       format_encoded_text(&encoded, format_prefix, ";", 0, true)
>   }
>   
> +/// Normalizes line endings before base64 encoding bytes. Stripping bare CR bytes is necessary
> +/// according to RFC 5321 Section 2.3.8
> +/// (https://datatracker.ietf.org/doc/html/rfc5321#section-2.3.8). RFC 2045 Section 6.8
> +/// (https://datatracker.ietf.org/doc/html/rfc2045#section-6.8) and RFC 2047 Section 4.1
> +/// (https://datatracker.ietf.org/doc/html/rfc2047#section-4.1) also specify that line endings
> +/// should be normalized to CRLF prior to base64 encoding in mime body text and header values.
> +fn normalize_line_endings_base64<T: AsRef<[u8]>>(raw: T) -> String {
> +    // 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: Vec<u8> = Vec::with_capacity(raw.as_ref().len() * 2);
> +
> +    for byte in raw.as_ref() {
> +        match byte {
> +            b'\r' => continue,
> +            b'\n' => normalized.extend_from_slice(b"\r\n"),
> +            b => normalized.push(*b),
> +        }
> +    }
> +
> +    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
> @@ -168,7 +192,7 @@ impl Recipient {
>               if !name.is_ascii() {
>                   format!(
>                       "=?utf-8?B?{}?= <{}>",
> -                    proxmox_base64::encode(name),
> +                    normalize_line_endings_base64(name),
>                       self.email
>                   )
>               } else {
> @@ -199,7 +223,7 @@ impl Attachment<'_> {
>           if self.filename.is_ascii() {
>               let _ = write!(attachment, "{}", &self.filename);
>           } else {
> -            let encoded = proxmox_base64::encode(&self.filename);
> +            let encoded = normalize_line_endings_base64(&self.filename);
>               let prefix_fn = |line| {
>                   if Some(0) == line {
>                       "=?utf-8?B?"
> @@ -325,12 +349,27 @@ impl<'a> Mail<'a> {
>           self
>       }
>   
> -    /// Adds an attachment with a specified file name and mime-type to an email.
> +    /// Adds an attachment with a specified filename and mime-type to an email. The filename will be
> +    /// stripped of control characters for better compatibility.
>       ///
>       /// Note: Adding attachments triggers `multipart/mixed` mode.
>       pub fn add_attachment(&mut self, filename: &str, mime_type: &str, content: &'a [u8]) {
> +        // Since we strip control characters the length of the original filename is an absolute
> +        // upper bound of the size of the final string.
> +        let mut normalized: String = String::with_capacity(filename.len());
> +
> +        // RFC 6266 Section 4.3 (https://datatracker.ietf.org/doc/html/rfc6266/#section-4.3) says
> +        // that recipients should remove things such as control characters from a filename. So since
> +        // these should be stripped anyway, do that here already to avoid encoding them later.
> +        for character in filename.chars() {
> +            match character {
> +                c if c <= '\x1f' || c == '\x7f' => continue,
> +                c => normalized.push(c),
> +            }
> +        }
> +
>           self.attachments.push(Attachment {
> -            filename: filename.to_string(),
> +            filename: normalized,
>               mime: mime_type.to_string(),
>               content,
>           });
> @@ -451,7 +490,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 +527,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\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 +576,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 +619,7 @@ impl<'a> Mail<'a> {
>               writeln!(
>                   header,
>                   "Subject: =?utf-8?B?{}?=",
> -                proxmox_base64::encode(&self.subject)
> +                normalize_line_endings_base64(&self.subject)
>               )?;
>           } else {
>               writeln!(header, "Subject: {}", self.subject)?;
> @@ -571,7 +629,7 @@ impl<'a> Mail<'a> {
>               writeln!(
>                   header,
>                   "From: =?utf-8?B?{}?= <{}>",
> -                proxmox_base64::encode(&self.mail_author),
> +                normalize_line_endings_base64(&self.mail_author),
>                   self.mail_from
>               )?;
>           } else {
> @@ -606,16 +664,18 @@ 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)
> +                    normalize_line_endings_base64(&value)
>                   )?;
>               }
>           }
> @@ -653,7 +713,11 @@ 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));
> +            // normalize line endings according to RFC 2045 Section 6.8
> +            // (https://datatracker.ietf.org/doc/html/rfc2045#section-6.8).
> +            body.push_str(&encode_base64_formatted(
> +                self.body_txt.replace('\r', "").replace('\n', "\r\n"),
> +            ));
>           }
>   
>           if let Some(html) = &self.body_html {
> @@ -666,7 +730,11 @@ 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));
> +                // normalize line endings according to RFC 2045 Section 6.8
> +                // (https://datatracker.ietf.org/doc/html/rfc2045#section-6.8).
> +                body.push_str(&encode_base64_formatted(
> +                    html.replace('\r', "").replace('\n', "\r\n"),
> +                ));
>               }
>   
>               write!(body, "\n--{html_boundary}--")?;
> @@ -895,8 +963,8 @@ Content-Type: text/plain;
>   	charset="UTF-8"
>   Content-Transfer-Encoding: base64
>   
> -VGhpcyB1dGYtOCBlbWFpbCBzaG91bGQgaGFuZGxlIGVtb2ppcwrwn6eR8J+TpwphbmQgd2Vp
> -cmQgZ2VybWFuIGNoYXJhY3RlcnM6IMO2w6TDvMOfCmFuZCBtb3JlLg=="#,
> +VGhpcyB1dGYtOCBlbWFpbCBzaG91bGQgaGFuZGxlIGVtb2ppcw0K8J+nkfCfk6cNCmFuZCB3
> +ZWlyZCBnZXJtYW4gY2hhcmFjdGVyczogw7bDpMO8w58NCmFuZCBtb3JlLg=="#,
>           )
>       }
>   
> @@ -1140,10 +1208,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)
> @@ -1176,14 +1244,14 @@ Content-Type: text/plain;
>   	charset="UTF-8"
>   Content-Transfer-Encoding: base64
>   
> -TG9yZW0gSXBzdW0gRG9sb3IgU2l0CkFtw6t0
> +TG9yZW0gSXBzdW0gRG9sb3IgU2l0DQpBbcOrdA==
>   ------_=_NextPart_002_1732806251
>   Content-Type: text/html;
>   	charset="UTF-8"
>   Content-Transfer-Encoding: base64
>   
> -PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4KCTxwcmU+CgkJTG9yZW0g
> -SXBzdW0gRMO2bG9yIFNpdCBBbWV0Cgk8L3ByZT4KPC9ib2R5PjwvaHRtbD4=
> +PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4NCgk8cHJlPg0KCQlMb3Jl
> +bSBJcHN1bSBEw7Zsb3IgU2l0IEFtZXQNCgk8L3ByZT4NCjwvYm9keT48L2h0bWw+
>   ------_=_NextPart_002_1732806251--
>   ------_=_NextPart_001_1732806251
>   Content-Type: application/octet-stream;
> @@ -1344,4 +1412,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 \r\nDolor Sit\nAmët",
> +        )
> +        .with_recipient_and_name("R\receiver Näme", "receive\rr@example.com")
> +        .with_attachment("🐄💀.\x7fb\rin", "ima\rge/bmp", &bin)
> +        .with_html_alt("<html lang=\"de-at\"><head></head><body>\n\t<pre>\r\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
> +
> +TG9yZW0gSXBzdW0gDQpEb2xvciBTaXQNCkFtw6t0
> +------_=_NextPart_002_1732806251
> +Content-Type: text/html;
> +	charset="UTF-8"
> +Content-Transfer-Encoding: base64
> +
> +PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4NCgk8cHJlPg0KCQlMb3Jl
> +bSBJcHN1bSBEw7Zsb3IgU2l0IEFtZXQNCgk8L3ByZT4NCjwvYm9keT48L2h0bWw+
> +------_=_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());
> +    }
>   }