all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal