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

Reply via email to