Henrik, thanks a lot, can confirm your fix works in my tests :-)
Cheers tobi Am 28.11.19 um 11:09 schrieb Henrik K: > > Fixed: > http://svn.apache.org/viewvc?view=revision&revision=1870552 > > On Thu, Nov 28, 2019 at 11:29:19AM +0200, Henrik K wrote: >> >> Trunk has already many revamps and fixes for these codes, works there. It >> seems 3.4 askdns has trouble using those, investigating minimal fix.. >> >> >> On Thu, Nov 28, 2019 at 10:20:39AM +0100, Tobi <jahli...@gmx.ch> wrote: >>> Henrik >>> >>> But my current testing clearly shows that without the explicit set_tag >>> _LASTEXTERNALRDNS_ and _LASTEXTERNALHELO_ won't be set. >>> I'm testing this using spamassassin in debug mode to scan a testmessage. >>> As soon as I re-add the set_tag to PerMsgStatus.pm the tags are set and >>> tests based on that tags are run. Removing the lines from pm and debug >>> shows that the tests are **not** run anymore. >>> >>> So I somewhat doubt that the set_tag "code is redundant and should be >>> removed" :-) >>> >>> Using: SA 3.4.2 on centos7 / perl 5.16.3 >>> >>> >>> Cheers >>> >>> tobi >>> >>> Am 28.11.19 um 08:36 schrieb Henrik K: >>>> >>>> There is nothing missing. All the LASTEXT* tags are already dynamically >>>> returning functions. If anything, that if ($lasthop) set_tag code is >>>> completely redundant and should be removed. >>>> >>>> >>>> BEGIN { >>>> LASTEXTERNALIP => sub { >>>> my $pms = shift; >>>> my $lasthop = $pms->{relays_external}->[0]; >>>> $lasthop ? $lasthop->{ip} : ''; >>>> }, >>>> >>>> LASTEXTERNALRDNS => sub { >>>> my $pms = shift; >>>> my $lasthop = $pms->{relays_external}->[0]; >>>> $lasthop ? $lasthop->{rdns} : ''; >>>> }, >>>> >>>> LASTEXTERNALHELO => sub { >>>> my $pms = shift; >>>> my $lasthop = $pms->{relays_external}->[0]; >>>> $lasthop ? $lasthop->{helo} : ''; >>>> }, >>>> >>>> >>>> On Wed, Nov 27, 2019 at 10:18:20AM -0500, Kevin A. McGrail wrote: >>>>> After a 10 minute or so study of the issue and comparing 3.4 and trunk, >>>>> it definitely looks like the code is missing. I am not 100% sure your >>>>> fix works but it's better than it currently exists :-) >>>>> >>>>> Have you been using that change in production? >>>>> >>>>> Regards, >>>>> >>>>> KAM >>>>> >>>>> On 11/27/2019 6:59 AM, Tobi <jahli...@gmx.ch> wrote: >>>>>> Hi, >>>>>> >>>>>> is there any specific reason why the two tags mentioned in subject are >>>>>> not set in SA? It took me a while to find out why an askdns test was not >>>>>> running. The test relies on _LASTEXTERNALRDNS_ but after running with >>>>>> --debug I found that those tags are not set by SA. Although >>>>>> _LASTEXTERNALIP_ is set. Also the man page states that the two tags >>>>>> should exist. >>>>>> >>>>>> In PerMsgStatus.pm I saw that everything is prepared for the two tags >>>>>> but they finally not set (around line 1721). So I changed to >>>>>> >>>>>>> if ($lasthop) { >>>>>>> $self->set_tag('LASTEXTERNALIP', $lasthop->{ip}); >>>>>>> $self->set_tag('LASTEXTERNALRDNS', $lasthop->{rdns}); >>>>>>> $self->set_tag('LASTEXTERNALHELO', $lasthop->{helo}); >>>>>>> } >>>>>> >>>>>> and --debug shows that the tags are now set and the askdns test is run. >>>>>> >>>>>> So I wonder if the code has just been forgotten or if there are deeper >>>>>> reasons to not set the tags? >>>>>> If no deeper reasons exist it would be nice to have those two tags set >>>>>> as default in PerMsgStatus.pm >>>>>> >>>>>> >>>>>> Cheers >>>>>> >>>>>> -- >>>>>> >>>>>> tobi >>>>> >>>>> -- >>>>> Kevin A. McGrail >>>>> kmcgr...@apache.org >>>>> >>>>> Member, Apache Software Foundation >>>>> Chair Emeritus Apache SpamAssassin Project >>>>> https://www.linkedin.com/in/kmcgrail - 703.798.0171 >>>> >
