From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 31F361FF13B for ; Wed, 25 Feb 2026 10:13:42 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1D1293113C; Wed, 25 Feb 2026 10:14:39 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 25 Feb 2026 10:14:03 +0100 Message-Id: To: "Shannon Sterz" , Subject: Re: [PATCH proxmox v2] sendmail: conform more to mime Content-Disposition header and wrap lines From: "Lukas Wagner" X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260219145805.1114672-1-s.sterz@proxmox.com> In-Reply-To: <20260219145805.1114672-1-s.sterz@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1772010827489 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.024 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1.113 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.358 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.659 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: IFYRXZNDTQNYR6WRNRXT5GSM7Q3LTTRF X-Message-ID-Hash: IFYRXZNDTQNYR6WRNRXT5GSM7Q3LTTRF X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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=3D"file.pdf"; > filename*=3DUTF-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*=3DUTF-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=3Dpmg-api.git;a=3Dcommitdiff;h=3D66f51c62f789b= 4c20308b7594fbbb357721b0844 > [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 That being said, I relied on your descriptions in the commit messages and did not read through the RFCs myself. [...] > + > +fn format_text( > + text: &str, > + prefix: F, > + suffix: &str, > + shorten_first: usize, > + skip_last_suffix: bool, > +) -> String > +where > + F: Fn(Option) -> String, > +{ > + const DEFAULT_TEXT_WIDTH: usize =3D 72; > + > + let bytes =3D text.as_bytes(); > + let suffix =3D suffix.as_bytes(); > + > + let format_text_len =3D suffix.len() + prefix(None).len(); > + let lines =3D (bytes.len() + shorten_first).div_ceil(DEFAULT_TEXT_WI= DTH - format_text_len); > + > + // bytes + formatting and "\n" per line - no "\n" in the last line > + let mut capacity =3D bytes.len() + (format_text_len + 1) * lines - 1= ; > + > + if skip_last_suffix { > + capacity -=3D suffix.len(); > + } > + > + let mut out =3D Vec::with_capacity(capacity); > + let first_line_length =3D DEFAULT_TEXT_WIDTH - shorten_first - forma= t_text_len; > + > + let (total_lines, bytes) =3D > + if let Some((first, rest)) =3D 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 =3D line + total_lines; > + out.extend_from_slice(prefix(Some(line)).as_bytes()); > out.extend_from_slice(chunk); > + > if line + 1 !=3D 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 alw= ays 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