From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 5B77296730 for ; Wed, 25 Jan 2023 11:05:13 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3F584CFE5 for ; Wed, 25 Jan 2023 11:04:43 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 25 Jan 2023 11:04:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 54C43460E3 for ; Wed, 25 Jan 2023 11:04:42 +0100 (CET) Message-ID: Date: Wed, 25 Jan 2023 11:04:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:110.0) Gecko/20100101 Thunderbird/110.0 Content-Language: de-AT, en-GB To: Stoiko Ivanov Cc: pmg-devel@lists.proxmox.com References: <20230123155521.28307-1-s.ivanov@proxmox.com> <20230123155521.28307-3-s.ivanov@proxmox.com> <20230125104844.304dca6b@rosa.proxmox.com> From: Thomas Lamprecht In-Reply-To: <20230125104844.304dca6b@rosa.proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.528 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.148 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [utils.pm] Subject: Re: [pmg-devel] [PATCH pmg-api 2/2] smtputf8: keep smtputf8 from incoming postfix, detect for local mail X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jan 2023 10:05:13 -0000 Am 25/01/2023 um 10:48 schrieb Stoiko Ivanov: > On Wed, 25 Jan 2023 10:30:09 +0100 > Thomas Lamprecht wrote: >> >>> - PMG::Utils::reinject_mail ($mail, '', [$receiver], undef, $fqdn); >>> + >>> + my $params; >>> + if (PMG::Utils::mail_needs_smtputf8($mail, '', [$receiver])) { >>> + $params->{mail}->{smtputf8} = 1; >>> + } >> >> I'd rather move this into reinject mail instead of copyi-pastaing the same >> code hunk five times around, after all it has all the info required to >> call mail_needs_smtputf8 there. FWICT, its done on all call sites, so you >> wouldn't even require to add an opt-out param. > > The call-sites it's not added are the ones in the rulesystem - > (PMG::RuleDB::Accept/BCC) - where the mail is received from the outside > and where we don't want to autodetect the need, but simply reuse what > postfix sends us. > (maybe I could have written that a bit more explicitly in the > commit-message) > then either: create a wrapper method that adds that param handling and switch the call sites here over to that or move it still into reinject_mail but allow for opt-ing out. I'd favor the first varian, as that then also allows for some clear naming distinction for which flow (direction) which method should be called. >> >>> + PMG::Utils::reinject_mail ($mail, '', [$receiver], undef, $fqdn, $params); >>> } >>> >>> __PACKAGE__->register_method ({ >> >> >>> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm >>> index 9c6f841..1ccd7d2 100644 >>> --- a/src/PMG/Utils.pm >>> +++ b/src/PMG/Utils.pm >>> @@ -232,6 +232,10 @@ sub mail_needs_smtputf8 { >>> } >>> } >>> >>> + if ($entity->head()->as_string() =~ /([^\p{PosixPrint}\n\r\t])/) { >>> + return 1; >>> + } >> >> >> you're reintroducing the hunk you removed in patch 1/2 without really adding any >> explicit reasoning, or is 1/2 just intended as uncontroversial stop gap to apply >> while 2/2 is still being checked more closely, or what's the deal here? > > The idea was to apply 1/2 (as stop-gap measure) quite soon and get it out - > so that most users with disabled smtputf8 and non-ascii characters in > received mail get their systems working again, while 2/2 was something > that might benefit from a more through review. > I'll try to rewrite the commit message to reference 1/2 (or it's commit > hash once applied) explicitly > Please mention that explicitly the next time.