all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save
@ 2023-06-30  8:27 Dominik Csapak
  2023-06-30  8:27 ` [pmg-devel] [PATCH pmg-api v2 2/2] statistics: fix update virusinfo Dominik Csapak
  2023-12-15 16:44 ` [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Stoiko Ivanov
  0 siblings, 2 replies; 7+ messages in thread
From: Dominik Csapak @ 2023-06-30  8:27 UTC (permalink / raw)
  To: pmg-devel

and warn only when it's an invalid regex on execution, because users may
have previously had such rules. Otherwise, pmg-smtp-filter will restart
every time it encounters such a rule.

do so for every rule type that uses a regex to match

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* add it for all relevant rule types (for those with recursive calls
  only check it once during execution)

 src/PMG/RuleDB/ArchiveFilter.pm        |  6 ++++++
 src/PMG/RuleDB/ContentTypeFilter.pm    |  7 +++++++
 src/PMG/RuleDB/MatchArchiveFilename.pm |  7 +++++++
 src/PMG/RuleDB/MatchFilename.pm        | 14 +++++++++++---
 src/PMG/RuleDB/WhoRegex.pm             | 12 +++++++++++-
 5 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
index 6d91556..3d9890c 100644
--- a/src/PMG/RuleDB/ArchiveFilter.pm
+++ b/src/PMG/RuleDB/ArchiveFilter.pm
@@ -48,6 +48,12 @@ sub parse_entity {
 
     my $res;
 
+    # test regex for validity
+    eval { "" =~ m|$self->{field_value}|; };
+    if (my $err = $@) {
+	warn "invalid regex: $err\n";
+	return $res;
+    }
     # match subtypes? We currently do exact matches only.
 
     if (my $id = $entity->head->mime_attr ('x-proxmox-tmp-aid')) {
diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index 76fc1ce..0199311 100644
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -60,6 +60,13 @@ sub parse_entity {
 
     my $res;
 
+    # test regex for validity
+    eval { "" =~ m|$self->{field_value}|; };
+    if (my $err = $@) {
+	warn "invalid regex: $err\n";
+	return $res;
+    }
+
     # match subtypes? We currently do exact matches only.
 
     if (my $id = $entity->head->mime_attr ('x-proxmox-tmp-aid')) {
diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
index 2ef3543..5b1cb6d 100644
--- a/src/PMG/RuleDB/MatchArchiveFilename.pm
+++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
@@ -25,6 +25,13 @@ sub parse_entity {
 
     my $res;
 
+    # test regex for validity
+    eval { "" =~ m|^$self->{fname}$|i; };
+    if (my $err = $@) {
+	warn "invalid regex: $err\n";
+	return $res;
+    }
+
     if (my $id = $entity->head->mime_attr('x-proxmox-tmp-aid')) {
 	chomp $id;
 
diff --git a/src/PMG/RuleDB/MatchFilename.pm b/src/PMG/RuleDB/MatchFilename.pm
index c9cdbe0..f6449c4 100644
--- a/src/PMG/RuleDB/MatchFilename.pm
+++ b/src/PMG/RuleDB/MatchFilename.pm
@@ -58,6 +58,11 @@ sub save {
     defined($self->{ogroup}) || die "undefined ogroup: ERROR";
 
     my $new_value = $self->{fname};
+
+    # test regex for validity
+    eval { "" =~ m|^$new_value$|i; };
+    die "invalid regex: $@\n" if $@;
+
     $new_value =~ s/\\/\\\\/g;
     $new_value = encode('UTF-8', $new_value);
 
@@ -91,9 +96,12 @@ sub parse_entity {
 	chomp $id;
 
 	if (my $value = PMG::Utils::extract_filename($entity->head)) {
-	    if ($value =~ m|^$self->{fname}$|i) {
-		push @$res, $id;
-	    }
+	    eval {
+		if ($value =~ m|^$self->{fname}$|i) {
+		    push @$res, $id;
+		}
+	    };
+	    warn "invalid regex: $@\n" if $@;
 	}
     }
 
diff --git a/src/PMG/RuleDB/WhoRegex.pm b/src/PMG/RuleDB/WhoRegex.pm
index 5c13604..1db6418 100644
--- a/src/PMG/RuleDB/WhoRegex.pm
+++ b/src/PMG/RuleDB/WhoRegex.pm
@@ -60,6 +60,11 @@ sub save {
     defined($self->{address}) || die "undefined address: ERROR";
 
     my $adr = $self->{address};
+
+    # test regex for validity
+    eval { "" =~ /^$adr$/i; };
+    die "invalid regex: $@\n" if $@;
+
     $adr =~ s/\\/\\\\/g;
     $adr = encode('UTF-8', $adr);
 
@@ -100,7 +105,12 @@ sub who_match {
 
     my $t = $self->address;
 
-    return $addr =~ m/^$t$/i;
+    my $res = '';
+    eval {
+	$res = $addr =~ m/^$t$/i;
+    };
+    warn "invalid regex: $@\n" if $@;
+    return $res;
 }
 
 sub address { 
-- 
2.30.2





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

* [pmg-devel] [PATCH pmg-api v2 2/2] statistics: fix update virusinfo
  2023-06-30  8:27 [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Dominik Csapak
@ 2023-06-30  8:27 ` Dominik Csapak
  2023-06-30 13:41   ` [pmg-devel] applied: " Stoiko Ivanov
  2023-12-15 16:44 ` [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Stoiko Ivanov
  1 sibling, 1 reply; 7+ messages in thread
From: Dominik Csapak @ 2023-06-30  8:27 UTC (permalink / raw)
  To: pmg-devel

by moving the closing parenthesis to the correct place

Fixes: 9972a7c ("postgresql compat: cast result from EXTRACT to INTEGER")
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* remove the wrongly set parenthesis
 src/PMG/Statistic.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PMG/Statistic.pm b/src/PMG/Statistic.pm
index 950dbc2..ef4f5ba 100644
--- a/src/PMG/Statistic.pm
+++ b/src/PMG/Statistic.pm
@@ -342,7 +342,7 @@ sub update_stats_virusinfo  {
 	my $sql = '';
 
 	push @values, "Count = Count + $ref->{count}" if $ref->{count};
-	push @values, "MTime = EXTRACT(EPOCH FROM now()::INTEGER)";
+	push @values, "MTime = EXTRACT(EPOCH FROM now())::INTEGER";
 
 	if (scalar (@values)) {
 	    $sql .= "UPDATE VirusInfo SET ";
-- 
2.30.2





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

* [pmg-devel] applied: [PATCH pmg-api v2 2/2] statistics: fix update virusinfo
  2023-06-30  8:27 ` [pmg-devel] [PATCH pmg-api v2 2/2] statistics: fix update virusinfo Dominik Csapak
@ 2023-06-30 13:41   ` Stoiko Ivanov
  0 siblings, 0 replies; 7+ messages in thread
From: Stoiko Ivanov @ 2023-06-30 13:41 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pmg-devel

applied this one to master and stable-7

Thanks for finding and fixing this so quickly!


On Fri, 30 Jun 2023 10:27:48 +0200
Dominik Csapak <d.csapak@proxmox.com> wrote:

> by moving the closing parenthesis to the correct place
> 
> Fixes: 9972a7c ("postgresql compat: cast result from EXTRACT to INTEGER")
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * remove the wrongly set parenthesis
>  src/PMG/Statistic.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PMG/Statistic.pm b/src/PMG/Statistic.pm
> index 950dbc2..ef4f5ba 100644
> --- a/src/PMG/Statistic.pm
> +++ b/src/PMG/Statistic.pm
> @@ -342,7 +342,7 @@ sub update_stats_virusinfo  {
>  	my $sql = '';
>  
>  	push @values, "Count = Count + $ref->{count}" if $ref->{count};
> -	push @values, "MTime = EXTRACT(EPOCH FROM now()::INTEGER)";
> +	push @values, "MTime = EXTRACT(EPOCH FROM now())::INTEGER";
>  
>  	if (scalar (@values)) {
>  	    $sql .= "UPDATE VirusInfo SET ";





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

* Re: [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save
  2023-06-30  8:27 [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Dominik Csapak
  2023-06-30  8:27 ` [pmg-devel] [PATCH pmg-api v2 2/2] statistics: fix update virusinfo Dominik Csapak
@ 2023-12-15 16:44 ` Stoiko Ivanov
  2023-12-15 17:06   ` Stoiko Ivanov
  2023-12-18 13:33   ` Dominik Csapak
  1 sibling, 2 replies; 7+ messages in thread
From: Stoiko Ivanov @ 2023-12-15 16:44 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pmg-devel

Thanks for the patch, this definitely improves UX!

one tiny nit in-line (I'll gladly change that upon applying directly, but
just don't want to miss anything):

On Fri, Jun 30, 2023 at 10:27:47AM +0200, Dominik Csapak wrote:
> and warn only when it's an invalid regex on execution, because users may
> have previously had such rules. Otherwise, pmg-smtp-filter will restart
> every time it encounters such a rule.
> 
> do so for every rule type that uses a regex to match
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> ..snip..
> diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
> index 2ef3543..5b1cb6d 100644
> --- a/src/PMG/RuleDB/MatchArchiveFilename.pm
> +++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
> @@ -25,6 +25,13 @@ sub parse_entity {
>  
>      my $res;
>  
> +    # test regex for validity
> +    eval { "" =~ m|^$self->{fname}$|i; };
> +    if (my $err = $@) {
> +	warn "invalid regex: $err\n";
> +	return $res;
> +    }
> +
>      if (my $id = $entity->head->mime_attr('x-proxmox-tmp-aid')) {
>  	chomp $id;
>  
> ..snip..
> diff --git a/src/PMG/RuleDB/WhoRegex.pm b/src/PMG/RuleDB/WhoRegex.pm
> index 5c13604..1db6418 100644
> --- a/src/PMG/RuleDB/WhoRegex.pm
> +++ b/src/PMG/RuleDB/WhoRegex.pm
> @@ -60,6 +60,11 @@ sub save {
>      defined($self->{address}) || die "undefined address: ERROR";
>  
>      my $adr = $self->{address};
> +
> +    # test regex for validity
> +    eval { "" =~ /^$adr$/i; };
> +    die "invalid regex: $@\n" if $@;
> +
>      $adr =~ s/\\/\\\\/g;
>      $adr = encode('UTF-8', $adr);
>  
> @@ -100,7 +105,12 @@ sub who_match {
>  
>      my $t = $self->address;
>  
> -    return $addr =~ m/^$t$/i;
> +    my $res = '';
we use the result as condition in an if (pmg-smtp-filter apply_rules),
I'd rather leave it as undef instead of '' - for consistency with the
other objects (see diff-context above) - as I asked myself a bit too long
why this needs to be an '' here instead of undef..
or am I missing something?


> +    eval {
> +	$res = $addr =~ m/^$t$/i;
> +    };
> +    warn "invalid regex: $@\n" if $@;
> +    return $res;
>  }
>  
>  sub address { 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
> 




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

* Re: [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save
  2023-12-15 16:44 ` [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Stoiko Ivanov
@ 2023-12-15 17:06   ` Stoiko Ivanov
  2023-12-18 13:34     ` Dominik Csapak
  2023-12-18 13:33   ` Dominik Csapak
  1 sibling, 1 reply; 7+ messages in thread
From: Stoiko Ivanov @ 2023-12-15 17:06 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pmg-devel

After testing this a bit more I realized that anchored regexes, don't die
on '^*foo$' (the quite common case of mistaking regex for globs (especially
for filenames) - the warning is printed to the journal - but I can happily
store it (additionally the eval-block in the what/who_match never dies -
and the 'invalid regex:' does not get printed.

maybe we could improve this even further by die'ing upon a warning when
saving the regex? - then the question is if we want to add the eval+warning
code in the who/what_match subs at all (afaict this would only add 'invalid
regex:' to the same log-line.

In any case - thanks for the patch - I never took the time to find out why
some invalid-regexes cause pmg-smtp-filter to exit+respawn, and some others
don't ;)


On Fri, Dec 15, 2023 at 05:44:19PM +0100, Stoiko Ivanov wrote:
> Thanks for the patch, this definitely improves UX!
> 
> one tiny nit in-line (I'll gladly change that upon applying directly, but
> just don't want to miss anything):
>..snip.. 




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

* Re: [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save
  2023-12-15 16:44 ` [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Stoiko Ivanov
  2023-12-15 17:06   ` Stoiko Ivanov
@ 2023-12-18 13:33   ` Dominik Csapak
  1 sibling, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2023-12-18 13:33 UTC (permalink / raw)
  To: Stoiko Ivanov; +Cc: pmg-devel

On 12/15/23 17:44, Stoiko Ivanov wrote:
> Thanks for the patch, this definitely improves UX!
> 
> one tiny nit in-line (I'll gladly change that upon applying directly, but
> just don't want to miss anything):
> 
> On Fri, Jun 30, 2023 at 10:27:47AM +0200, Dominik Csapak wrote:
>> and warn only when it's an invalid regex on execution, because users may
>> have previously had such rules. Otherwise, pmg-smtp-filter will restart
>> every time it encounters such a rule.
>>
>> do so for every rule type that uses a regex to match
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> changes from v1:
>> ..snip..
>> diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
>> index 2ef3543..5b1cb6d 100644
>> --- a/src/PMG/RuleDB/MatchArchiveFilename.pm
>> +++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
>> @@ -25,6 +25,13 @@ sub parse_entity {
>>   
>>       my $res;
>>   
>> +    # test regex for validity
>> +    eval { "" =~ m|^$self->{fname}$|i; };
>> +    if (my $err = $@) {
>> +	warn "invalid regex: $err\n";
>> +	return $res;
>> +    }
>> +
>>       if (my $id = $entity->head->mime_attr('x-proxmox-tmp-aid')) {
>>   	chomp $id;
>>   
>> ..snip..
>> diff --git a/src/PMG/RuleDB/WhoRegex.pm b/src/PMG/RuleDB/WhoRegex.pm
>> index 5c13604..1db6418 100644
>> --- a/src/PMG/RuleDB/WhoRegex.pm
>> +++ b/src/PMG/RuleDB/WhoRegex.pm
>> @@ -60,6 +60,11 @@ sub save {
>>       defined($self->{address}) || die "undefined address: ERROR";
>>   
>>       my $adr = $self->{address};
>> +
>> +    # test regex for validity
>> +    eval { "" =~ /^$adr$/i; };
>> +    die "invalid regex: $@\n" if $@;
>> +
>>       $adr =~ s/\\/\\\\/g;
>>       $adr = encode('UTF-8', $adr);
>>   
>> @@ -100,7 +105,12 @@ sub who_match {
>>   
>>       my $t = $self->address;
>>   
>> -    return $addr =~ m/^$t$/i;
>> +    my $res = '';
> we use the result as condition in an if (pmg-smtp-filter apply_rules),
> I'd rather leave it as undef instead of '' - for consistency with the
> other objects (see diff-context above) - as I asked myself a bit too long
> why this needs to be an '' here instead of undef..
> or am I missing something?

nope, returning undef is better, i'll send a v3
> 
> 
>> +    eval {
>> +	$res = $addr =~ m/^$t$/i;
>> +    };
>> +    warn "invalid regex: $@\n" if $@;
>> +    return $res;
>>   }
>>   
>>   sub address {
>> -- 
>> 2.30.2
>>
>>
>>
>> _______________________________________________
>> pmg-devel mailing list
>> pmg-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>>
>>





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

* Re: [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save
  2023-12-15 17:06   ` Stoiko Ivanov
@ 2023-12-18 13:34     ` Dominik Csapak
  0 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2023-12-18 13:34 UTC (permalink / raw)
  To: Stoiko Ivanov; +Cc: pmg-devel

On 12/15/23 18:06, Stoiko Ivanov wrote:
> After testing this a bit more I realized that anchored regexes, don't die
> on '^*foo$' (the quite common case of mistaking regex for globs (especially
> for filenames) - the warning is printed to the journal - but I can happily
> store it (additionally the eval-block in the what/who_match never dies -
> and the 'invalid regex:' does not get printed.
> 
> maybe we could improve this even further by die'ing upon a warning when
> saving the regex? - then the question is if we want to add the eval+warning
> code in the who/what_match subs at all (afaict this would only add 'invalid
> regex:' to the same log-line.

can we check if the regex match warns though? how would we do that?

> 
> In any case - thanks for the patch - I never took the time to find out why
> some invalid-regexes cause pmg-smtp-filter to exit+respawn, and some others
> don't ;)
> 
> 
> On Fri, Dec 15, 2023 at 05:44:19PM +0100, Stoiko Ivanov wrote:
>> Thanks for the patch, this definitely improves UX!
>>
>> one tiny nit in-line (I'll gladly change that upon applying directly, but
>> just don't want to miss anything):
>> ..snip..





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

end of thread, other threads:[~2023-12-18 13:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-30  8:27 [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Dominik Csapak
2023-06-30  8:27 ` [pmg-devel] [PATCH pmg-api v2 2/2] statistics: fix update virusinfo Dominik Csapak
2023-06-30 13:41   ` [pmg-devel] applied: " Stoiko Ivanov
2023-12-15 16:44 ` [pmg-devel] [PATCH pmg-api v2 1/2] fix #4811: rule db: test regex validity on save Stoiko Ivanov
2023-12-15 17:06   ` Stoiko Ivanov
2023-12-18 13:34     ` Dominik Csapak
2023-12-18 13:33   ` Dominik Csapak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal