public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines
@ 2026-02-19 14:58 Shannon Sterz
  2026-02-25  9:14 ` Lukas Wagner
  2026-02-25 11:22 ` superseded: " Shannon Sterz
  0 siblings, 2 replies; 4+ messages in thread
From: Shannon Sterz @ 2026-02-19 14:58 UTC (permalink / raw)
  To: pbs-devel

the http and mime Content-Disposition headers function slightly
differently. for the http version the following would be valid
according to rfc 6266 [1]:

Content-Disposition: attachment; filename="file.pdf";
    filename*=UTF-8''file.pdf

specifying multiple filename attributes like this isn't valid for mime
messages. according to rfc 2183 [2] the above should be expressed like
so for mime messages (one variant, multiple are supported):

Content-Disposition: attachment;
    filename*0*=UTF-8''file.bin

while rfc 2183 [2] would in theory allow for multiple filename
parameters similar to rfc 6266, in practice MIME::Tools [2] considers
this as ambigous content [3]. in more recent versions of Proxmox Mail
Gateway, such messages are rejected [4].

this patch implements proper filename handling and also wraps
filenames in the content disposition and content type headers
appropriatelly to a mail being rejected due to having mime headers
that are too long as described in rfcs 2231 [5] and 2045 [6].

[1]: https://datatracker.ietf.org/doc/html/rfc6266/
[2]: https://datatracker.ietf.org/doc/html/rfc2183#section-2
[3]: https://metacpan.org/pod/MIME::Head#ambiguous_content
[4]:
https://git.proxmox.com/?p=pmg-api.git;a=commitdiff;h=66f51c62f789b4c20308b7594fbbb357721b0844
[5]: https://datatracker.ietf.org/doc/html/rfc2231#section-7
[6]: https://datatracker.ietf.org/doc/html/rfc2045/#section-6.8

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---

tested this with peat on the following muas:

* aerc
* thunderbird
* ios mail app
* canary mail app
* start mail web client
* gmail web client
* outlook web client

noticed this when peat suddenly struggled to send mails. thanks @ Stoiko
Ivanov for helping with analysing this issue.

Changelog:
---------

* wrap lines more correctly ensuring that encoded lines don't exceed 76
  chars (thanks @ Stoiko Ivanov)
* refactored the wrapping logic to be more general

 proxmox-sendmail/src/lib.rs | 167 ++++++++++++++++++++++++++++--------
 1 file changed, 129 insertions(+), 38 deletions(-)

diff --git a/proxmox-sendmail/src/lib.rs b/proxmox-sendmail/src/lib.rs
index f5b04afd..ec8488a4 100644
--- a/proxmox-sendmail/src/lib.rs
+++ b/proxmox-sendmail/src/lib.rs
@@ -44,23 +44,83 @@ const RFC5987SET: &AsciiSet = &CONTROLS
 /// base64 encode and hard-wrap the base64 encoded string every 72 characters. this improves
 /// compatibility.
 fn encode_base64_formatted<T: AsRef<[u8]>>(raw: T) -> String {
-    const TEXT_WIDTH: usize = 72;
-
     let encoded = proxmox_base64::encode(raw);
-    let bytes = encoded.as_bytes();

-    let lines = bytes.len().div_ceil(TEXT_WIDTH);
-    let mut out = Vec::with_capacity(bytes.len() + lines - 1); // account for 1 newline per line
+    format_text(&encoded, |_| "".into(), "", 0, true)
+}

-    for (line, chunk) in bytes.chunks(TEXT_WIDTH).enumerate() {
+fn encode_filename_formatted(filename: &str) -> String {
+    let encoded = format!("UTF-8''{}", utf8_percent_encode(filename, RFC5987SET));
+
+    let format_prefix = |l| {
+        if let Some(l) = l {
+            format!("\tfilename*{l}*=")
+        } else {
+            "\tfilename*NN*=".into()
+        }
+    };
+
+    format_text(&encoded, format_prefix, ";", 0, true)
+}
+
+fn format_text<F>(
+    text: &str,
+    prefix: F,
+    suffix: &str,
+    shorten_first: usize,
+    skip_last_suffix: bool,
+) -> String
+where
+    F: Fn(Option<usize>) -> String,
+{
+    const DEFAULT_TEXT_WIDTH: usize = 72;
+
+    let bytes = text.as_bytes();
+    let suffix = suffix.as_bytes();
+
+    let format_text_len = suffix.len() + prefix(None).len();
+    let lines = (bytes.len() + shorten_first).div_ceil(DEFAULT_TEXT_WIDTH - format_text_len);
+
+    // bytes + formatting and "\n" per line - no "\n" in the last line
+    let mut capacity = bytes.len() + (format_text_len + 1) * lines - 1;
+
+    if skip_last_suffix {
+        capacity -= suffix.len();
+    }
+
+    let mut out = Vec::with_capacity(capacity);
+    let first_line_length = DEFAULT_TEXT_WIDTH - shorten_first - format_text_len;
+
+    let (total_lines, bytes) =
+        if let Some((first, rest)) = bytes.split_at_checked(first_line_length) {
+            out.extend_from_slice(prefix(Some(0)).as_bytes());
+            out.extend_from_slice(first);
+            out.extend_from_slice(suffix);
+            if !rest.is_empty() {
+                out.push(b'\n');
+            }
+            (1, rest)
+        } else {
+            (0, bytes)
+        };
+
+    for (line, chunk) in bytes
+        .chunks(DEFAULT_TEXT_WIDTH - format_text_len)
+        .enumerate()
+    {
+        let line = line + total_lines;
+        out.extend_from_slice(prefix(Some(line)).as_bytes());
         out.extend_from_slice(chunk);
+
         if line + 1 != lines {
-            // do not end last line with newline to give caller control over doing so.
+            out.extend_from_slice(suffix);
             out.push(b'\n');
+        } else if !skip_last_suffix {
+            out.extend_from_slice(suffix);
         }
     }

-    // SAFETY: base64 encoded, which is 7-bit chars (ASCII) and thus always valid UTF8
+    // SAFETY: all input slices were valid utf-8 string slices
     unsafe { String::from_utf8_unchecked(out) }
 }

@@ -104,24 +164,41 @@ impl Attachment<'_> {

         let mut attachment = String::new();

-        let encoded_filename = if self.filename.is_ascii() {
-            &self.filename
-        } else {
-            &format!("=?utf-8?B?{}?=", proxmox_base64::encode(&self.filename))
-        };
-
         let _ = writeln!(attachment, "\n--{file_boundary}");
-        let _ = writeln!(
-            attachment,
-            "Content-Type: {}; name=\"{encoded_filename}\"",
-            self.mime,
-        );

-        // both `filename` and `filename*` are included for additional compatibility
+        let _ = write!(attachment, "Content-Type: {};\n\tname=\"", self.mime);
+
+        if self.filename.is_ascii() {
+            let _ = write!(attachment, "{}", &self.filename);
+        } else {
+            let encoded = proxmox_base64::encode(&self.filename);
+            let prefix_fn = |line| {
+                if Some(0) == line {
+                    "=?utf-8?B?"
+                } else {
+                    "\t=?utf-8?B?"
+                }
+                .into()
+            };
+
+            // minus one here as the first line isn't indented
+            let skip_first = "\tname=\"".len() - 1;
+
+            let _ = write!(
+                attachment,
+                "{}",
+                format_text(&encoded, prefix_fn, "?=", skip_first, false)
+            );
+        }
+
+        // should be fine to add one here, the last line should be at most 72 chars. according to
+        // rfc 2045 encoded output lines should be "no more than 76 characters each"
+        let _ = writeln!(attachment, "\"");
+
         let _ = writeln!(
             attachment,
-            "Content-Disposition: attachment; filename=\"{encoded_filename}\";\n\tfilename*=UTF-8''{}",
-            utf8_percent_encode(&self.filename, RFC5987SET)
+            "Content-Disposition: attachment;\n{}",
+            encode_filename_formatted(&self.filename)
         );

         attachment.push_str("Content-Transfer-Encoding: base64\n\n");
@@ -809,9 +886,10 @@ Content-Transfer-Encoding: 7bit
 Lorem Ipsum Dolor Sit
 Amet
 ------_=_NextPart_001_1732806251
-Content-Type: application/octet-stream; name="deadbeef.bin"
-Content-Disposition: attachment; filename="deadbeef.bin";
-	filename*=UTF-8''deadbeef.bin
+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
@@ -838,7 +916,7 @@ Content-Transfer-Encoding: base64
         )
         .with_recipient_and_name("Receiver Name", "receiver@example.com")
         .with_attachment("deadbeef.bin", "application/octet-stream", &bin)
