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

["Shannon Sterz" <s.sterz@proxmox.com>]

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!