On Thu, May 05, 2022 at 10:25:07PM +0200, Michael Storz wrote:
>
> @Henrik: I was only surprised that you took the asynchronous method with
> rule_pending/rule_ready + check_cleanup instead of the method for tags with
> action_depends_on_tags. This separates the evaluation of the eval functions
> of the two plugins in time, since check_cleanup is called at the very end
> after the evaluation of all priorities. Since _check_fromnamespoof in turn
> defines a set of tags, all functions that depend on these tags are also
> evaluated only at this time and thus all meta rules that depend on them,
> etc.
>
> If, on the other hand, action_depends_on_tags is used, e.g.
>
> sub _check_eval {
> my ($self, $pms, $result) = @_;
>
> if (exists $pms->{fromname_async_queue}) {
> my $rulename = $pms->get_current_eval_rule_name();
> $pms->action_depends_on_tags2('DKIMDOMAIN', sub {
> $self->_check_fromnamespoof($pms);
> return $result->() ? $pms->got_hit($rulename, '', ruletype =>
> 'header') : 0 ;
> });
> return;
> }
>
> $self->_check_fromnamespoof($pms);
> return $result->();
> }
>
> then after the DKIMDOMAIN tag is defined, all eval functions that depend on
> it are called immediately. Thus, all eval functions run at priority level 0
> and all dependent eval and meta rules can run at the same priority level.
> This solution runs on 3.4.6. Therefore I would be interested which reasons
> speak against this solution. Maybe there is something in 4.0 which changes
> the game.
Appreciate the thoughtful post.
I was initially scared of users messing with priorities so that some of the
evals run at completely different times. This can happen for example with
meta rules and $pms->fix_priorities() which can adjust priorities if some of
the FNS rules are depended on. So it's possible that when DKIMDOMAIN is
set, not all eval functions are even registered in async queue etc.
( As a random note, just noticed that $pms->fix_priorities() seems to act
strange too, adjusting different/random rules on each call. Since all
meta rules were changed into dynamic evaluation some time ago, not sure if
fix_priorities() is even needed, atleast for anything else than
shortcircuiting meta rules.. )
I see now that we can keep adding evals into queue as they are called, and
keep the check_cleanup() around to make sure any pending queue is run.
As we all can see, it's very hard to make 100% robust logic when there are
async stuff or complex dependencies between plugin states. Imagine someone
new stumbling into SA and trying to make some plugins, there really isn't
any sane documentation for it. I've been tweaking these for a long time,
but keep getting bitten. Even just noticed that the ASN plugin logic is
broken (for example expecting 'ASN' tag, which might not be never set as
user can change the tag name).
Cheers,
Henrik