-        .with_attachment("🐄💀.bin", "image/bmp", &bin)
+        .with_attachment("🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀.bin", "image/bmp", &bin)
         .with_html_alt("<html lang=\"de-at\"><head></head><body>\n\t<pre>\n\t\tLorem Ipsum Dolor Sit Amet\n\t</pre>\n</body></html>");

         let body = mail.format_mail(1732806251).expect("could not format mail");
@@ -879,17 +957,28 @@ Content-Transfer-Encoding: 7bit
 </body></html>
 ------_=_NextPart_002_1732806251--
 ------_=_NextPart_001_1732806251
-Content-Type: application/octet-stream; name="deadbeef.bin"
-Content-Disposition: attachment; filename="deadbeef.bin";
-	filename*=UTF-8''deadbeef.bin
+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+796tvu8=
 ------_=_NextPart_001_1732806251
-Content-Type: image/bmp; name="=?utf-8?B?8J+QhPCfkoAuYmlu?="
-Content-Disposition: attachment; filename="=?utf-8?B?8J+QhPCfkoAuYmlu?=";
-	filename*=UTF-8''%F0%9F%90%84%F0%9F%92%80.bin
+Content-Type: image/bmp;
+	name="=?utf-8?B?8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE8J+Sg?=
+	=?utf-8?B?PCfkITwn5KA8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE?=
+	=?utf-8?B?8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoA?=
+	=?utf-8?B?uYmlu?="
+Content-Disposition: attachment;
+	filename*0*=UTF-8''%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F;
+	filename*1*=0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%8;
+	filename*2*=4%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%9;
+	filename*3*=2%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9;
+	filename*4*=F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F;
+	filename*5*=0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%8;
+	filename*6*=0%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80.bin
 Content-Transfer-Encoding: base64

 3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7v
@@ -997,17 +1086,19 @@ PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4KCTxwcmU+CgkJTG9yZW0g
 SXBzdW0gRMO2bG9yIFNpdCBBbWV0Cgk8L3ByZT4KPC9ib2R5PjwvaHRtbD4=
 ------_=_NextPart_002_1732806251--
 ------_=_NextPart_001_1732806251
-Content-Type: application/octet-stream; name="deadbeef.bin"
-Content-Disposition: attachment; filename="deadbeef.bin";
-	filename*=UTF-8''deadbeef.bin
+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+796tvu8=
 ------_=_NextPart_001_1732806251
