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
>
