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
> > >
