public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Shannon Sterz <s.sterz@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox] sendmail: conform more to mime Content-Disposition header and wrap lines
Date: Wed, 18 Feb 2026 16:26:38 +0100	[thread overview]
Message-ID: <20260218162638.5ac3e38b@rosa.proxmox.com> (raw)
In-Reply-To: <20260218142047.263732-1-s.sterz@proxmox.com>

Thanks for addressing this so quickly and the good analysis!

On Wed, 18 Feb 2026 14:21:19 +0000
Shannon Sterz <s.sterz@proxmox.com> wrote:

> the http and mime Content-Disposition headers function slightly
> differently. for the http the following would be valid according to
> rfc 6266:
> 
> 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 the above should be expressed like so
> for mime messages (one variant, multiple are supported):
to be (a bit) more verbose/pedantic - rfc 2183 would allow for multiple
filename parameters:
https://datatracker.ietf.org/doc/html/rfc2183#section-2
However MIME::Tools considers this (arguably so) as ambiguous content:
https://metacpan.org/pod/MIME::Head#ambiguous_content

Which in turn is something PMG rejects while parsing as of a recent change:
https://git.proxmox.com/?p=pmg-api.git;a=commitdiff;h=66f51c62f789b4c20308b7594fbbb357721b0844

> 
> Content-Disposition: attachment;
>     filename*0*=UTF-8''file.bin
as for the splitting into multiple parts (and providing of encoding and
language) - that is described in rfc2231 - and we checked that the
filename parameter and values fit with:
https://datatracker.ietf.org/doc/html/rfc2231#section-7

> 
> 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.
> 
> 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
I checked:
* claws-mail
* mutt
(and the spamanalysis by rspamd of a generated mail)

as far as receiving mails generated with this patch is concerned:
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>

1 nit and one comment inline:
> 
> noticed this when peat suddenly struggled to send mails. thanks @ Stoiko
> Ivanov for helping with analysing this issue.
> 
>  proxmox-sendmail/src/lib.rs | 131 +++++++++++++++++++++++++-----------
>  1 file changed, 93 insertions(+), 38 deletions(-)
> 
> diff --git a/proxmox-sendmail/src/lib.rs b/proxmox-sendmail/src/lib.rs
> index f5b04afd..4b686bd9 100644
> --- a/proxmox-sendmail/src/lib.rs
> +++ b/proxmox-sendmail/src/lib.rs
> @@ -41,18 +41,19 @@ const RFC5987SET: &AsciiSet = &CONTROLS
>      .add(b'{')
>      .add(b'}');
> 
> +const DEFAULT_TEXT_WIDTH: usize = 72;
> +
>  /// 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;
> -
> +fn encode_base64_formatted<T: AsRef<[u8]>>(raw: T, text_width: Option<usize>) -> String {
>      let encoded = proxmox_base64::encode(raw);
>      let bytes = encoded.as_bytes();
> +    let text_width = text_width.unwrap_or(DEFAULT_TEXT_WIDTH);
> 
> -    let lines = bytes.len().div_ceil(TEXT_WIDTH);
> +    let lines = bytes.len().div_ceil(text_width);
>      let mut out = Vec::with_capacity(bytes.len() + lines - 1); // account for 1 newline per line
> 
> -    for (line, chunk) in bytes.chunks(TEXT_WIDTH).enumerate() {
> +    for (line, chunk) in bytes.chunks(text_width).enumerate() {
>          out.extend_from_slice(chunk);
>          if line + 1 != lines {
>              // do not end last line with newline to give caller control over doing so.
> @@ -64,6 +65,38 @@ fn encode_base64_formatted<T: AsRef<[u8]>>(raw: T) -> String {
>      unsafe { String::from_utf8_unchecked(out) }
>  }
> 
> +fn encode_filename_formatted(filename: &str) -> String {
> +    let encoded = format!("{}", utf8_percent_encode(filename, RFC5987SET));
> +    let bytes = encoded.as_bytes();
> +
> +    let prefix_len = "\tfilename*00*=".len();
> +    // + 1 here to include a ";" per line
> +    let lines = bytes.len().div_ceil(DEFAULT_TEXT_WIDTH - (prefix_len + 1));
> +
> +    // bytes + prefixes + plus ";\n" - 2 (the last line does not end in ";n")
> +    let mut out = Vec::with_capacity(bytes.len() + prefix_len * lines + lines * 2 - 2);
nit: for me bytes.len()+ (prefix_len + 2) *lines -2 would read more easily
but with the comments it's straight-forward to follow

> +
> +    for (line, chunk) in bytes
> +        .chunks(DEFAULT_TEXT_WIDTH - (prefix_len + 1))
> +        .enumerate()
> +    {
> +        out.extend_from_slice(format!("\tfilename*{line}*=").as_bytes());
> +
> +        if line == 0 {
> +            out.extend_from_slice("UTF-8''".as_bytes());
> +        }
the UTF-8'' is not considered above while chunking - thus the first line
of the filename parameter ends at 78 characters - vs. the 72 defined above.
(which is still in line with:
https://datatracker.ietf.org/doc/html/rfc5322#section-2.1.1)
not sure if this was intended - thus the comment.

else this LGTM.


> +
> +        out.extend_from_slice(chunk);
> +        if line + 1 != lines {
> +            out.push(b';');
> +            out.push(b'\n')
> +        }
> +    }
> +
> +    // SAFETY: bytes are utf-8 since they are percent encoded.
> +    unsafe { String::from_utf8_unchecked(out) }
> +}
> +
>  struct Recipient {
>      name: Option<String>,
>      email: String,
> @@ -104,28 +137,36 @@ 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 mut first = true;
> +            for line in
> +                encode_base64_formatted(&self.filename, Some(DEFAULT_TEXT_WIDTH - 14)).split('\n')
> +            {
> +                if first {
> +                    let _ = write!(attachment, "=?utf-8?B?{line}?=");
> +                    first = false;
> +                } else {
> +                    let _ = write!(attachment, "\n\t=?utf-8?B?{line}?=");
> +                }
> +            }
> +        }
> +
> +        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");
> -        attachment.push_str(&encode_base64_formatted(self.content));
> +        attachment.push_str(&encode_base64_formatted(self.content, None));
> 
>          attachment
>      }
> @@ -514,7 +555,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, None));
>          }
> 
>          if let Some(html) = &self.body_html {
> @@ -527,7 +568,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, None));
>              }
> 
>              write!(body, "\n--{html_boundary}--")?;
> @@ -809,9 +850,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 +880,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 +921,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+SgPCfkI?=
> +	=?utf-8?B?Twn5KA8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoDwn5CE8J+S?=
> +	=?utf-8?B?gPCfkITwn5KA8J+QhPCfkoDwn5CE8J+SgPCfkITwn5KA8J+QhPCfkoAuYm?=
> +	=?utf-8?B?lu?="
> +Content-Disposition: attachment;
> +	filename*0*=UTF-8''%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90;
> +	filename*1*=%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F;
> +	filename*2*=%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0;
> +	filename*3*=%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84;
> +	filename*4*=%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92;
> +	filename*5*=%80%F0%9F%90%84%F0%9F%92%80%F0%9F%90%84%F0%9F%92%80%F0%9F;
> +	filename*6*=%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 +1050,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
> 
> 
> 
> 
> 





  reply	other threads:[~2026-02-18 15:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-18 14:20 Shannon Sterz
2026-02-18 15:26 ` Stoiko Ivanov [this message]
2026-02-19 15:00 ` Shannon Sterz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260218162638.5ac3e38b@rosa.proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=s.sterz@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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