Re: [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines

["Lukas Wagner" <l.wagner@proxmox.com>]

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