You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by do...@apache.org on 2005/05/21 05:22:57 UTC

svn commit: r171190 - /spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Author: dos
Date: Fri May 20 20:22:56 2005
New Revision: 171190

URL: http://svn.apache.org/viewcvs?rev=171190&view=rev
Log:
bug 4347: validate report_safe values

Modified:
    spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm?rev=171190&r1=171189&r2=171190&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm Fri May 20 20:22:56 2005
@@ -748,6 +748,10 @@
     default => 1,
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      if ($value !~ /^\s*(|0|1|2)\s*$/) {
+        return $INVALID_VALUE;
+      }
+
       $self->{report_safe} = $value+0;
       if (! $self->{report_safe}) {
         $self->{headers_spam}->{"Report"} = "_REPORT_";



Re: svn commit: r171190 - /spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Posted by "Daryl C. W. O'Shea" <sp...@dostech.ca>.
Theo Van Dinter wrote:
> On Sat, May 21, 2005 at 03:22:57AM -0000, dos@apache.org wrote:
> 
>>       my ($self, $key, $value, $line) = @_;
>>+      if ($value !~ /^\s*(|0|1|2)\s*$/) {
>>+        return $INVALID_VALUE;
>>+      }
>>+
>>       $self->{report_safe} = $value+0;
> 
> 
> -1
> 
> "report_safe" isn't valid, but this explicitly accepts it without error.
> In this case, if value is missing (always defined, but could be ''),
> we should return missing_value, and if value isn't [0-2], return
> invalid_value.

The preexisting  $self->{report_safe} = $value+0;   provided for 
"report_safe" by itself.  I didn't want to break that, and then have 
people complaining on the users' list.

> Also, $value has no whitespace at the front or the end, based on the parser,
> so there's no point in the \s* business. :)

OK.




Re: svn commit: r171190 - /spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Posted by Theo Van Dinter <fe...@kluge.net>.
On Sat, May 21, 2005 at 12:18:49AM -0400, Daryl C. W. O'Shea wrote:
> $self->{report_safe} was set to 0 when I put just "report_safe" in the 
> config file -- and I got no error.

Exactly, that's the problem.  ;)

'' + 0 == 0

But with warnings, perl will let you know that '' isn't numeric.
3.1 seems to do something to warnings such that this type of thing is
no longer reported, for better or worse.

-- 
Randomly Generated Tagline:
Know what I like about Windows? Not a damn thing.

Re: svn commit: r171190 - /spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Posted by "Daryl C. W. O'Shea" <sp...@dostech.ca>.
Theo Van Dinter wrote:
> (I originally responded in private, but then I noticed the same thing went to
> dev, so ... sending a copy of my response there too.)

Yeah, I hit the wrong key.

> On Sat, May 21, 2005 at 12:05:36AM -0400, Daryl C. W. O'Shea wrote:
> 
>>The preexisting  $self->{report_safe} = $value+0;   provided for 
>>"report_safe" by itself.  I didn't want to break that, and then have 
>>people complaining on the users' list.
> 
> 
> Well, that's actually the whole reason for bug 4347.  Missing values were
> throwing the "Argument '' isn't numeric in addition (+) at ..." warning.
> 
> In 3.1, the warning doesn't actually get displayed, probably due to Logger's
> warping of space-time.

$self->{report_safe} was set to 0 when I put just "report_safe" in the 
config file -- and I got no error.

I've got no issue with requiring what the POD asks for though.


Daryl




Re: svn commit: r171190 - /spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Posted by Theo Van Dinter <fe...@kluge.net>.
(I originally responded in private, but then I noticed the same thing went to
dev, so ... sending a copy of my response there too.)

On Sat, May 21, 2005 at 12:05:36AM -0400, Daryl C. W. O'Shea wrote:
> The preexisting  $self->{report_safe} = $value+0;   provided for 
> "report_safe" by itself.  I didn't want to break that, and then have 
> people complaining on the users' list.

Well, that's actually the whole reason for bug 4347.  Missing values were
throwing the "Argument '' isn't numeric in addition (+) at ..." warning.

In 3.1, the warning doesn't actually get displayed, probably due to Logger's
warping of space-time.

-- 
Randomly Generated Tagline:
"Oh gee, there it is, too bad."            - Prof. Farr

Re: svn commit: r171190 - /spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm

Posted by Theo Van Dinter <fe...@kluge.net>.
On Sat, May 21, 2005 at 03:22:57AM -0000, dos@apache.org wrote:
>        my ($self, $key, $value, $line) = @_;
> +      if ($value !~ /^\s*(|0|1|2)\s*$/) {
> +        return $INVALID_VALUE;
> +      }
> +
>        $self->{report_safe} = $value+0;

-1

"report_safe" isn't valid, but this explicitly accepts it without error.
In this case, if value is missing (always defined, but could be ''),
we should return missing_value, and if value isn't [0-2], return
invalid_value.

Also, $value has no whitespace at the front or the end, based on the parser,
so there's no point in the \s* business. :)

-- 
Randomly Generated Tagline:
"Where's Roxanne?  Not here today...  She might have a lab... Those poor
  Calc. 2 kids ..."               - Prof. Farr