You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by "Kevin A. McGrail" <km...@apache.org> on 2019/11/27 15:18:20 UTC
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
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 <ja...@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
KMcGrail@Apache.org
Member, Apache Software Foundation
Chair Emeritus Apache SpamAssassin Project
https://www.linkedin.com/in/kmcgrail - 703.798.0171
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by "Kevin A. McGrail" <km...@apache.org>.
Yes, but waiting let's me work through testing where I've found quite a
few issues. I was going to roll a release this weekend but SVN was
under DDOS per Infra. They are working on the issue.
On 12/1/2019 3:23 AM, Giovanni Bechis wrote:
> 3.4.3 have many bugs fixed respect to 3.4.2 and waiting won't attract
> new testers
--
Kevin A. McGrail
KMcGrail@Apache.org
Member, Apache Software Foundation
Chair Emeritus Apache SpamAssassin Project
https://www.linkedin.com/in/kmcgrail - 703.798.0171
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Giovanni Bechis <gi...@paclan.it>.
On Sat, Nov 30, 2019 at 11:25:21PM -0500, Kevin A. McGrail wrote:
> The commits around that were renaming the plugin and testing which
> uncovered some issues.
>
> I agree. RTC should be a short lived status.
>
> As for active management and committers, the issue is that I find I like
> versionless releases and I'm happy to run with svn checkouts.
With my pkg maintainer hat on, versionless releases is not practical
because you never know when a software should be condidered as ready
if you are not strictly involved.
> Combine
> that with an inability to get tester feedback and I'm worried that a
> release with tons of issues makes us look bad.
>
3.4.3 have many bugs fixed respect to 3.4.2 and waiting won't attract
new testers
Giovanni
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by "Kevin A. McGrail" <km...@apache.org>.
Is there a need to change trunk to a 4.0 branch if we don't have two
branches?
On 12/1/2019 2:05 AM, Henrik K wrote:
> On Sat, Nov 30, 2019 at 11:25:21PM -0500, Kevin A. McGrail wrote:
>> As for active management and committers, the issue is that I find I like
>> versionless releases and I'm happy to run with svn checkouts.
> I know.. I've only ever run trunk, because it works best. :-D As there's
> only intermittent bursts of commits, it would be rare that someone
> downloaded a broken revision. But we should cut a 4.0 branch soon anyway
> for proper development. And encourage people more to use it.
>
--
Kevin A. McGrail
KMcGrail@Apache.org
Member, Apache Software Foundation
Chair Emeritus Apache SpamAssassin Project
https://www.linkedin.com/in/kmcgrail - 703.798.0171
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
On Sat, Nov 30, 2019 at 11:25:21PM -0500, Kevin A. McGrail wrote:
>
> As for active management and committers, the issue is that I find I like
> versionless releases and I'm happy to run with svn checkouts.
I know.. I've only ever run trunk, because it works best. :-D As there's
only intermittent bursts of commits, it would be rare that someone
downloaded a broken revision. But we should cut a 4.0 branch soon anyway
for proper development. And encourage people more to use it.
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by "Kevin A. McGrail" <km...@apache.org>.
The commits around that were renaming the plugin and testing which
uncovered some issues.
I agree. RTC should be a short lived status.
As for active management and committers, the issue is that I find I like
versionless releases and I'm happy to run with svn checkouts. Combine
that with an inability to get tester feedback and I'm worried that a
release with tons of issues makes us look bad.
Right now, though, I'm struggling with SVN issues from Apache since the
27th causing timeouts and ruleqa issues.
Regards,
KAM
On 11/28/2019 7:20 AM, Henrik K wrote:
> I'm not talking about "your" bugs, I think they were fixed long time ago?
> There's been bunch of commits recently like OLEMacro plgin etc that isn't
> even enabled by default. Yea we are supposed to be RTC but even I don't
> care about it anymore, release was supposed to be done half a year ago or
> something. The project simply doesn't have enough active management and
> committers, so I guess we just go with the flow. :-)
--
Kevin A. McGrail
KMcGrail@Apache.org
Member, Apache Software Foundation
Chair Emeritus Apache SpamAssassin Project
https://www.linkedin.com/in/kmcgrail - 703.798.0171
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
I'm not talking about "your" bugs, I think they were fixed long time ago?
There's been bunch of commits recently like OLEMacro plgin etc that isn't
even enabled by default. Yea we are supposed to be RTC but even I don't
care about it anymore, release was supposed to be done half a year ago or
something. The project simply doesn't have enough active management and
committers, so I guess we just go with the flow. :-)
On Thu, Nov 28, 2019 at 06:43:25AM -0500, Kevin A. McGrail wrote:
> Many distros like to backport so I am not worried about that issue.
>
> I don't like bugs :-). And 3.4.3 needed the RCs to work for my system so it
> made sense to fix the bugs. But it makes no sense for me to spend time
> packaging them for so little feedback.
>
> On Thu, Nov 28, 2019, 06:40 Henrik K <[1...@hege.li> wrote:
>
>
> Bugs like these aren't serious, probably even not worth postponing a
> release, all that matters is that basic functionality works, we should make
> no guarantees for stuff like using strange tags in AskDNS rules. It's not
> end of the world if we have to release 3.4.4 anyway. I wonder if
> distributions even adopt 3.4.3..
>
> On Thu, Nov 28, 2019 at 06:24:55AM -0500, Kevin A. McGrail wrote:
> > The extreme lack of testing has had me worried as I have consistently
> found
> > bugs in the release. I think we failed a bit in the test driven
> development
> > and the ability to release fast and often is hurt by it. I really wanted
> to
> > close out 3.4 and move on to 4.0!
> >
> > With the current rc, I now have all the new functionality on a production
> > system and working but I have a grand total of about 3 emails from ALL
> the RCs
> > from testing. I am very nervous!
> >
> > On Thu, Nov 28, 2019, 06:19 Henrik K <[1...@hege.li> wrote:
> >
> >
> > Sure why not.. should try actually releasing after that, before we
> make a
> > world record in rc-count. :-D
> >
> > On Thu, Nov 28, 2019 at 06:11:51AM -0500, Kevin A. McGrail wrote:
> > > Fair enough. Ok to move forward with another rc do.you think?
> > >
> > > On Thu, Nov 28, 2019, 05:40 Henrik K <[1...@hege.li> wrote:
> > >
> > >
> > > It's not that the issue itself needs a test. AskDNS and/or tag
> > handling
> > > currently have no tests at all. In addition there's some open
> bugs
> > looking
> > > at the ways to use AskDNS and it's shortcomings (empty tags,
> meta
> > tests
> > > breaking on askdns etc). TLDR: It actually requires a lot of
> work,
> > don't
> > > have time or stamina right now.
> > >
> > > On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail
> wrote:
> > > > I never would have found that.á Nice job.
> > > >
> > > > Any chance you can create a regression test for the issue ?
> > > >
> > > > On Thu, Nov 28, 2019, 05:09 Henrik K <[1][2][3][4]
> hege@hege.li> wrote:
> > > >
> > > >
> > > > Fixed:
> > > > [2][3][4][5]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 <[3][4]
> > > [5][6]jahlives@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 <[4][5][6][7]
> jahlives@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
> > > > > > >> KMcGrail@Apache.org
> > > > > > >>
> > > > > > >> Member, Apache Software Foundation
> > > > > > >> Chair Emeritus Apache SpamAssassin Project
> > > > > > >> [5][6][7][8]https://www.linkedin.com/in/kmcgrail -
> > 703.798.0171
> > > > > > >
> > > >
> > > >
> > > > References:
> > > >
> > > > [1] mailto:[7][8][9]hege@hege.li
> > > > [2] [8][9][10]http://svn.apache.org/viewvc?view=revision&
> revision=
> > 1870552
> > > > [3] mailto:[9][10][11]jahlives@gmx.ch
> > > > [4] mailto:[10][11][12]jahlives@gmx.ch
> > > > [5] [11][12][13]https://www.linkedin.com/in/kmcgrail
> > >
> > >
> > > References:
> > >
> > > [1] mailto:[13][14]hege@hege.li
> > > [2] mailto:[14][15]hege@hege.li
> > > [3] [15][16]http://svn.apache.org/viewvc?view=revision&revision=
> 1870552
> > > [4] mailto:[16][17]jahlives@gmx.ch
> > > [5] mailto:[17][18]jahlives@gmx.ch
> > > [6] [18][19]https://www.linkedin.com/in/kmcgrail
> > > [7] mailto:[19][20]hege@hege.li
> > > [8] [20][21]http://svn.apache.org/viewvc?view=revision&revision=
> 1870552
> > > [9] mailto:[21][22]jahlives@gmx.ch
> > > [10] mailto:[22][23]jahlives@gmx.ch
> > > [11] [23][24]https://www.linkedin.com/in/kmcgrail
> >
> >
> > References:
> >
> > [1] mailto:[25]hege@hege.li
> > [2] mailto:[26]hege@hege.li
> > [3] mailto:[27]hege@hege.li
> > [4] [28]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [5] mailto:[29]jahlives@gmx.ch
> > [6] mailto:[30]jahlives@gmx.ch
> > [7] [31]https://www.linkedin.com/in/kmcgrail
> > [8] mailto:[32]hege@hege.li
> > [9] [33]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [10] mailto:[34]jahlives@gmx.ch
> > [11] mailto:[35]jahlives@gmx.ch
> > [12] [36]https://www.linkedin.com/in/kmcgrail
> > [13] mailto:[37]hege@hege.li
> > [14] mailto:[38]hege@hege.li
> > [15] [39]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [16] mailto:[40]jahlives@gmx.ch
> > [17] mailto:[41]jahlives@gmx.ch
> > [18] [42]https://www.linkedin.com/in/kmcgrail
> > [19] mailto:[43]hege@hege.li
> > [20] [44]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [21] mailto:[45]jahlives@gmx.ch
> > [22] mailto:[46]jahlives@gmx.ch
> > [23] [47]https://www.linkedin.com/in/kmcgrail
>
>
> References:
>
> [1] mailto:hege@hege.li
> [2] mailto:hege@hege.li
> [3] mailto:hege@hege.li
> [4] mailto:hege@hege.li
> [5] http://svn.apache.org/viewvc?view=revision&revision=
> [6] mailto:jahlives@gmx.ch
> [7] mailto:jahlives@gmx.ch
> [8] https://www.linkedin.com/in/kmcgrail
> [9] mailto:hege@hege.li
> [10] http://svn.apache.org/viewvc?view=revision&revision=
> [11] mailto:jahlives@gmx.ch
> [12] mailto:jahlives@gmx.ch
> [13] https://www.linkedin.com/in/kmcgrail
> [14] mailto:hege@hege.li
> [15] mailto:hege@hege.li
> [16] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [17] mailto:jahlives@gmx.ch
> [18] mailto:jahlives@gmx.ch
> [19] https://www.linkedin.com/in/kmcgrail
> [20] mailto:hege@hege.li
> [21] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [22] mailto:jahlives@gmx.ch
> [23] mailto:jahlives@gmx.ch
> [24] https://www.linkedin.com/in/kmcgrail
> [25] mailto:hege@hege.li
> [26] mailto:hege@hege.li
> [27] mailto:hege@hege.li
> [28] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [29] mailto:jahlives@gmx.ch
> [30] mailto:jahlives@gmx.ch
> [31] https://www.linkedin.com/in/kmcgrail
> [32] mailto:hege@hege.li
> [33] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [34] mailto:jahlives@gmx.ch
> [35] mailto:jahlives@gmx.ch
> [36] https://www.linkedin.com/in/kmcgrail
> [37] mailto:hege@hege.li
> [38] mailto:hege@hege.li
> [39] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [40] mailto:jahlives@gmx.ch
> [41] mailto:jahlives@gmx.ch
> [42] https://www.linkedin.com/in/kmcgrail
> [43] mailto:hege@hege.li
> [44] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [45] mailto:jahlives@gmx.ch
> [46] mailto:jahlives@gmx.ch
> [47] https://www.linkedin.com/in/kmcgrail
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm ?
Posted by "Kevin A. McGrail" <km...@apache.org>.
Many distros like to backport so I am not worried about that issue.
I don't like bugs :-). And 3.4.3 needed the RCs to work for my system so it
made sense to fix the bugs. But it makes no sense for me to spend time
packaging them for so little feedback.
On Thu, Nov 28, 2019, 06:40 Henrik K <he...@hege.li> wrote:
>
> Bugs like these aren't serious, probably even not worth postponing a
> release, all that matters is that basic functionality works, we should make
> no guarantees for stuff like using strange tags in AskDNS rules. It's not
> end of the world if we have to release 3.4.4 anyway. I wonder if
> distributions even adopt 3.4.3..
>
> On Thu, Nov 28, 2019 at 06:24:55AM -0500, Kevin A. McGrail wrote:
> > The extreme lack of testing has had me worried as I have consistently
> found
> > bugs in the release. I think we failed a bit in the test driven
> development
> > and the ability to release fast and often is hurt by it. I really wanted
> to
> > close out 3.4 and move on to 4.0!
> >
> > With the current rc, I now have all the new functionality on a production
> > system and working but I have a grand total of about 3 emails from ALL
> the RCs
> > from testing. I am very nervous!
> >
> > On Thu, Nov 28, 2019, 06:19 Henrik K <[1...@hege.li> wrote:
> >
> >
> > Sure why not.. should try actually releasing after that, before we
> make a
> > world record in rc-count. :-D
> >
> > On Thu, Nov 28, 2019 at 06:11:51AM -0500, Kevin A. McGrail wrote:
> > > Fair enough. Ok to move forward with another rc do.you think?
> > >
> > > On Thu, Nov 28, 2019, 05:40 Henrik K <[1...@hege.li> wrote:
> > >
> > >
> > > It's not that the issue itself needs a test. AskDNS and/or tag
> > handling
> > > currently have no tests at all. In addition there's some open
> bugs
> > looking
> > > at the ways to use AskDNS and it's shortcomings (empty tags,
> meta
> > tests
> > > breaking on askdns etc). TLDR: It actually requires a lot of
> work,
> > don't
> > > have time or stamina right now.
> > >
> > > On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail
> wrote:
> > > > I never would have found that.á Nice job.
> > > >
> > > > Any chance you can create a regression test for the issue ?
> > > >
> > > > On Thu, Nov 28, 2019, 05:09 Henrik K <[1...@hege.li>
> wrote:
> > > >
> > > >
> > > > Fixed:
> > > > [2][3][4]
> 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 <[3][4]
> > > [5]jahlives@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 <[4][5][6]
> jahlives@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
> > > > > > >> KMcGrail@Apache.org
> > > > > > >>
> > > > > > >> Member, Apache Software Foundation
> > > > > > >> Chair Emeritus Apache SpamAssassin Project
> > > > > > >> [5][6][7]https://www.linkedin.com/in/kmcgrail -
> > 703.798.0171
> > > > > > >
> > > >
> > > >
> > > > References:
> > > >
> > > > [1] mailto:[7][8]hege@hege.li
> > > > [2] [8][9]
> http://svn.apache.org/viewvc?view=revision&revision=
> > 1870552
> > > > [3] mailto:[9][10]jahlives@gmx.ch
> > > > [4] mailto:[10][11]jahlives@gmx.ch
> > > > [5] [11][12]https://www.linkedin.com/in/kmcgrail
> > >
> > >
> > > References:
> > >
> > > [1] mailto:[13]hege@hege.li
> > > [2] mailto:[14]hege@hege.li
> > > [3] [15]
> http://svn.apache.org/viewvc?view=revision&revision=1870552
> > > [4] mailto:[16]jahlives@gmx.ch
> > > [5] mailto:[17]jahlives@gmx.ch
> > > [6] [18]https://www.linkedin.com/in/kmcgrail
> > > [7] mailto:[19]hege@hege.li
> > > [8] [20]
> http://svn.apache.org/viewvc?view=revision&revision=1870552
> > > [9] mailto:[21]jahlives@gmx.ch
> > > [10] mailto:[22]jahlives@gmx.ch
> > > [11] [23]https://www.linkedin.com/in/kmcgrail
> >
> >
> > References:
> >
> > [1] mailto:hege@hege.li
> > [2] mailto:hege@hege.li
> > [3] mailto:hege@hege.li
> > [4] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [5] mailto:jahlives@gmx.ch
> > [6] mailto:jahlives@gmx.ch
> > [7] https://www.linkedin.com/in/kmcgrail
> > [8] mailto:hege@hege.li
> > [9] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [10] mailto:jahlives@gmx.ch
> > [11] mailto:jahlives@gmx.ch
> > [12] https://www.linkedin.com/in/kmcgrail
> > [13] mailto:hege@hege.li
> > [14] mailto:hege@hege.li
> > [15] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [16] mailto:jahlives@gmx.ch
> > [17] mailto:jahlives@gmx.ch
> > [18] https://www.linkedin.com/in/kmcgrail
> > [19] mailto:hege@hege.li
> > [20] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [21] mailto:jahlives@gmx.ch
> > [22] mailto:jahlives@gmx.ch
> > [23] https://www.linkedin.com/in/kmcgrail
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
Bugs like these aren't serious, probably even not worth postponing a
release, all that matters is that basic functionality works, we should make
no guarantees for stuff like using strange tags in AskDNS rules. It's not
end of the world if we have to release 3.4.4 anyway. I wonder if
distributions even adopt 3.4.3..
On Thu, Nov 28, 2019 at 06:24:55AM -0500, Kevin A. McGrail wrote:
> The extreme lack of testing has had me worried as I have consistently found
> bugs in the release. I think we failed a bit in the test driven development
> and the ability to release fast and often is hurt by it. I really wanted to
> close out 3.4 and move on to 4.0!
>
> With the current rc, I now have all the new functionality on a production
> system and working but I have a grand total of about 3 emails from ALL the RCs
> from testing. I am very nervous!
>
> On Thu, Nov 28, 2019, 06:19 Henrik K <[1...@hege.li> wrote:
>
>
> Sure why not.. should try actually releasing after that, before we make a
> world record in rc-count. :-D
>
> On Thu, Nov 28, 2019 at 06:11:51AM -0500, Kevin A. McGrail wrote:
> > Fair enough. Ok to move forward with another rc do.you think?
> >
> > On Thu, Nov 28, 2019, 05:40 Henrik K <[1...@hege.li> wrote:
> >
> >
> > It's not that the issue itself needs a test. AskDNS and/or tag
> handling
> > currently have no tests at all. In addition there's some open bugs
> looking
> > at the ways to use AskDNS and it's shortcomings (empty tags, meta
> tests
> > breaking on askdns etc). TLDR: It actually requires a lot of work,
> don't
> > have time or stamina right now.
> >
> > On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail wrote:
> > > I never would have found that.á Nice job.
> > >
> > > Any chance you can create a regression test for the issue ?
> > >
> > > On Thu, Nov 28, 2019, 05:09 Henrik K <[1...@hege.li> wrote:
> > >
> > >
> > > Fixed:
> > > [2][3][4]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 <[3][4]
> > [5]jahlives@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 <[4...@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
> > > > > >> KMcGrail@Apache.org
> > > > > >>
> > > > > >> Member, Apache Software Foundation
> > > > > >> Chair Emeritus Apache SpamAssassin Project
> > > > > >> [5][6][7]https://www.linkedin.com/in/kmcgrail -
> 703.798.0171
> > > > > >
> > >
> > >
> > > References:
> > >
> > > [1] mailto:[7][8]hege@hege.li
> > > [2] [8][9]http://svn.apache.org/viewvc?view=revision&revision=
> 1870552
> > > [3] mailto:[9][10]jahlives@gmx.ch
> > > [4] mailto:[10][11]jahlives@gmx.ch
> > > [5] [11][12]https://www.linkedin.com/in/kmcgrail
> >
> >
> > References:
> >
> > [1] mailto:[13]hege@hege.li
> > [2] mailto:[14]hege@hege.li
> > [3] [15]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [4] mailto:[16]jahlives@gmx.ch
> > [5] mailto:[17]jahlives@gmx.ch
> > [6] [18]https://www.linkedin.com/in/kmcgrail
> > [7] mailto:[19]hege@hege.li
> > [8] [20]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [9] mailto:[21]jahlives@gmx.ch
> > [10] mailto:[22]jahlives@gmx.ch
> > [11] [23]https://www.linkedin.com/in/kmcgrail
>
>
> References:
>
> [1] mailto:hege@hege.li
> [2] mailto:hege@hege.li
> [3] mailto:hege@hege.li
> [4] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [5] mailto:jahlives@gmx.ch
> [6] mailto:jahlives@gmx.ch
> [7] https://www.linkedin.com/in/kmcgrail
> [8] mailto:hege@hege.li
> [9] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [10] mailto:jahlives@gmx.ch
> [11] mailto:jahlives@gmx.ch
> [12] https://www.linkedin.com/in/kmcgrail
> [13] mailto:hege@hege.li
> [14] mailto:hege@hege.li
> [15] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [16] mailto:jahlives@gmx.ch
> [17] mailto:jahlives@gmx.ch
> [18] https://www.linkedin.com/in/kmcgrail
> [19] mailto:hege@hege.li
> [20] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [21] mailto:jahlives@gmx.ch
> [22] mailto:jahlives@gmx.ch
> [23] https://www.linkedin.com/in/kmcgrail
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm ?
Posted by "Kevin A. McGrail" <km...@apache.org>.
The extreme lack of testing has had me worried as I have consistently found
bugs in the release. I think we failed a bit in the test driven
development and the ability to release fast and often is hurt by it. I
really wanted to close out 3.4 and move on to 4.0!
With the current rc, I now have all the new functionality on a production
system and working but I have a grand total of about 3 emails from ALL the
RCs from testing. I am very nervous!
On Thu, Nov 28, 2019, 06:19 Henrik K <he...@hege.li> wrote:
>
> Sure why not.. should try actually releasing after that, before we make a
> world record in rc-count. :-D
>
> On Thu, Nov 28, 2019 at 06:11:51AM -0500, Kevin A. McGrail wrote:
> > Fair enough. Ok to move forward with another rc do.you think?
> >
> > On Thu, Nov 28, 2019, 05:40 Henrik K <[1...@hege.li> wrote:
> >
> >
> > It's not that the issue itself needs a test. AskDNS and/or tag
> handling
> > currently have no tests at all. In addition there's some open bugs
> looking
> > at the ways to use AskDNS and it's shortcomings (empty tags, meta
> tests
> > breaking on askdns etc). TLDR: It actually requires a lot of work,
> don't
> > have time or stamina right now.
> >
> > On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail wrote:
> > > I never would have found that.á Nice job.
> > >
> > > Any chance you can create a regression test for the issue ?
> > >
> > > On Thu, Nov 28, 2019, 05:09 Henrik K <[1...@hege.li> wrote:
> > >
> > >
> > > Fixed:
> > > [2][3]
> 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 <[3][4]
> > jahlives@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 <[4...@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
> > > > > >> KMcGrail@Apache.org
> > > > > >>
> > > > > >> Member, Apache Software Foundation
> > > > > >> Chair Emeritus Apache SpamAssassin Project
> > > > > >> [5][6]https://www.linkedin.com/in/kmcgrail -
> 703.798.0171
> > > > > >
> > >
> > >
> > > References:
> > >
> > > [1] mailto:[7]hege@hege.li
> > > [2] [8]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > > [3] mailto:[9]jahlives@gmx.ch
> > > [4] mailto:[10]jahlives@gmx.ch
> > > [5] [11]https://www.linkedin.com/in/kmcgrail
> >
> >
> > References:
> >
> > [1] mailto:hege@hege.li
> > [2] mailto:hege@hege.li
> > [3] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [4] mailto:jahlives@gmx.ch
> > [5] mailto:jahlives@gmx.ch
> > [6] https://www.linkedin.com/in/kmcgrail
> > [7] mailto:hege@hege.li
> > [8] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [9] mailto:jahlives@gmx.ch
> > [10] mailto:jahlives@gmx.ch
> > [11] https://www.linkedin.com/in/kmcgrail
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
Sure why not.. should try actually releasing after that, before we make a
world record in rc-count. :-D
On Thu, Nov 28, 2019 at 06:11:51AM -0500, Kevin A. McGrail wrote:
> Fair enough. Ok to move forward with another rc do.you think?
>
> On Thu, Nov 28, 2019, 05:40 Henrik K <[1...@hege.li> wrote:
>
>
> It's not that the issue itself needs a test. AskDNS and/or tag handling
> currently have no tests at all. In addition there's some open bugs looking
> at the ways to use AskDNS and it's shortcomings (empty tags, meta tests
> breaking on askdns etc). TLDR: It actually requires a lot of work, don't
> have time or stamina right now.
>
> On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail wrote:
> > I never would have found that.á Nice job.
> >
> > Any chance you can create a regression test for the issue ?
> >
> > On Thu, Nov 28, 2019, 05:09 Henrik K <[1...@hege.li> wrote:
> >
> >
> > Fixed:
> > [2][3]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 <[3][4]
> jahlives@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 <[4...@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
> > > > >> KMcGrail@Apache.org
> > > > >>
> > > > >> Member, Apache Software Foundation
> > > > >> Chair Emeritus Apache SpamAssassin Project
> > > > >> [5][6]https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > > > >
> >
> >
> > References:
> >
> > [1] mailto:[7]hege@hege.li
> > [2] [8]http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [3] mailto:[9]jahlives@gmx.ch
> > [4] mailto:[10]jahlives@gmx.ch
> > [5] [11]https://www.linkedin.com/in/kmcgrail
>
>
> References:
>
> [1] mailto:hege@hege.li
> [2] mailto:hege@hege.li
> [3] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [4] mailto:jahlives@gmx.ch
> [5] mailto:jahlives@gmx.ch
> [6] https://www.linkedin.com/in/kmcgrail
> [7] mailto:hege@hege.li
> [8] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [9] mailto:jahlives@gmx.ch
> [10] mailto:jahlives@gmx.ch
> [11] https://www.linkedin.com/in/kmcgrail
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm ?
Posted by "Kevin A. McGrail" <km...@apache.org>.
Fair enough. Ok to move forward with another rc do.you think?
On Thu, Nov 28, 2019, 05:40 Henrik K <he...@hege.li> wrote:
>
> It's not that the issue itself needs a test. AskDNS and/or tag handling
> currently have no tests at all. In addition there's some open bugs looking
> at the ways to use AskDNS and it's shortcomings (empty tags, meta tests
> breaking on askdns etc). TLDR: It actually requires a lot of work, don't
> have time or stamina right now.
>
> On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail wrote:
> > I never would have found that.á Nice job.
> >
> > Any chance you can create a regression test for the issue ?
> >
> > On Thu, Nov 28, 2019, 05:09 Henrik K <[1...@hege.li> wrote:
> >
> >
> > Fixed:
> > [2]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 <[3]jahlives@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 <[4...@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
> > > > >> KMcGrail@Apache.org
> > > > >>
> > > > >> Member, Apache Software Foundation
> > > > >> Chair Emeritus Apache SpamAssassin Project
> > > > >> [5]https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > > > >
> >
> >
> > References:
> >
> > [1] mailto:hege@hege.li
> > [2] http://svn.apache.org/viewvc?view=revision&revision=1870552
> > [3] mailto:jahlives@gmx.ch
> > [4] mailto:jahlives@gmx.ch
> > [5] https://www.linkedin.com/in/kmcgrail
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
It's not that the issue itself needs a test. AskDNS and/or tag handling
currently have no tests at all. In addition there's some open bugs looking
at the ways to use AskDNS and it's shortcomings (empty tags, meta tests
breaking on askdns etc). TLDR: It actually requires a lot of work, don't
have time or stamina right now.
On Thu, Nov 28, 2019 at 05:33:03AM -0500, Kevin A. McGrail wrote:
> I never would have found that. Nice job.
>
> Any chance you can create a regression test for the issue ?
>
> On Thu, Nov 28, 2019, 05:09 Henrik K <[1...@hege.li> wrote:
>
>
> Fixed:
> [2]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 <[3...@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 <[4...@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
> > > >> KMcGrail@Apache.org
> > > >>
> > > >> Member, Apache Software Foundation
> > > >> Chair Emeritus Apache SpamAssassin Project
> > > >> [5]https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > > >
>
>
> References:
>
> [1] mailto:hege@hege.li
> [2] http://svn.apache.org/viewvc?view=revision&revision=1870552
> [3] mailto:jahlives@gmx.ch
> [4] mailto:jahlives@gmx.ch
> [5] https://www.linkedin.com/in/kmcgrail
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm ?
Posted by "Kevin A. McGrail" <km...@apache.org>.
I never would have found that. Nice job.
Any chance you can create a regression test for the issue ?
On Thu, Nov 28, 2019, 05:09 Henrik K <he...@hege.li> wrote:
>
> 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 <ja...@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 <ja...@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
> > > >> KMcGrail@Apache.org
> > > >>
> > > >> Member, Apache Software Foundation
> > > >> Chair Emeritus Apache SpamAssassin Project
> > > >> https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > > >
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by "Tobi <jahlives@gmx.ch>" <ja...@gmx.ch>.
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 <ja...@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 <ja...@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
>>>>> KMcGrail@Apache.org
>>>>>
>>>>> Member, Apache Software Foundation
>>>>> Chair Emeritus Apache SpamAssassin Project
>>>>> https://www.linkedin.com/in/kmcgrail - 703.798.0171
>>>>
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm ?
Posted by "Kevin A. McGrail" <km...@apache.org>.
I never would have found that. Nice job.
Any chance you can create a regression test for the issue ?
On Thu, Nov 28, 2019, 05:09 Henrik K <he...@hege.li> wrote:
>
> 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 <ja...@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 <ja...@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
> > > >> KMcGrail@Apache.org
> > > >>
> > > >> Member, Apache Software Foundation
> > > >> Chair Emeritus Apache SpamAssassin Project
> > > >> https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > > >
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
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 <ja...@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 <ja...@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
> > >> KMcGrail@Apache.org
> > >>
> > >> Member, Apache Software Foundation
> > >> Chair Emeritus Apache SpamAssassin Project
> > >> https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > >
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
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 <ja...@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 <ja...@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
> > >> KMcGrail@Apache.org
> > >>
> > >> Member, Apache Software Foundation
> > >> Chair Emeritus Apache SpamAssassin Project
> > >> https://www.linkedin.com/in/kmcgrail - 703.798.0171
> > >
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
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 <ja...@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 <ja...@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
> >> KMcGrail@Apache.org
> >>
> >> Member, Apache Software Foundation
> >> Chair Emeritus Apache SpamAssassin Project
> >> https://www.linkedin.com/in/kmcgrail - 703.798.0171
> >
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by "Tobi <jahlives@gmx.ch>" <ja...@gmx.ch>.
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 <ja...@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
>> KMcGrail@Apache.org
>>
>> Member, Apache Software Foundation
>> Chair Emeritus Apache SpamAssassin Project
>> https://www.linkedin.com/in/kmcgrail - 703.798.0171
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
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 <ja...@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
> KMcGrail@Apache.org
>
> Member, Apache Software Foundation
> Chair Emeritus Apache SpamAssassin Project
> https://www.linkedin.com/in/kmcgrail - 703.798.0171
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in
PerMsgStatus.pm ?
Posted by RW <rw...@googlemail.com>.
On Wed, 27 Nov 2019 17:00:26 +0100
Tobi <ja...@gmx.ch> wrote:
>
> Issue could occur if trusted/internal networks is not set properly and
> the message contains a non-smtp received header with a public IP. Like
> for example could occur if the message is sent via lmtp. Then the
> received header does not contain ptr info therefore _LASTEXTERNALRDNS_
> cannot be set or only set with empty string. That could potentially
> lead to fancy dns queries if using the tag in an askdns rule
AskDNS is supposed to skip empty or undefined strings.
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by "Tobi <jahlives@gmx.ch>" <ja...@gmx.ch>.
Hi Kevin
sorry for sending offlist the first time :-)
> Have you been using that change in production?
Yes I changed it in my prod perl file today. Works so far for the last
couple of hours.
Issue could occur if trusted/internal networks is not set properly and
the message contains a non-smtp received header with a public IP. Like
for example could occur if the message is sent via lmtp. Then the
received header does not contain ptr info therefore _LASTEXTERNALRDNS_
cannot be set or only set with empty string. That could potentially lead
to fancy dns queries if using the tag in an askdns rule
From my point of view it would be very nice to have these two tags set
by default
Cheers
tobi
Am 27.11.19 um 16:18 schrieb Kevin A. McGrail:
> 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 <ja...@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
>
Re: LASTEXTERNALRDNS and LASTEXTERNALHELO not set in PerMsgStatus.pm
?
Posted by Henrik K <he...@hege.li>.
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 <ja...@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
> KMcGrail@Apache.org
>
> Member, Apache Software Foundation
> Chair Emeritus Apache SpamAssassin Project
> https://www.linkedin.com/in/kmcgrail - 703.798.0171