public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField
@ 2025-02-20 20:13 Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 1/5] ruledb: disclaimer: simplify update-case Stoiko Ivanov
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

v3->v4:
* rework patch 3/5 for pmg-api according to feedback from Dominik and Friedrich
  after understanding that file-paths (and in result content-types are not
  set for mail-parts that are multipart (mime-entities either have parts
  or a body in general)

cover-letter for v3:

This series replaces two previous patch-series.

v2->v3:
for the content-type matching based on magic infromation only two patches
were added that remove the fallback on the content-type header in case no
filetype could be found through magic.
The one place were I decided to fall back on header information was when
determining if a mime-part is an archive.

places where the content-type information was used are based on grepping
for PMX_ in the pmg-api sources.

Tested with Friedrich's swaks reproducer - the random blob was not
detected as pdf anymore.

no changes to the series for matching headers only on the top-part
(resending so that the comment that it's based on v2 of the content-type
series does not cause confusion)

original cover-letters:

for https://lore.proxmox.com/pmg-devel/20250218135416.54504-1-s.ivanov@proxmox.com/T/#t

v1->v2:
Based on Domink's feedback reworked the series - it now offers only one
optional new attribute for the content-type filter ('only-content') to
indicate that it should only compare the content-type obtained via
magic/file-conents. The other 2 match options can already be expressed
with the Filenamefilter ('application/pdf' -> '.*\.pdf') and the Match
Field filter (to match the 'Content-type' header)

original cover-letter for v1[0]:
The following patch series was started shortly after the release of PMG 8.1,
but I did not find the time to get it in shape for sending.
They follow the patches for adding 'top' and 'add_separator' to the
Disclaimer action.

The current content-type filter can sometimes surprise users (e.g.
https://bugzilla.proxmox.com/show_bug.cgi?id=5618#c2 and
https://bugzilla.proxmox.com/show_bug.cgi?id=2691#c0 ,but also a few
cases in our technical support-channels come up here and there):

It matches if any of:
* content-type header
* file-magic
* filename (extension)
match the content type, the what-object matches.
by adding an attribute for each of the sources users can restrict which
of the sources should be taken into consideration

the first patches for both repositories are independent (I just ran into
them while looking into this).

minimally tested locally, by sending a plain-text file called
testtext.pdf, and a pdf-file renamed to have a `.docx` suffix.

[0]
https://lore.proxmox.com/pmg-devel/20250212151241.91077-1-s.ivanov@proxmox.com/ 


for https://lore.proxmox.com/pmg-devel/20250218194829.80095-1-s.ivanov@proxmox.com/T/#t
based on top of:
https://lore.proxmox.com/pmg-devel/20250218135416.54504-1-s.ivanov@proxmox.com/T/#t

as I think testing both should go well together, and to avoid a trivial
merge-conflict (can of course resend on top of current master) in the
pmg-api patch

Tested minimally in my setup - more testing  would be appreciated


pmg-api:
Stoiko Ivanov (5):
  ruledb: disclaimer: simplify update-case
  utils: content-type: don't fallback to header information for magic
  pmg-smtp-filter: archive-detection: use header information as well
  ruledb: content-type: add flag for matching only based on
    magic/content
  fix #2709: ruledb: match-field: optionally restrict to top mime-part

 src/PMG/RuleDB/ArchiveFilter.pm     |  2 +-
 src/PMG/RuleDB/ContentTypeFilter.pm | 79 +++++++++++++++++++++++++----
 src/PMG/RuleDB/Disclaimer.pm        |  8 ++-
 src/PMG/RuleDB/MatchField.pm        | 47 +++++++++++++++--
 src/PMG/Utils.pm                    | 12 ++---
 src/bin/pmg-smtp-filter             | 10 +++-
 6 files changed, 130 insertions(+), 28 deletions(-)

pmg-gui:
Stoiko Ivanov (3):
  rules/object: remove icon from remove button
  rules/content-typefilter: add checkbox for file content only matching
  fix #2709: rules: match-field: add top-level-only checkbox

 js/ObjectGroup.js |  3 +--
 js/Utils.js       | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-api v4 1/5] ruledb: disclaimer: simplify update-case
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 2/5] utils: content-type: don't fallback to header information for magic Stoiko Ivanov
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

as one object(_id) belongs is of one object type, we can safely remove
all associated attribut entries from the database at once instead of
running 2 separate statements to delete each individually

noticed this while adding new attribut entries to the content-type
check.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/RuleDB/Disclaimer.pm | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/PMG/RuleDB/Disclaimer.pm b/src/PMG/RuleDB/Disclaimer.pm
index 574f62e..cb9b5fb 100644
--- a/src/PMG/RuleDB/Disclaimer.pm
+++ b/src/PMG/RuleDB/Disclaimer.pm
@@ -113,11 +113,9 @@ sub save {
 	    "UPDATE Object SET Value = ? WHERE ID = ?",
 	    undef, $value, $self->{id});
 
-	for my $prop (qw(top separator)) {
-	    $ruledb->{dbh}->do(
-		"DELETE FROM Attribut WHERE Name = ? and Object_ID = ?",
-		undef, $prop,  $self->{id});
-	}
+	$ruledb->{dbh}->do(
+	    "DELETE FROM Attribut WHERE Object_ID = ?",
+		undef, $self->{id});
     } else {
 	# insert
 
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-api v4 2/5] utils: content-type: don't fallback to header information for magic
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 1/5] ruledb: disclaimer: simplify update-case Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well Stoiko Ivanov
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

file-type detection based on content/magic is the single piece of
information not determined by the headers of the e-mail, and thus not
directly controlled by the sender.

this patch removes the fallback to the content-type header mime-type
in case magic_mime_type_for_file does not detect the type.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/RuleDB/ArchiveFilter.pm     |  2 +-
 src/PMG/RuleDB/ContentTypeFilter.pm |  2 +-
 src/PMG/Utils.pm                    | 12 ++++--------
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
index 3d9890c..d7f6399 100644
--- a/src/PMG/RuleDB/ArchiveFilter.pm
+++ b/src/PMG/RuleDB/ArchiveFilter.pm
@@ -59,7 +59,7 @@ sub parse_entity {
     if (my $id = $entity->head->mime_attr ('x-proxmox-tmp-aid')) {
 	chomp $id;
 
-	my $header_ct = $entity->head->mime_attr ('content-type');
+	my $header_ct = $entity->{PMX_header_ct};
 
 	my $magic_ct = $entity->{PMX_magic_ct};
 
diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index 0199311..fb45e95 100644
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -72,7 +72,7 @@ sub parse_entity {
     if (my $id = $entity->head->mime_attr ('x-proxmox-tmp-aid')) {
 	chomp $id;
 
-	my $header_ct = $entity->head->mime_attr ('content-type');
+	my $header_ct = $entity->{PMX_header_ct};
 
 	my $magic_ct = $entity->{PMX_magic_ct};
 
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 0b8945f..b2a75fb 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -598,7 +598,7 @@ sub magic_mime_type_for_file {
     my $bufsize = Xdgmime::xdg_mime_get_max_buffer_extents();
     die "got strange value for max_buffer_extents" if $bufsize > 4096*10;
 
-    my $ct = "application/octet-stream";
+    my $ct;
 
     my $fh = IO::File->new("<$filename") ||
 	die "unable to open file '$filename' - $!";
@@ -611,6 +611,7 @@ sub magic_mime_type_for_file {
 
     die "unable to read file '$filename' - $!" if ($len < 0);
 
+    $ct ||= "application/octet-stream";
     return $ct;
 }
 
@@ -619,14 +620,9 @@ sub add_ct_marks {
 
     if (my $path = $entity->{PMX_decoded_path}) {
 
-	# set a reasonable default if magic does not give a result
-	$entity->{PMX_magic_ct} = $entity->head->mime_attr('content-type');
+	$entity->{PMX_header_ct} = $entity->head->mime_attr('content-type');
 
-	if (my $ct = magic_mime_type_for_file($path)) {
-	    if ($ct ne 'application/octet-stream' || !$entity->{PMX_magic_ct}) {
-		$entity->{PMX_magic_ct} = $ct;
-	    }
-	}
+	$entity->{PMX_magic_ct} = magic_mime_type_for_file($path);
 
 	my $filename = $entity->head->recommended_filename;
 	$filename = basename($path) if !defined($filename) || $filename eq '';
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 1/5] ruledb: disclaimer: simplify update-case Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 2/5] utils: content-type: don't fallback to header information for magic Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-21  9:49   ` Dominik Csapak
  2025-02-21  9:54   ` Dominik Csapak
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 4/5] ruledb: content-type: add flag for matching only based on magic/content Stoiko Ivanov
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

This restores the fall-back to the content-type from the header for
detecting archives, which we want to unpack, as eagerly trying to get
all information seems sensible in this case.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/bin/pmg-smtp-filter | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
index 6061459..6c03631 100755
--- a/src/bin/pmg-smtp-filter
+++ b/src/bin/pmg-smtp-filter
@@ -561,9 +561,15 @@ sub run_dequeue {
 sub unpack_entity {
     my ($self, $unpack, $entity, $msginfo, $queue) = @_;
 
-    my ($magic, $path) = $entity->@{'PMX_magic_ct', 'PMX_decoded_path'};
+    my ($magic, $headerct, $path) = $entity->@{'PMX_magic_ct', 'PMX_header_ct', 'PMX_decoded_path'};
+
+    if ($path) {
+	# in order to not miss information from a misdetected archive use information provided in the
+	# header here as well
+	if ($headerct && ($magic && $magic eq 'application/octet-stream')) {
+	    $magic = $headerct;
+	}
 
-    if ($magic && $path) {
 	my $filename = basename ($path);
 
 	if (PMG::Unpack::is_archive ($magic)) {
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-api v4 4/5] ruledb: content-type: add flag for matching only based on magic/content
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (2 preceding siblings ...)
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part Stoiko Ivanov
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

our current content-type matching is sensibly quite cautious in
matching if any available information indicates a potential match:
* mime-type detection based on file contents
* mime-type detection based on file suffix
* content-type header

Sometimes this can lead to surprises (e.g. when a MUA sets the
filetype of a pdf to application/octet-stream (the default type if no
information is available), or a filter for zip-files matching
docx-files.

This change gives users the option to restrict matching only on the
content as detected by xdg_mime_get_mime_type_for_data.

This is a fix for the intial request in #2691 and addresses the
suggestion from Friedrich from:
https://bugzilla.proxmox.com/show_bug.cgi?id=5618#c2

matches on the other items can be created with Match Field objects
(for the content-type header) and Filename (for the match based on the
provided filename - combinations of those should give us the complete
flexibility.

inspired by the changes for disclaimer released with PMG 8.1:
51d1507 ("fix #2430: ruledb disclaimer: make separator configurable")

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/RuleDB/ContentTypeFilter.pm | 75 ++++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index fb45e95..e44bf3c 100644
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -26,7 +26,7 @@ sub otype_text {
 }
 
 sub new {
-    my ($type, $fvalue, $ogroup) = @_;
+    my ($type, $fvalue, $ogroup, $only_content) = @_;
 
     my $class = ref($type) || $type;
 
@@ -36,6 +36,7 @@ sub new {
     }
 
     my $self = $class->SUPER::new('content-type', $fvalue, $ogroup);
+    $self->{only_content} = $only_content;
 
     return $self;
 }
@@ -52,9 +53,50 @@ sub load_attr {
 	$obj->{field_value} = $nt;
     }
 
+    my $sth = $ruledb->{dbh}->prepare(
+	"SELECT * FROM Attribut WHERE Object_ID = ?");
+
+    $sth->execute($id);
+
+    $obj->{only_content} = 0;
+
+    while (my $ref = $sth->fetchrow_hashref()) {
+	if ($ref->{name} eq 'only_content') {
+	    $obj->{only_content} = $ref->{value};
+	}
+    }
+
+    $sth->finish();
+
+    $obj->{id} = $id;
+
+    $obj->{digest} = Digest::SHA::sha1_hex( $id, $value, $ogroup, $obj->{only_content});
+
     return $obj;
 }
 
+sub save {
+    my ($self, $ruledb) = @_;
+
+    if (defined($self->{id})) {
+	#update - clean old attribut entries
+	$ruledb->{dbh}->do(
+	    "DELETE FROM Attribut WHERE Object_ID = ?",
+	    undef, $self->{id});
+    }
+
+    $self->{id} = $self->SUPER::save($ruledb);
+
+    if (defined($self->{only_content})) {
+	$ruledb->{dbh}->do(
+	    "INSERT INTO Attribut (Value, Name, Object_ID) VALUES (?, 'only_content', ?) ".
+	    "ON CONFLICT(Object_ID, Name) DO UPDATE SET Value = Excluded.Value ",
+	    undef, $self->{only_content},  $self->{id});
+    }
+
+    return $self->{id};
+}
+
 sub parse_entity {
     my ($self, $entity) = @_;
 
@@ -78,12 +120,16 @@ sub parse_entity {
 
 	my $glob_ct = $entity->{PMX_glob_ct};
 
-	if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
-	    push @$res, $id;
-	} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
-	    push @$res, $id;
-	} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
+	my $check_only_content = ${self}->{only_content} // 1;
+
+	if ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
 	    push @$res, $id;
+	} elsif (!$check_only_content) {
+	    if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
+		push @$res, $id;
+	    } elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
+		push @$res, $id;
+	    }
 	}
     }
 
@@ -112,19 +158,34 @@ sub properties {
 	    pattern => '[0-9a-zA-Z\/\\\[\]\+\-\.\*\_]+',
 	    maxLength => 1024,
 	},
+	'only-content' => {
+	    description => "use content-type from scanning only (ignore filename and header)",
+	    type => 'boolean',
+	    optional => 1,
+	    default => 0,
+	},
     };
 }
 
 sub get {
     my ($self) = @_;
 
-    return { contenttype => $self->{field_value} };
+    return {
+	contenttype => $self->{field_value},
+	'only-content' => $self->{only_content},
+    };
 }
 
 sub update {
     my ($self, $param) = @_;
 
     $self->{field_value} = $param->{contenttype};
+
+    if (defined($param->{'only-content'}) && $param->{'only-content'} == 1) {
+	$self->{only_content} = 1;
+    } else {
+	delete $self->{only_content};
+    }
 }
 
 1;
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (3 preceding siblings ...)
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 4/5] ruledb: content-type: add flag for matching only based on magic/content Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-21 10:10   ` Dominik Csapak
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 1/3] rules/object: remove icon from remove button Stoiko Ivanov
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

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@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;
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-gui v4 1/3] rules/object: remove icon from remove button
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (4 preceding siblings ...)
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 2/3] rules/content-typefilter: add checkbox for file content only matching Stoiko Ivanov
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

the icons were introduced to the listing, and it seems their rendering
when removing was not noticed - w/o this the message is e.g.:
"Are you sure you want to remove entry
'<span class="fa-fw fa fa-file-image-o'"></span> Content Type Filter:
..."

Fixes: ea4f2a7 ("add icons to the object types")

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 js/ObjectGroup.js | 3 +--
 js/Utils.js       | 5 +++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/js/ObjectGroup.js b/js/ObjectGroup.js
index 3c8de64..1807e97 100644
--- a/js/ObjectGroup.js
+++ b/js/ObjectGroup.js
@@ -125,8 +125,7 @@ Ext.define('PMG.ObjectGroup', {
 	    },
 	    callback: reload,
 	    getRecordName: function(rec) {
-		return PMG.Utils.format_otype(rec.data.otype) +
-		    ': ' + rec.data.descr;
+		return PMG.Utils.format_otype_subject(rec.data.otype) + ': ' + rec.data.descr;
 	    },
 	    waitMsgTarget: me,
 	});
diff --git a/js/Utils.js b/js/Utils.js
index 9b5f054..94e3c95 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -128,6 +128,11 @@ Ext.define('PMG.Utils', {
 	return icon + text;
     },
 
+    format_otype_subject: function(otype) {
+	let editor = PMG.Utils.object_editors[otype];
+	return editor.subject ?? 'unknown';
+    },
+
     format_otype: function(otype) {
 	let editor = PMG.Utils.object_editors[otype];
 	let iconCls = 'fa fa-question-circle';
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-gui v4 2/3] rules/content-typefilter: add checkbox for file content only matching
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (5 preceding siblings ...)
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 1/3] rules/object: remove icon from remove button Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 3/3] fix #2709: rules: match-field: add top-level-only checkbox Stoiko Ivanov
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 js/Utils.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/js/Utils.js b/js/Utils.js
index 94e3c95..257226d 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -416,6 +416,12 @@ Ext.define('PMG.Utils', {
 		    allowBlank: false,
 		    reset: Ext.emptyFn,
 		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'only-content',
+		    fieldLabel: gettext("Ignore header information"),
+		    uncheckedValue: '0',
+		},
 	    ],
 	},
 	3004: {
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [pmg-devel] [PATCH pmg-gui v4 3/3] fix #2709: rules: match-field: add top-level-only checkbox
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (6 preceding siblings ...)
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 2/3] rules/content-typefilter: add checkbox for file content only matching Stoiko Ivanov
@ 2025-02-20 20:13 ` Stoiko Ivanov
  2025-02-21 10:33 ` [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Dominik Csapak
  2025-02-21 16:12 ` Friedrich Weber
  9 siblings, 0 replies; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-20 20:13 UTC (permalink / raw)
  To: pmg-devel

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 js/Utils.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/js/Utils.js b/js/Utils.js
index 257226d..2dcf392 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -376,6 +376,12 @@ Ext.define('PMG.Utils', {
 		    fieldLabel: gettext('Test String'),
 		    regexFieldReference: 'value',
 		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'top-part-only',
+		    fieldLabel: gettext("Only top level headers"),
+		    uncheckedValue: '0',
+		},
 	    ],
 	},
 	3003: {
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well Stoiko Ivanov
@ 2025-02-21  9:49   ` Dominik Csapak
  2025-02-21  9:54   ` Dominik Csapak
  1 sibling, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2025-02-21  9:49 UTC (permalink / raw)
  To: Stoiko Ivanov, pmg-devel

On 2/20/25 21:13, Stoiko Ivanov wrote:
> This restores the fall-back to the content-type from the header for
> detecting archives, which we want to unpack, as eagerly trying to get
> all information seems sensible in this case.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>   src/bin/pmg-smtp-filter | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
> index 6061459..6c03631 100755
> --- a/src/bin/pmg-smtp-filter
> +++ b/src/bin/pmg-smtp-filter
> @@ -561,9 +561,15 @@ sub run_dequeue {
>   sub unpack_entity {
>       my ($self, $unpack, $entity, $msginfo, $queue) = @_;
>   
> -    my ($magic, $path) = $entity->@{'PMX_magic_ct', 'PMX_decoded_path'};
> +    my ($magic, $headerct, $path) = $entity->@{'PMX_magic_ct', 'PMX_header_ct', 'PMX_decoded_path'};
> +
> +    if ($path) {

while not really wrong (currently, since $path and $magic are always set or unset together)
not checking for $magic here sets us up for future bugs

we use both in the branch (independently of one another), so if some codechange in the future
would set $path but not $magic, we'd enter the branch with '$magic = undef' here unintentionally

so keeping the `if ($magic && $path)` here does not hurt us, and only makes
it clearer that both must be set for the branch to be executed..

> +	# in order to not miss information from a misdetected archive use information provided in the
> +	# header here as well
> +	if ($headerct && ($magic && $magic eq 'application/octet-stream')) {
> +	    $magic = $headerct;
> +	}
>   
> -    if ($magic && $path) {
>   	my $filename = basename ($path);
>   
>   	if (PMG::Unpack::is_archive ($magic)) {



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well Stoiko Ivanov
  2025-02-21  9:49   ` Dominik Csapak
@ 2025-02-21  9:54   ` Dominik Csapak
  1 sibling, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2025-02-21  9:54 UTC (permalink / raw)
  To: Stoiko Ivanov, pmg-devel

also (just remembered this)

IMHO this patch should be squashed into the previous one, because the change
is incomplete with just one of them


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part Stoiko Ivanov
@ 2025-02-21 10:10   ` Dominik Csapak
  2025-02-21 11:23     ` Stoiko Ivanov
  0 siblings, 1 reply; 16+ messages in thread
From: Dominik Csapak @ 2025-02-21 10:10 UTC (permalink / raw)
  To: Stoiko Ivanov, pmg-devel

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.

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



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (7 preceding siblings ...)
  2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 3/3] fix #2709: rules: match-field: add top-level-only checkbox Stoiko Ivanov
@ 2025-02-21 10:33 ` Dominik Csapak
  2025-02-21 16:12 ` Friedrich Weber
  9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2025-02-21 10:33 UTC (permalink / raw)
  To: Stoiko Ivanov, pmg-devel

tested a bit with swaks + manual injection with some rules + let the regression tests run

Seemed all to work as intended

aside from the small thing in api 3/5 (+squash into 2/5) consider this series

Tested-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part
  2025-02-21 10:10   ` Dominik Csapak
@ 2025-02-21 11:23     ` Stoiko Ivanov
  2025-02-21 12:01       ` Dominik Csapak
  0 siblings, 1 reply; 16+ messages in thread
From: Stoiko Ivanov @ 2025-02-21 11:23 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pmg-devel

On Fri, 21 Feb 2025 11:10:36 +0100
Dominik Csapak <d.csapak@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?


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



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part
  2025-02-21 11:23     ` Stoiko Ivanov
@ 2025-02-21 12:01       ` Dominik Csapak
  0 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2025-02-21 12:01 UTC (permalink / raw)
  To: Stoiko Ivanov; +Cc: pmg-devel

On 2/21/25 12:23, Stoiko Ivanov wrote:
> On Fri, 21 Feb 2025 11:10:36 +0100
> Dominik Csapak <d.csapak@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@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;
>>
>>
> 



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField
  2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
                   ` (8 preceding siblings ...)
  2025-02-21 10:33 ` [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Dominik Csapak
@ 2025-02-21 16:12 ` Friedrich Weber
  9 siblings, 0 replies; 16+ messages in thread
From: Friedrich Weber @ 2025-02-21 16:12 UTC (permalink / raw)
  To: Stoiko Ivanov, pmg-devel

On 20/02/2025 21:13, Stoiko Ivanov wrote:
> v3->v4:
> * rework patch 3/5 for pmg-api according to feedback from Dominik and Friedrich
>   after understanding that file-paths (and in result content-types are not
>   set for mail-parts that are multipart (mime-entities either have parts
>   or a body in general)

Looks like the "Use of uninitialized value" warning from v3 is gone, thx!

Tested a bit the new top-part-only and only-content flags, all seemed to
work as expected:

Tested-by: Friedrich Weber <f.weber@proxmox.com>



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-02-21 16:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-20 20:13 [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Stoiko Ivanov
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 1/5] ruledb: disclaimer: simplify update-case Stoiko Ivanov
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 2/5] utils: content-type: don't fallback to header information for magic Stoiko Ivanov
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 3/5] pmg-smtp-filter: archive-detection: use header information as well Stoiko Ivanov
2025-02-21  9:49   ` Dominik Csapak
2025-02-21  9:54   ` Dominik Csapak
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 4/5] ruledb: content-type: add flag for matching only based on magic/content Stoiko Ivanov
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-api v4 5/5] fix #2709: ruledb: match-field: optionally restrict to top mime-part Stoiko Ivanov
2025-02-21 10:10   ` Dominik Csapak
2025-02-21 11:23     ` Stoiko Ivanov
2025-02-21 12:01       ` Dominik Csapak
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 1/3] rules/object: remove icon from remove button Stoiko Ivanov
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 2/3] rules/content-typefilter: add checkbox for file content only matching Stoiko Ivanov
2025-02-20 20:13 ` [pmg-devel] [PATCH pmg-gui v4 3/3] fix #2709: rules: match-field: add top-level-only checkbox Stoiko Ivanov
2025-02-21 10:33 ` [pmg-devel] [PATCH pmg-api/pmg-gui v4] add additional attributes to ContentTypeFilter and MatchField Dominik Csapak
2025-02-21 16:12 ` Friedrich Weber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal