Re: [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic

[Leo Nunner <l.nunner@proxmox.com>]

Just noticed something: the matching logic here won't work for most
objects as intended, since, if the rule is negated, it will already
return 'true' if the first object in the list (e.g., the first email
address) doesn't match… I think it should rather be if the match isn't
present in the *whole* list, instead of "there are objects that don't
match in this list". I'll fix this in a v2, but I'll wait for further
feedback before I send it.

On 2023-04-07 15:42, Leo Nunner wrote:
> Straightforward for most objects, where the matching result gets XOR'd
> with the specific negation setting. For 'what' objects, more changes
> were necessary: the different types of what matches all needed to be
> expanded inside their respective parse_entity functions. For some of
> these, the result might be _strange_ (which is more due to the concept
> of inverting what objects itself), but seems to work as intended.
>
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
>  src/PMG/RuleCache.pm                   | 8 ++++----
>  src/PMG/RuleDB/ArchiveFilter.pm        | 6 ++++--
>  src/PMG/RuleDB/ContentTypeFilter.pm    | 6 ++++--
>  src/PMG/RuleDB/MatchArchiveFilename.pm | 4 ++--
>  src/PMG/RuleDB/MatchField.pm           | 2 +-
>  src/PMG/RuleDB/MatchFilename.pm        | 2 +-
>  6 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
> index c5a57f6..04e35da 100644
> --- a/src/PMG/RuleCache.pm
> +++ b/src/PMG/RuleCache.pm
> @@ -252,7 +252,7 @@ sub from_match {
>      }
>  
>      foreach my $obj (@$from) {
> -	return 1 if $obj->who_match($addr, $ip, $ldap);
> +	return 1 if ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
>      }
>  
>      return 0;
> @@ -266,7 +266,7 @@ sub to_match {
>      return 1 if !defined ($to);
>  
>      foreach my $obj (@$to) {
> -	return 1 if $obj->who_match($addr, undef, $ldap);
> +	return 1 if ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
>      }
>  
>      return 0;
> @@ -280,7 +280,7 @@ sub when_match {
>      return 1 if !defined ($when);
>  
>      foreach my $obj (@$when) {
> -	return 1 if $obj->when_match($time);
> +	return 1 if ($obj->when_match($time) xor $obj->{negate});
>      }
>  
>      return 0;
> @@ -325,7 +325,7 @@ sub what_match {
>      foreach my $obj (@$what) {
>  	if ($obj->can ("what_match_targets")) {
>  	    my $target_info;
> -	    if ($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
> +	    if (($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) xor $obj->{negate}) {
>  		foreach my $k (keys %$target_info) {
>  		    my $cmarks = $target_info->{$k}->{marks}; # make a copy
>  		    $res->{$k} = $target_info->{$k};
> diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
> index 6d91556..61f6c50 100755
> --- a/src/PMG/RuleDB/ArchiveFilter.pm
> +++ b/src/PMG/RuleDB/ArchiveFilter.pm
> @@ -60,10 +60,12 @@ sub parse_entity {
>  	my $glob_ct = $entity->{PMX_glob_ct};
>  
>  	if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
> -	    push @$res, $id;
> +	    push @$res, $id if !$self->{negate};
>  	} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
> -	    push @$res, $id;
> +	    push @$res, $id if !$self->{negate};
>  	} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
> +	    push @$res, $id if !$self->{negate};
> +	} elsif ($self->{negate}) {
>  	    push @$res, $id;
>  	} else {
>  	    # match inside archives 
> diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
> index 76fc1ce..eb35292 100755
> --- a/src/PMG/RuleDB/ContentTypeFilter.pm
> +++ b/src/PMG/RuleDB/ContentTypeFilter.pm
> @@ -72,10 +72,12 @@ sub parse_entity {
>  	my $glob_ct = $entity->{PMX_glob_ct};
>  
>  	if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
> -	    push @$res, $id;
> +	    push @$res, $id if !$self->{negate};
>  	} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
> -	    push @$res, $id;
> +	    push @$res, $id if !$self->{negate};
>  	} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
> +	    push @$res, $id if !$self->{negate};
> +	} elsif ($self->{negate}) {
>  	    push @$res, $id;
>  	}
>      }
> diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
> index 2ef3543..8abd592 100644
> --- a/src/PMG/RuleDB/MatchArchiveFilename.pm
> +++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
> @@ -29,12 +29,12 @@ sub parse_entity {
>  	chomp $id;
>  
>  	my $fn = PMG::Utils::extract_filename($entity->head);
> -	if (defined($fn) && $fn =~ m|^$self->{fname}$|i) {
> +	if (defined($fn) && ($fn =~ m|^$self->{fname}$|i xor $self->{negate})) {
>  	    push @$res, $id;
>  	} elsif (my $filenames = $entity->{PMX_filenames}) {
>  	    # Match inside archives
>  	    for my $fn (keys %$filenames) {
> -		if ($fn =~ m|^$self->{fname}$|i) {
> +		if ($fn =~ m|^$self->{fname}$|i xor $self->{negate}) {
>  		    push @$res, $id;
>  		    last;
>  		}
> diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
> index 2b56058..8f89297 100644
> --- a/src/PMG/RuleDB/MatchField.pm
> +++ b/src/PMG/RuleDB/MatchField.pm
> @@ -111,7 +111,7 @@ sub parse_entity {
>  	    my $decvalue = PMG::Utils::decode_rfc1522($value);
>  	    $decvalue = PMG::Utils::try_decode_utf8($decvalue);
>  
> -	    if ($decvalue =~ m|$self->{field_value}|i) {
> +	    if (($decvalue =~ m|$self->{field_value}|i) xor $self->{negate}) {
>  		push @$res, $id;
>  	    }
>  	}
> diff --git a/src/PMG/RuleDB/MatchFilename.pm b/src/PMG/RuleDB/MatchFilename.pm
> index c9cdbe0..0665efc 100644
> --- a/src/PMG/RuleDB/MatchFilename.pm
> +++ b/src/PMG/RuleDB/MatchFilename.pm
> @@ -91,7 +91,7 @@ sub parse_entity {
>  	chomp $id;
>  
>  	if (my $value = PMG::Utils::extract_filename($entity->head)) {
> -	    if ($value =~ m|^$self->{fname}$|i) {
> +	    if (($value =~ m|^$self->{fname}$|i) xor $self->{negate}) {
>  		push @$res, $id;
>  	    }
>  	}