[pmg-devel] [PATCH pmg-api v2 0/3] fix content-type matching for parts without bodyhandle.

[pmg-devel] [PATCH pmg-api v2 0/3] fix content-type matching for parts without bodyhandle.

[Stoiko Ivanov]

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

v1->v2: incorporated Dominik's feedback and added patch 3 which should
simplify the code a bit.

patches 2+3 could probably be squashed - if you want I can resent - else -
feel free to do so when applying.

tested with the failing regression-test, which now works again.


Stoiko Ivanov (3):
  ruledb: content-type: drop unreachable fallback for only_content
  ruledb: content-type: do not rely on PMX_header_ct
  utils: drop unneeded PMX_header_ct

 src/PMG/RuleDB/ArchiveFilter.pm     | 2 +-
 src/PMG/RuleDB/ContentTypeFilter.pm | 4 ++--
 src/PMG/Utils.pm                    | 2 --
 src/bin/pmg-smtp-filter             | 6 ++++--
 4 files changed, 7 insertions(+), 7 deletions(-)

-- 
2.39.5



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

[pmg-devel] [PATCH pmg-api v2 1/3] ruledb: content-type: drop unreachable fallback for only_content

[Stoiko Ivanov]

the default of 'only_content' gets set when fetching the entries from the
database in load_attr - and it defaults to false.

the `// 1` is thus never needed and confusing.

Noticed while debugging another issue, and considered if this false
setting might be the cause.

Fixes:
af418f4 ("ruledb: content-type: add flag for matching only based on magic/content")

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/RuleDB/ContentTypeFilter.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index 6818de1..ef1d377 100644
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -120,7 +120,7 @@ sub parse_entity {
 
 	my $glob_ct = $entity->{PMX_glob_ct};
 
-	my $check_only_content = ${self}->{only_content} // 1;
+	my $check_only_content = ${self}->{only_content};
 
 	if ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
 	    push @$res, $id;
-- 
2.39.5



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

[pmg-devel] [PATCH pmg-api v2 2/3] ruledb: content-type: do not rely on PMX_header_ct

[Stoiko Ivanov]

as noted in the cover-letter for v3 of the series introducing this:
https://lore.proxmox.com/pmg-devel/74a089b6-d858-4525-9822-da58b9c8f4d7@proxmox.com/T/#t

the PMX_magic_ct, PMX_glob_ct and PMX_header_ct are only set if there
is a bodyhandle for a mime-part, which only exists if the part has no
sub-parts.

However we want to match on content-types even for mail-parts which
have no bodyhandle:
* our internal regression-test suite has a mail with `message/partial`
  set
* there are probably sensible uses for matching `multipart/related`
  parts (e.g. when trying to get a negated match.

Noticed during a check of our regression-tests.

Fixes:
c320d56 ("utils: content-type: don't fallback to header information for magic")

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/RuleDB/ArchiveFilter.pm     | 2 +-
 src/PMG/RuleDB/ContentTypeFilter.pm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
index d7f6399..3d9890c 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->{PMX_header_ct};
+	my $header_ct = $entity->head->mime_attr ('content-type');
 
 	my $magic_ct = $entity->{PMX_magic_ct};
 
diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index ef1d377..2308829 100644
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -114,7 +114,7 @@ sub parse_entity {
     if (my $id = $entity->head->mime_attr ('x-proxmox-tmp-aid')) {
 	chomp $id;
 
-	my $header_ct = $entity->{PMX_header_ct};
+	my $header_ct = $entity->head->mime_attr ('content-type');
 
 	my $magic_ct = $entity->{PMX_magic_ct};
 
-- 
2.39.5



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

[pmg-devel] [PATCH pmg-api v2 3/3] utils: drop unneeded PMX_header_ct

[Stoiko Ivanov]

The variable was introduced in a early version of
c320d56 ("utils: content-type: don't fallback to header information for magic"
without realizing that the PMX_magic|glob_ct variables are only set
for MIME-Parts with bodyhandle - and expecting to replace the
header-lookup in multiple places. As the header is also present and
needed for MIME-Parts without bodyhandle (parts that have subparts
have no bodyhandle) - drop the variable, in order to cause less
confusion.

Also drop '$magic' as part of the nested if-condition, as it is
already checked in the if one line above.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Utils.pm        | 2 --
 src/bin/pmg-smtp-filter | 6 ++++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index b2a75fb..b225a9e 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -620,8 +620,6 @@ sub add_ct_marks {
 
     if (my $path = $entity->{PMX_decoded_path}) {
 
-	$entity->{PMX_header_ct} = $entity->head->mime_attr('content-type');
-
 	$entity->{PMX_magic_ct} = magic_mime_type_for_file($path);
 
 	my $filename = $entity->head->recommended_filename;
diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
index 60737ea..760a2ef 100755
--- a/src/bin/pmg-smtp-filter
+++ b/src/bin/pmg-smtp-filter
@@ -561,12 +561,14 @@ sub run_dequeue {
 sub unpack_entity {
     my ($self, $unpack, $entity, $msginfo, $queue) = @_;
 
-    my ($magic, $headerct, $path) = $entity->@{'PMX_magic_ct', 'PMX_header_ct', 'PMX_decoded_path'};
+    my ($magic, $path) = $entity->@{'PMX_magic_ct', 'PMX_decoded_path'};
 
     if ($magic && $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')) {
+
+	my $headerct = $entity->head->mime_attr('content-type');
+	if ($headerct && $magic eq 'application/octet-stream') {
 	    $magic = $headerct;
 	}
 
-- 
2.39.5



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

Re: [pmg-devel] [PATCH pmg-api v2 0/3] fix content-type matching for parts without bodyhandle.

[Dominik Csapak]

LGTM now, regression tests still work, my swaks tests still work

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-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

[pmg-devel] applied: [PATCH pmg-api v2 0/3] fix content-type matching for parts without bodyhandle.

[Thomas Lamprecht]

Am 25.02.25 um 12:15 schrieb Stoiko Ivanov:
> 
> supersedes:
> https://lore.proxmox.com/pmg-devel/20250224210728.378493-1-s.ivanov@proxmox.com/T/#t
> 
> v1->v2: incorporated Dominik's feedback and added patch 3 which should
> simplify the code a bit.
> 
> patches 2+3 could probably be squashed - if you want I can resent - else -
> feel free to do so when applying.
> 
> tested with the failing regression-test, which now works again.
> 
> 
> Stoiko Ivanov (3):
>   ruledb: content-type: drop unreachable fallback for only_content
>   ruledb: content-type: do not rely on PMX_header_ct
>   utils: drop unneeded PMX_header_ct
> 
>  src/PMG/RuleDB/ArchiveFilter.pm     | 2 +-
>  src/PMG/RuleDB/ContentTypeFilter.pm | 4 ++--
>  src/PMG/Utils.pm                    | 2 --
>  src/bin/pmg-smtp-filter             | 6 ++++--
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 


applied series with Dominiks R-b and T-b and amended the commit message
to not break the Fixes trailer over multiple lines, thanks!


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