-Content-Type: image/bmp; name="=?utf-8?B?8J+QhPCfkoAuYmlu?="
-Content-Disposition: attachment; filename="=?utf-8?B?8J+QhPCfkoAuYmlu?=";
-	filename*=UTF-8''%F0%9F%90%84%F0%9F%92%80.bin
+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
--
2.47.3





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines
  2026-02-19 14:58 [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines Shannon Sterz
@ 2026-02-25  9:14 ` Lukas Wagner
  2026-02-25 10:20   ` Shannon Sterz
  2026-02-25 11:22 ` superseded: " Shannon Sterz
  1 sibling, 1 reply; 4+ messages in thread
From: Lukas Wagner @ 2026-02-25  9:14 UTC (permalink / raw)
  To: Shannon Sterz, pbs-devel

On Thu Feb 19, 2026 at 3:58 PM CET, Shannon Sterz wrote:
> the http and mime Content-Disposition headers function slightly
> differently. for the http version the following would be valid
> according to rfc 6266 [1]:
>
> Content-Disposition: attachment; filename="file.pdf";
>     filename*=UTF-8''file.pdf
>
> specifying multiple filename attributes like this isn't valid for mime
> messages. according to rfc 2183 [2] the above should be expressed like
> so for mime messages (one variant, multiple are supported):
>
> Content-Disposition: attachment;
>     filename*0*=UTF-8''file.bin
>
> while rfc 2183 [2] would in theory allow for multiple filename
> parameters similar to rfc 6266, in practice MIME::Tools [2] considers
> this as ambigous content [3]. in more recent versions of Proxmox Mail
> Gateway, such messages are rejected [4].
>
> this patch implements proper filename handling and also wraps
> filenames in the content disposition and content type headers
> appropriatelly to a mail being rejected due to having mime headers
> that are too long as described in rfcs 2231 [5] and 2045 [6].
>
> [1]: https://datatracker.ietf.org/doc/html/rfc6266/
> [2]: https://datatracker.ietf.org/doc/html/rfc2183#section-2
> [3]: https://metacpan.org/pod/MIME::Head#ambiguous_content
> [4]:
> https://git.proxmox.com/?p=pmg-api.git;a=commitdiff;h=66f51c62f789b4c20308b7594fbbb357721b0844
> [5]: https://datatracker.ietf.org/doc/html/rfc2231#section-7
> [6]: https://datatracker.ietf.org/doc/html/rfc2045/#section-6.8
>

Looks mostly good, one comment inline. My suggestion can be fixed in a
follow-up patch, I don't want to block applying this.

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

That being said, I relied on your descriptions in the commit messages
and did not read through the RFCs myself.

[...]
> +
> +fn format_text<F>(
> +    text: &str,
> +    prefix: F,
> +    suffix: &str,
> +    shorten_first: usize,
> +    skip_last_suffix: bool,
> +) -> String
> +where
> +    F: Fn(Option<usize>) -> String,
> +{
> +    const DEFAULT_TEXT_WIDTH: usize = 72;
> +
> +    let bytes = text.as_bytes();
> +    let suffix = suffix.as_bytes();
> +
> +    let format_text_len = suffix.len() + prefix(None).len();
> +    let lines = (bytes.len() + shorten_first).div_ceil(DEFAULT_TEXT_WIDTH - format_text_len);
> +
> +    // bytes + formatting and "\n" per line - no "\n" in the last line
> +    let mut capacity = bytes.len() + (format_text_len + 1) * lines - 1;
> +
> +    if skip_last_suffix {
> +        capacity -= suffix.len();
> +    }
> +
> +    let mut out = Vec::with_capacity(capacity);
> +    let first_line_length = DEFAULT_TEXT_WIDTH - shorten_first - format_text_len;
> +
> +    let (total_lines, bytes) =
> +        if let Some((first, rest)) = bytes.split_at_checked(first_line_length) {
> +            out.extend_from_slice(prefix(Some(0)).as_bytes());
> +            out.extend_from_slice(first);
> +            out.extend_from_slice(suffix);
> +            if !rest.is_empty() {
> +                out.push(b'\n');
> +            }
> +            (1, rest)
> +        } else {
> +            (0, bytes)
> +        };
> +
> +    for (line, chunk) in bytes
> +        .chunks(DEFAULT_TEXT_WIDTH - format_text_len)
> +        .enumerate()
> +    {
> +        let line = line + total_lines;
> +        out.extend_from_slice(prefix(Some(line)).as_bytes());
>          out.extend_from_slice(chunk);
> +
>          if line + 1 != lines {
> -            // do not end last line with newline to give caller control over doing so.
> +            out.extend_from_slice(suffix);
>              out.push(b'\n');
> +        } else if !skip_last_suffix {
> +            out.extend_from_slice(suffix);
>          }
>      }
>
> -    // SAFETY: base64 encoded, which is 7-bit chars (ASCII) and thus always valid UTF8
> +    // SAFETY: all input slices were valid utf-8 string slices
>      unsafe { String::from_utf8_unchecked(out) }
>  }

It took me uncomfortably long to fully grasp what this function was
doing, some ideas that could help further readers to understand it more
quickly:

  - maybe some comments would be warranted? So maybe a doc comment for
    the inputs/outputs, and then in the code some comments explaining
    the steps (e.g. why the first line is handled differently, etc.)
  - make the function name a bit less generic (although I have a hard
    time coming up with a good one at the spot, I have to admit)
  - add one ore two test cases calling the function directly, so that
    one can see the inputs and expected outputs








^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines
  2026-02-25  9:14 ` Lukas Wagner
@ 2026-02-25 10:20   ` Shannon Sterz
  0 siblings, 0 replies; 4+ messages in thread
From: Shannon Sterz @ 2026-02-25 10:20 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: Lukas Wagner, pbs-devel

On Wed Feb 25, 2026 at 10:14 AM CET, Lukas Wagner wrote:
> On Thu Feb 19, 2026 at 3:58 PM CET, Shannon Sterz wrote:
>> the http and mime Content-Disposition headers function slightly
>> differently. for the http version the following would be valid
>> according to rfc 6266 [1]:
>>
>> Content-Disposition: attachment; filename="file.pdf";
>>     filename*=UTF-8''file.pdf
>>
>> specifying multiple filename attributes like this isn't valid for mime
>> messages. according to rfc 2183 [2] the above should be expressed like
>> so for mime messages (one variant, multiple are supported):
>>
>> Content-Disposition: attachment;
>>     filename*0*=UTF-8''file.bin
>>
>> while rfc 2183 [2] would in theory allow for multiple filename
>> parameters similar to rfc 6266, in practice MIME::Tools [2] considers
>> this as ambigous content [3]. in more recent versions of Proxmox Mail
>> Gateway, such messages are rejected [4].
>>
>> this patch implements proper filename handling and also wraps
>> filenames in the content disposition and content type headers
>> appropriatelly to a mail being rejected due to having mime headers
>> that are too long as described in rfcs 2231 [5] and 2045 [6].
>>
>> [1]: https://datatracker.ietf.org/doc/html/rfc6266/
>> [2]: https://datatracker.ietf.org/doc/html/rfc2183#section-2
>> [3]: https://metacpan.org/pod/MIME::Head#ambiguous_content
>> [4]:
>> https://git.proxmox.com/?p=pmg-api.git;a=commitdiff;h=66f51c62f789b4c20308b7594fbbb357721b0844
>> [5]: https://datatracker.ietf.org/doc/html/rfc2231#section-7
>> [6]: https://datatracker.ietf.org/doc/html/rfc2045/#section-6.8
>>
>
> Looks mostly good, one comment inline. My suggestion can be fixed in a
> follow-up patch, I don't want to block applying this.
>
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
>
> That being said, I relied on your descriptions in the commit messages
> and did not read through the RFCs myself.
>
> [...]
>> +
>> +fn format_text<F>(
>> +    text: &str,
>> +    prefix: F,
>> +    suffix: &str,
>> +    shorten_first: usize,
>> +    skip_last_suffix: bool,
>> +) -> String
>> +where
>> +    F: Fn(Option<usize>) -> String,
>> +{
>> +    const DEFAULT_TEXT_WIDTH: usize = 72;
>> +
>> +    let bytes = text.as_bytes();
>> +    let suffix = suffix.as_bytes();
>> +
>> +    let format_text_len = suffix.len() + prefix(None).len();
>> +    let lines = (bytes.len() + shorten_first).div_ceil(DEFAULT_TEXT_WIDTH - format_text_len);
>> +
>> +    // bytes + formatting and "\n" per line - no "\n" in the last line
>> +    let mut capacity = bytes.len() + (format_text_len + 1) * lines - 1;
>> +
>> +    if skip_last_suffix {
>> +        capacity -= suffix.len();
>> +    }
>> +
>> +    let mut out = Vec::with_capacity(capacity);
>> +    let first_line_length = DEFAULT_TEXT_WIDTH - shorten_first - format_text_len;
>> +
>> +    let (total_lines, bytes) =
>> +        if let Some((first, rest)) = bytes.split_at_checked(first_line_length) {
>> +            out.extend_from_slice(prefix(Some(0)).as_bytes());
>> +            out.extend_from_slice(first);
>> +            out.extend_from_slice(suffix);
>> +            if !rest.is_empty() {
>> +                out.push(b'\n');
>> +            }
>> +            (1, rest)
>> +        } else {
>> +            (0, bytes)
>> +        };
>> +
>> +    for (line, chunk) in bytes
>> +        .chunks(DEFAULT_TEXT_WIDTH - format_text_len)
>> +        .enumerate()
>> +    {
>> +        let line = line + total_lines;
>> +        out.extend_from_slice(prefix(Some(line)).as_bytes());
>>          out.extend_from_slice(chunk);
>> +
>>          if line + 1 != lines {
>> -            // do not end last line with newline to give caller control over doing so.
>> +            out.extend_from_slice(suffix);
>>              out.push(b'\n');
>> +        } else if !skip_last_suffix {
>> +            out.extend_from_slice(suffix);
>>          }
>>      }
>>
>> -    // SAFETY: base64 encoded, which is 7-bit chars (ASCII) and thus always valid UTF8
>> +    // SAFETY: all input slices were valid utf-8 string slices
>>      unsafe { String::from_utf8_unchecked(out) }
>>  }
>
> It took me uncomfortably long to fully grasp what this function was
> doing, some ideas that could help further readers to understand it more
> quickly:
>
>   - maybe some comments would be warranted? So maybe a doc comment for
>     the inputs/outputs, and then in the code some comments explaining
>     the steps (e.g. why the first line is handled differently, etc.)
>   - make the function name a bit less generic (although I have a hard
>     time coming up with a good one at the spot, I have to admit)
>   - add one ore two test cases calling the function directly, so that
>     one can see the inputs and expected outputs

yeah i'll see to it that i'll add a patch that adds documentation and
test cases. i'll admit that after going over the related rfcs i was
mostly focused on implementing this right and didn't sufficient context.

going over this again i also noticed a small bug, so i'll send a v2
shortly.

thanks for the review!





^ permalink raw reply	[flat|nested] 4+ messages in thread

* superseded: Re: [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines
  2026-02-19 14:58 [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines Shannon Sterz
  2026-02-25  9:14 ` Lukas Wagner
@ 2026-02-25 11:22 ` Shannon Sterz
  1 sibling, 0 replies; 4+ messages in thread
From: Shannon Sterz @ 2026-02-25 11:22 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

Superseded-by: https://lore.proxmox.com/pbs-devel/20260225112107.99905-1-s.sterz@proxmox.com/T/#u

On Thu Feb 19, 2026 at 3:58 PM CET, Shannon Sterz wrote:
> the http and mime Content-Disposition headers function slightly
> differently. for the http version the following would be valid
> according to rfc 6266 [1]:
>
> Content-Disposition: attachment; filename="file.pdf";
>     filename*=UTF-8''file.pdf
>
> specifying multiple filename attributes like this isn't valid for mime
> messages. according to rfc 2183 [2] the above should be expressed like
> so for mime messages (one variant, multiple are supported):
>
> Content-Disposition: attachment;
>     filename*0*=UTF-8''file.bin
>
> while rfc 2183 [2] would in theory allow for multiple filename
> parameters similar to rfc 6266, in practice MIME::Tools [2] considers
> this as ambigous content [3]. in more recent versions of Proxmox Mail
> Gateway, such messages are rejected [4].
>
> this patch implements proper filename handling and also wraps
> filenames in the content disposition and content type headers
> appropriatelly to a mail being rejected due to having mime headers
> that are too long as described in rfcs 2231 [5] and 2045 [6].
>
> [1]: https://datatracker.ietf.org/doc/html/rfc6266/
> [2]: https://datatracker.ietf.org/doc/html/rfc2183#section-2
> [3]: https://metacpan.org/pod/MIME::Head#ambiguous_content
> [4]:
> https://git.proxmox.com/?p=pmg-api.git;a=commitdiff;h=66f51c62f789b4c20308b7594fbbb357721b0844
> [5]: https://datatracker.ietf.org/doc/html/rfc2231#section-7
> [6]: https://datatracker.ietf.org/doc/html/rfc2045/#section-6.8
>
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>
> tested this with peat on the following muas:
>
> * aerc
> * thunderbird
> * ios mail app
> * canary mail app
> * start mail web client
> * gmail web client
> * outlook web client
>
> noticed this when peat suddenly struggled to send mails. thanks @ Stoiko
> Ivanov for helping with analysing this issue.
>
> Changelog:
> ---------
>
> * wrap lines more correctly ensuring that encoded lines don't exceed 76
>   chars (thanks @ Stoiko Ivanov)
> * refactored the wrapping logic to be more general
>
>  proxmox-sendmail/src/lib.rs | 167 ++++++++++++++++++++++++++++--------
>  1 file changed, 129 insertions(+), 38 deletions(-)
>
> diff --git a/proxmox-sendmail/src/lib.rs b/proxmox-sendmail/src/lib.rs
> index f5b04afd..ec8488a4 100644
> --- a/proxmox-sendmail/src/lib.rs
> +++ b/proxmox-sendmail/src/lib.rs
> @@ -44,23 +44,83 @@ const RFC5987SET: &AsciiSet = &CONTROLS
>  /// base64 encode and hard-wrap the base64 encoded string every 72 characters. this improves
>  /// compatibility.
>  fn encode_base64_formatted<T: AsRef<[u8]>>(raw: T) -> String {
> -    const TEXT_WIDTH: usize = 72;
> -
>      let encoded = proxmox_base64::encode(raw);
> -    let bytes = encoded.as_bytes();
>
> -    let lines = bytes.len().div_ceil(TEXT_WIDTH);
> -    let mut out = Vec::with_capacity(bytes.len() + lines - 1); // account for 1 newline per line
> +    format_text(&encoded, |_| "".into(), "", 0, true)
> +}
>
> -    for (line, chunk) in bytes.chunks(TEXT_WIDTH).enumerate() {
> +fn encode_filename_formatted(filename: &str) -> String {
> +    let encoded = format!("UTF-8''{}", utf8_percent_encode(filename, RFC5987SET));
> +
> +    let format_prefix = |l| {
> +        if let Some(l) = l {
> +            format!("\tfilename*{l}*=")
> +        } else {
> +            "\tfilename*NN*=".into()
> +        }
> +    };
> +
> +    format_text(&encoded, format_prefix, ";", 0, true)
> +}
> +
> +fn format_text<F>(
> +    text: &str,
> +    prefix: F,
> +    suffix: &str,
> +    shorten_first: usize,
> +    skip_last_suffix: bool,
> +) -> String
> +where
> +    F: Fn(Option<usize>) -> String,
> +{
> +    const DEFAULT_TEXT_WIDTH: usize = 72;
> +
> +    let bytes = text.as_bytes();
> +    let suffix = suffix.as_bytes();
> +
> +    let format_text_len = suffix.len() + prefix(None).len();
> +    let lines = (bytes.len() + shorten_first).div_ceil(DEFAULT_TEXT_WIDTH - format_text_len);
> +
> +    // bytes + formatting and "\n" per line - no "\n" in the last line
> +    let mut capacity = bytes.len() + (format_text_len + 1) * lines - 1;
> +
> +    if skip_last_suffix {
> +        capacity -= suffix.len();
> +    }
> +
> +    let mut out = Vec::with_capacity(capacity);
> +    let first_line_length = DEFAULT_TEXT_WIDTH - shorten_first - format_text_len;
> +
> +    let (total_lines, bytes) =
> +        if let Some((first, rest)) = bytes.split_at_checked(first_line_length) {
> +            out.extend_from_slice(prefix(Some(0)).as_bytes());
> +            out.extend_from_slice(first);
> +            out.extend_from_slice(suffix);
> +            if !rest.is_empty() {
> +                out.push(b'\n');
> +            }
> +            (1, rest)
> +        } else {
> +            (0, bytes)
> +        };
> +
> +    for (line, chunk) in bytes
> +        .chunks(DEFAULT_TEXT_WIDTH - format_text_len)
> +        .enumerate()
> +    {
> +        let line = line + total_lines;
> +        out.extend_from_slice(prefix(Some(line)).as_bytes());
>          out.extend_from_slice(chunk);
> +
>          if line + 1 != lines {
> -            // do not end last line with newline to give caller control over doing so.
> +            out.extend_from_slice(suffix);
>              out.push(b'\n');
> +        } else if !skip_last_suffix {
> +            out.extend_from_slice(suffix);
>          }
>      }
>
> -    // SAFETY: base64 encoded, which is 7-bit chars (ASCII) and thus always valid UTF8
> +    // SAFETY: all input slices were valid utf-8 string slices
>      unsafe { String::from_utf8_unchecked(out) }
>  }
>
> @@ -104,24 +164,41 @@ impl Attachment<'_> {
>
>          let mut attachment = String::new();
>
> -        let encoded_filename = if self.filename.is_ascii() {
> -            &self.filename
> -        } else {
> -            &format!("=?utf-8?B?{}?=", proxmox_base64::encode(&self.filename))
> -        };
> -
>          let _ = writeln!(attachment, "\n--{file_boundary}");
> -        let _ = writeln!(
> -            attachment,
> -            "Content-Type: {}; name=\"{encoded_filename}\"",
> -            self.mime,
> -        );
>
> -        // both `filename` and `filename*` are included for additional compatibility
> +        let _ = write!(attachment, "Content-Type: {};\n\tname=\"", self.mime);
> +
> +        if self.filename.is_ascii() {
> +            let _ = write!(attachment, "{}", &self.filename);
> +        } else {
> +            let encoded = proxmox_base64::encode(&self.filename);
> +            let prefix_fn = |line| {
> +                if Some(0) == line {
> +                    "=?utf-8?B?"
> +                } else {
> +                    "\t=?utf-8?B?"
> +                }
> +                .into()
> +            };
> +
> +            // minus one here as the first line isn't indented
> +            let skip_first = "\tname=\"".len() - 1;
> +
> +            let _ = write!(
> +                attachment,
> +                "{}",
> +                format_text(&encoded, prefix_fn, "?=", skip_first, false)
> +            );
> +        }
> +
> +        // should be fine to add one here, the last line should be at most 72 chars. according to
> +        // rfc 2045 encoded output lines should be "no more than 76 characters each"
> +        let _ = writeln!(attachment, "\"");
> +
>          let _ = writeln!(
>              attachment,
> -            "Content-Disposition: attachment; filename=\"{encoded_filename}\";\n\tfilename*=UTF-8''{}",
> -            utf8_percent_encode(&self.filename, RFC5987SET)
> +            "Content-Disposition: attachment;\n{}",
> +            encode_filename_formatted(&self.filename)
>          );
>
>          attachment.push_str("Content-Transfer-Encoding: base64\n\n");
> @@ -809,9 +886,10 @@ Content-Transfer-Encoding: 7bit
>  Lorem Ipsum Dolor Sit
>  Amet
>  ------_=_NextPart_001_1732806251
> -Content-Type: application/octet-stream; name="deadbeef.bin"
> -Content-Disposition: attachment; filename="deadbeef.bin";
> -	filename*=UTF-8''deadbeef.bin
> +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
> @@ -838,7 +916,7 @@ Content-Transfer-Encoding: base64
>          )
>          .with_recipient_and_name("Receiver Name", "receiver@example.com")
>          .with_attachment("deadbeef.bin", "application/octet-stream", &bin)
> -        .with_attachment("🐄💀.bin", "image/bmp", &bin)
> +        .with_attachment("🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀🐄💀.bin", "image/bmp", &bin)
>          .with_html_alt("<html lang=\"de-at\"><head></head><body>\n\t<pre>\n\t\tLorem Ipsum Dolor Sit Amet\n\t</pre>\n</body></html>");
>
>          let body = mail.format_mail(1732806251).expect("could not format mail");
> @@ -879,17 +957,28 @@ Content-Transfer-Encoding: 7bit
>  </body></html>
>  ------_=_NextPart_002_1732806251--
>  ------_=_NextPart_001_1732806251
> -Content-Type: application/octet-stream; name="deadbeef.bin"
> -Content-Disposition: attachment; filename="deadbeef.bin";
> -	filename*=UTF-8''deadbeef.bin
> +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+796tvu8=
>  ------_=_NextPart_001_1732806251
> -Content-Type: image/bmp; name="=?utf-8?B?8J+QhPCfkoAuYmlu?="
> -Content-Disposition: attachment; filename="=?utf-8?B?8J+QhPCfkoAuYmlu?=";
> -	filename*=UTF-8''%F0%9F%90%84%F0%9F%92%80.bin
> +Content-Type: image/bmp;
> +	name="=?utf-8?B?8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE8J+Sg?=
> +	=?utf-8?B?PCfkITwn5KA8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE?=
> +	=?utf-8?B?8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoA?=
> +	=?utf-8?B?uYmlu?="
> +Content-Disposition: attachment;
> +	filename*0*=UTF-8''%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F;
> +	filename*1*=0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%8;
> +	filename*2*=4%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%9;
> +	filename*3*=2%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9;
> +	filename*4*=F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F;
> +	filename*5*=0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%8;
> +	filename*6*=0%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80.bin
>  Content-Transfer-Encoding: base64
>
>  3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7v
> @@ -997,17 +1086,19 @@ PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4KCTxwcmU+CgkJTG9yZW0g
>  SXBzdW0gRMO2bG9yIFNpdCBBbWV0Cgk8L3ByZT4KPC9ib2R5PjwvaHRtbD4=
>  ------_=_NextPart_002_1732806251--
>  ------_=_NextPart_001_1732806251
> -Content-Type: application/octet-stream; name="deadbeef.bin"
> -Content-Disposition: attachment; filename="deadbeef.bin";
> -	filename*=UTF-8''deadbeef.bin
> +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+796tvu8=
>  ------_=_NextPart_001_1732806251
> -Content-Type: image/bmp; name="=?utf-8?B?8J+QhPCfkoAuYmlu?="
> -Content-Disposition: attachment; filename="=?utf-8?B?8J+QhPCfkoAuYmlu?=";
> -	filename*=UTF-8''%F0%9F%90%84%F0%9F%92%80.bin
> +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
> --
> 2.47.3





^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-02-25 11:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-19 14:58 [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines Shannon Sterz
2026-02-25  9:14 ` Lukas Wagner
2026-02-25 10:20   ` Shannon Sterz
2026-02-25 11:22 ` superseded: " Shannon Sterz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal