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 91B8D1FF396 for ; Wed, 22 May 2024 14:05:48 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 069CFB6DB; Wed, 22 May 2024 14:06:07 +0200 (CEST) Mime-Version: 1.0 Date: Wed, 22 May 2024 14:05:32 +0200 Message-Id: From: "Shannon Sterz" To: "Nigel van Keulen" , X-Mailer: aerc 0.17.0-69-g65571b67d7d3-dirty References: <20240518145855.670-1-nigel2392@gmail.com> <20240518145855.670-2-nigel2392@gmail.com> In-Reply-To: <20240518145855.670-2-nigel2392@gmail.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.066 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 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. [accept.pm, proxmox.com] Subject: Re: [pmg-devel] SPAM: [PATCH pmg-api 1/1] fix: 2971, to DKIM signing for OOO messages by using postmaster@ as a sender if it is not present. 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pmg-devel-bounces@lists.proxmox.com Sender: "pmg-devel" hey thanks for contributing to PMG! i just wanted to provide some quick feedback on the code you sent in, but please make sure to follow our development guidelines and also note that if you haven't already, you need to sign a harmony CLA [1]. some notes on the commit message: a) it is to long, we generally enforce a limit of 70 characters per line. please provide a short summary in the first line and if needed more context in a second paragraph. b) i am assumig OOO stands for "out of office", however your patch applies to all mails as long as they don't have a sender. so this commit message is somewhat confusing. On Sat May 18, 2024 at 4:58 PM CEST, Nigel van Keulen wrote: > --- > src/PMG/RuleDB/Accept.pm | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/PMG/RuleDB/Accept.pm b/src/PMG/RuleDB/Accept.pm > index e3e39a7..8a6261f 100644 > --- a/src/PMG/RuleDB/Accept.pm > +++ b/src/PMG/RuleDB/Accept.pm > @@ -101,8 +101,16 @@ sub execute { > PMG::Utils::remove_marks($entity); > > if ($dkim->{sign}) { > + > + my $mailSender = $msginfo->{sender}; > + if ($mailSender eq '') { > + my $mailDomain = $msginfo->{domain}; > + $mailSender = "postmaster\@$mailDomain"; i wonder if more or less hardcoding a sender for mails here makes sense. it might be neater to make this configurable somehow. i also wonder if this adds much functionality in reality. most users may be able to just use the `dkim-use-domain` to use the sender from the envelop instead. > + syslog('info', "%s: No sender found, using default sender: %s", $queue->{logid}, $mailSender); note that this line is too long. lines should not be longer than 100 characters. you can wrap this line like so to fit that limit ```pl syslog( 'info', "%s: No sender found, using default sender: %s", $queue->{logid}, $mailSender ); ``` this would be in accordance with our style guide [2]. also note that you did not correctly indent your code. please follow the indentation guidelines in the style guide as well. > + } > + > eval { > - $entity = PMG::DKIMSign::sign_entity($entity, $dkim, $msginfo->{sender}); > + $entity = PMG::DKIMSign::sign_entity($entity, $dkim, $mailSender); > }; > if ($@) { > syslog('warning', [1]: https://pmg.proxmox.com/wiki/index.php/Developer_Documentation#Software_License_and_Copyright [2]: https://pve.proxmox.com/wiki/Perl_Style_Guide#Breaking_long_lines_and_strings _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel