[pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part

[Dominik Csapak]

On 2/21/25 12:23, Stoiko Ivanov wrote:
> On Fri, 21 Feb 2025 11:10:36 +0100
> Dominik Csapak <d.csapak at proxmox.com> wrote:
> 
>> an alternative to this could be that we don't traverse entities if they
>> are a 'message/rfc822' ? that way we could still match fields from
>> individual parts, just not in nested messages.
> 
> Currently don't see an example where any field I'd want to match in
> the first-part headers only (From/To/Subject/....) would occur anywhere in
> a attachment, which is not 'message/rfc822'.
> OTOH if I want to match fields of non-top-MIME-Parts (usually attachments,
> alternatives/.. - Content-Type/Disposition/Encoding) I think I would not
> want to restrict the match to not include attachments of attached e-mails?
> 
> I think we can wait for the use-case for this to come up in the community
> - and then change the checkbox to a combobox (default, top-only,
> ignore-message/rfc822) - What do you think?
> 

yeah i think that makes sense. I believe I was too hung
up on the 'don't match attached mails' part, but as you mention, if you want
to match fields of attachments, you want to catch the whole chain...

> 
>>
>> But I think this patch is still fine since we document the
>> match field for mail headers and those (should) only
>> appear in the top-level part anyway..
>>
>> On 2/20/25 21:13, Stoiko Ivanov wrote:
>>> The current Match Field (header) what-objects always traverse each
>>> mime-part of a mail.
>>> This can be inconvenient, and causes unexpected matches when you
>>> forward a message as attachment(message/rfc822).
>>>
>>> following the patches for adding a Disclaimer on top of a mail from an
>>> implementation point of view, this simply adds an optional
>>> top-level-only attribute for the MatchField object, which is disabled
>>> by default for backwards-compatibility.
>>>
>>> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
>>> ---
>>>    src/PMG/RuleDB/ContentTypeFilter.pm |  2 +-
>>>    src/PMG/RuleDB/MatchField.pm        | 47 +++++++++++++++++++++++++++--
>>>    2 files changed, 45 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
>>> index e44bf3c..6818de1 100644
>>> --- a/src/PMG/RuleDB/ContentTypeFilter.pm
>>> +++ b/src/PMG/RuleDB/ContentTypeFilter.pm
>>> @@ -35,7 +35,7 @@ sub new {
>>>    	$fvalue = $nt;
>>>        }
>>>    
>>> -    my $self = $class->SUPER::new('content-type', $fvalue, $ogroup);
>>> +    my $self = $class->SUPER::new('content-type', $fvalue, $ogroup, undef);
>>>        $self->{only_content} = $only_content;
>>>    
>>>        return $self;
>>> diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
>>> index ee1851a..f6787e8 100644
>>> --- a/src/PMG/RuleDB/MatchField.pm
>>> +++ b/src/PMG/RuleDB/MatchField.pm
>>> @@ -27,7 +27,7 @@ sub otype_text {
>>>    }
>>>    
>>>    sub new {
>>> -    my ($type, $field, $field_value, $ogroup) = @_;
>>> +    my ($type, $field, $field_value, $ogroup, $top_part_only) = @_;
>>>    
>>>        my $class = ref($type) || $type;
>>>    
>>> @@ -35,6 +35,7 @@ sub new {
>>>    
>>>        $self->{field} = $field;
>>>        $self->{field_value} = $field_value;
>>> +    $self->{top_part_only} = $top_part_only;
>>>    
>>>        return $self;
>>>    }
>>> @@ -54,12 +55,28 @@ sub load_attr {
>>>        my $decoded_field_value = PMG::Utils::try_decode_utf8($field_value);
>>>        # use known constructor, bless afterwards (because sub class can have constructor
>>>        # with other parameter signature).
>>> -    my $obj =  PMG::RuleDB::MatchField->new($field, $decoded_field_value, $ogroup);
>>> +    my $obj =  PMG::RuleDB::MatchField->new($field, $decoded_field_value, $ogroup, undef);
>>>        bless $obj, $class;
>>>    
>>> +    my $sth = $ruledb->{dbh}->prepare(
>>> +	"SELECT * FROM Attribut WHERE Object_ID = ?");
>>> +
>>> +    $sth->execute($id);
>>> +
>>> +    $obj->{top_part_only} = 0;
>>> +
>>> +    while (my $ref = $sth->fetchrow_hashref()) {
>>> +	if ($ref->{name} eq 'top_part_only') {
>>> +	    $obj->{top_part_only} = $ref->{value};
>>> +	}
>>> +    }
>>> +
>>> +    $sth->finish();
>>> +
>>>        $obj->{id} = $id;
>>>    
>>> -    $obj->{digest} = Digest::SHA::sha1_hex($id, $field, $field_value, $ogroup);
>>> +    $obj->{digest} = Digest::SHA::sha1_hex(
>>> +	$id, $field, $field_value, $ogroup, $obj->{top_part_only});
>>>    
>>>        return $obj;
>>>    }
>>> @@ -79,6 +96,9 @@ sub save {
>>>    
>>>        if (defined ($self->{id})) {
>>>    	# update
>>> +	$ruledb->{dbh}->do(
>>> +	    "DELETE FROM Attribut WHERE Object_ID = ?",
>>> +	    undef, $self->{id});
>>>    
>>>    	$ruledb->{dbh}->do(
>>>    	    "UPDATE Object SET Value = ? WHERE ID = ?",
>>> @@ -96,6 +116,12 @@ sub save {
>>>    	$self->{id} = PMG::Utils::lastid($ruledb->{dbh}, 'object_id_seq');
>>>        }
>>>    
>>> +    if (defined($self->{top_part_only})) {
>>> +	$ruledb->{dbh}->do(
>>> +	    "INSERT INTO Attribut (Value, Name, Object_ID) VALUES (?, 'top_part_only', ?)",
>>> +	    undef, $self->{top_part_only}, $self->{id});
>>> +    }
>>> +
>>>        return $self->{id};
>>>    }
>>>    
>>> @@ -124,6 +150,8 @@ sub parse_entity {
>>>    	}
>>>        }
>>>    
>>> +    return $res if $self->{top_part_only};
>>> +
>>>        foreach my $part ($entity->parts)  {
>>>    	if (my $match = $self->parse_entity($part)) {
>>>    	    push @$res, @$match;
>>> @@ -160,6 +188,12 @@ sub properties {
>>>    	    type => 'string',
>>>    	    maxLength => 1024,
>>>    	},
>>> +	'top-part-only' => {
>>> +	    description => "only match the headers in the first MIME-Part",
>>> +	    type => 'boolean',
>>> +	    optional => 1,
>>> +	    default => 0,
>>> +	},
>>>        };
>>>    }
>>>    
>>> @@ -169,6 +203,7 @@ sub get {
>>>        return {
>>>    	field => $self->{field},
>>>    	value => $self->{field_value},
>>> +	'top-part-only' => $self->{top_part_only},
>>>        };
>>>    }
>>>    
>>> @@ -177,6 +212,12 @@ sub update {
>>>    
>>>        $self->{field_value} = $param->{value};
>>>        $self->{field} = $param->{field};
>>> +
>>> +    if (defined($param->{'top-part-only'}) && $param->{'top-part-only'} == 1) {
>>> +	$self->{top_part_only} = 1;
>>> +    } else {
>>> +	delete $self->{top_part_only};
>>> +    }
>>>    }
>>>    
>>>    1;
>>
>>
>