You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by jm...@apache.org on 2005/12/12 23:23:47 UTC

svn commit: r356398 - in /spamassassin/trunk/lib/Mail: SpamAssassin.pm SpamAssassin/Conf.pm SpamAssassin/Conf/Parser.pm

Author: jm
Date: Mon Dec 12 14:23:43 2005
New Revision: 356398

URL: http://svn.apache.org/viewcvs?rev=356398&view=rev
Log:
add two features to core rule-parsing code; 1. optional behaviour to recurse through subdirs looking for .cf/.pre's, to support rules compilers working on rulesrc dir.  2. call back into invoking code on lint failure, so rule compiler can detect which rules exactly fail the lint check

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

Modified: spamassassin/trunk/lib/Mail/SpamAssassin.pm
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/lib/Mail/SpamAssassin.pm?rev=356398&r1=356397&r2=356398&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin.pm Mon Dec 12 14:23:43 2005
@@ -185,7 +185,7 @@
 
 =item rules_filename
 
-The filename to load spam-identifying rules from. (optional)
+The filename/directory to load spam-identifying rules from. (optional)
 
 =item site_rules_filename
 
@@ -199,6 +199,11 @@
 
 The directory user state is stored in. (optional)
 
+=item config_tree_recurse
+
+Set to C<1> to recurse through directories when reading configuration
+files, instead of just reading a single level.  (optional, default 0)
+
 =item config_text
 
 The text of all rules and preferences.  If you prefer not to load the rules
@@ -1608,24 +1613,44 @@
   $path;
 }
 
+###########################################################################
+
 sub get_cf_files_in_dir {
   my ($self, $dir) = @_;
-
-  opendir(SA_CF_DIR, $dir) or warn "config: cannot opendir $dir: $!\n";
-  my @cfs = grep { /\.cf$/i && -f "$dir/$_" } readdir(SA_CF_DIR);
-  closedir SA_CF_DIR;
-
-  return map { "$dir/$_" } sort { $a cmp $b } @cfs;
+  return $self->_get_cf_pre_files_in_dir($dir, 'cf');
 }
 
 sub get_pre_files_in_dir {
   my ($self, $dir) = @_;
+  return $self->_get_cf_pre_files_in_dir($dir, 'pre');
+}
+
+sub _get_cf_pre_files_in_dir {
+  my ($self, $dir, $type) = @_;
+
+  if ($self->{config_tree_recurse}) {
+    # use "eval" to avoid loading File::Find unless this is specified
+    eval {
+      use File::Find qw();
+
+      my @cfs = ();
+      File::Find::find(
+        sub {
+          return unless /\.${type}$/i && -f $_;
+          push @cfs, $File::Find::name;
+        }, $dir);
+      return map { "$dir/$_" } sort { $a cmp $b } @cfs;
+    };
 
-  opendir(SA_PRE_DIR, $dir) or warn "config: cannot opendir $dir: $!\n";
-  my @cfs = grep { /\.pre$/i && -f "$dir/$_" } readdir(SA_PRE_DIR);
-  closedir SA_PRE_DIR;
+    die "oops! $@";     # should never get here
+  }
+  else {
+    opendir(SA_CF_DIR, $dir) or warn "config: cannot opendir $dir: $!\n";
+    my @cfs = grep { /\.${type}$/i && -f "$dir/$_" } readdir(SA_CF_DIR);
+    closedir SA_CF_DIR;
 
-  return map { "$dir/$_" } sort { $a cmp $b } @cfs;
+    return map { "$dir/$_" } sort { $a cmp $b } @cfs;
+  }
 }
 
 ###########################################################################

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm?rev=356398&r1=356397&r2=356398&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm Mon Dec 12 14:23:43 2005
@@ -224,15 +224,8 @@
       }
 
       if ($relative && !exists $self->{scoreset}->[0]->{$rule}) {
-        my $msg = "config: relative score without previous setting in ".
-                    "configuration, skipping: $line";
-
-        if ($self->{lint_rules}) {
-          warn $msg."\n";
-        } else {
-          info($msg);
-        }
-        $self->{errors}++;
+        $self->{parser}->lint_warn("config: relative score without ".
+            "previous setting in configuration, skipping: $line", $rule);
         return $INVALID_VALUE;
       }
 
@@ -252,15 +245,9 @@
         }
       }
       else {
-        my $msg = "config: score configuration option without actual scores, skipping: $line";
-
-        if ($self->{lint_rules}) {
-          warn $msg."\n";
-        } else {
-          info($msg);
-        }
-        $self->{errors}++;
-        return $MISSING_REQUIRED_VALUE;
+        $self->{parser}->lint_warn("config: score configuration ".
+            "option without actual scores, skipping: $line", $rule);
+        return $INVALID_VALUE;
       }
     }
   });

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm?rev=356398&r1=356397&r2=356398&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm Mon Dec 12 14:23:43 2005
@@ -270,8 +270,10 @@
           my $cond = pop @if_stack;
 
           if ($cond->{type} eq 'if') {
-            warn "config: unclosed 'if' in ".
+            my $msg = "config: unclosed 'if' in ".
                   $self->{currentfile}.": if ".$cond->{conditional}."\n";
+            warn $msg;
+            $self->lint_warn($msg, undef);
           }
           else {
             # die seems a bit excessive here, but this shouldn't be possible
@@ -279,7 +281,6 @@
             die "config: unknown 'if' type: ".$cond->{type}."\n";
           }
 
-          $conf->{errors}++;
           @if_stack = ();
         }
         $skip_parsing = 0;
@@ -339,13 +340,14 @@
       #$value =~ s/^(\d+)\.(\d{1,3}).*$/sprintf "%d.%d", $1, $2/e;
 
       if ($ver ne $value) {
-        warn "config: configuration file \"$self->{currentfile}\" requires version ".
-                "$value of SpamAssassin, but this is code version ".
+        my $msg = "config: configuration file \"$self->{currentfile}\" requires ".
+                "version $value of SpamAssassin, but this is code version ".
                 "$ver. Maybe you need to use ".
                 "the -C switch, or remove the old config files? ".
                 "Skipping this file";
+        warn $msg;
+        $self->lint_warn($msg, undef);
         $skip_parsing = 1;
-        $conf->{errors}++;
       }
       next;
     }
@@ -424,15 +426,7 @@
       }
     }
 
-    if ($conf->{lint_rules}) {
-      warn $msg."\n";
-    } else {
-      info($msg);
-    }
-
-    if ($is_error) {
-      $conf->{errors}++;
-    }
+    $self->lint_warn($msg, undef, $is_error);
   }
 
   $self->lint_check();
@@ -471,7 +465,7 @@
   }
 
   if ($bad) {
-    $conf->{errors}++;
+    $self->lint_warn("bad 'if' line", undef);
     return -1;
   }
 
@@ -508,15 +502,13 @@
     # Check for description and score issues in lint fashion
     while ( ($k,$v) = each %{$conf->{descriptions}} ) {
       if (!exists $conf->{tests}->{$k}) {
-        warn "config: warning: description exists for non-existent rule $k\n";
-        $conf->{errors}++;
+        $self->lint_warn("config: warning: description exists for non-existent rule $k\n", $k);
       }
     }
 
     while ( my($sk) = each %{$conf->{scores}} ) {
       if (!exists $conf->{tests}->{$sk}) {
-        warn "config: warning: score set for non-existent rule $sk\n";
-        $conf->{errors}++;
+        $self->lint_warn("config: warning: score set for non-existent rule $k\n", $k);
       }
     }
   }
@@ -682,8 +674,7 @@
 	}
         unshift(@args, $function);
 	if ($args) {
-	  $conf->{errors}++;
-	  warn("syntax error (unparsable argument: $args) for eval function: $name: $text");
+          $self->lint_warn("syntax error (unparsable argument: $args) for eval function: $name: $text", $name);
 	}
         elsif ($type == $Mail::SpamAssassin::Conf::TYPE_BODY_EVALS) {
           $conf->{body_evals}->{$priority}->{$name} = \@args;
@@ -705,13 +696,11 @@
         #  $conf->{uri_evals}->{$priority}->{$name} = \@args;
         #}
         else {
-          $conf->{errors}++;
-	  warn("unknown type $type for $name: $text");
+          $self->lint_warn("unknown type $type for $name: $text", $name);
         }
       }
       else {
-        $conf->{errors}++;
-        warn("syntax error for eval function $name: $text");
+        $self->lint_warn("syntax error for eval function $name: $text", $name);
       }
     }
     # non-eval tests
@@ -744,8 +733,7 @@
         $conf->{full_tests}->{$priority}->{$name} = $text;
       }
       else {
-        $conf->{errors}++;
-        warn("unknown type $type for $name: $text");
+        $self->lint_warn("unknown type $type for $name: $text", $name);
       }
     }
   }
@@ -765,9 +753,8 @@
 
   # Don't allow invalid names ...
   if ($name !~ /^\D\w*$/) {
-    warn "config: error: rule '$name' has invalid characters ".
-	   "(not Alphanumeric + Underscore + starting with a non-digit)\n";
-    $conf->{errors}++;
+    $self->lint_warn("config: error: rule '$name' has invalid characters ".
+	   "(not Alphanumeric + Underscore + starting with a non-digit)\n", $name);
     return;
   }
 
@@ -775,18 +762,16 @@
   # characters throw warnings).  Check this separately from the above
   # pattern to avoid vague error messages.
   if (length $name > 200) {
-    warn "config: error: rule '$name' is way too long ".
-	   "(recommended maximum length is 22 characters)\n";
-    $conf->{errors}++;
+    $self->lint_warn("config: error: rule '$name' is way too long ".
+	   "(recommended maximum length is 22 characters)\n", $name);
     return;
   }
 
   # Warn about, but use, long rule names during --lint
   if ($conf->{lint_rules}) {
     if (length($name) > 50 && $name !~ /^__/ && $name !~ /^T_/) {
-      warn "config: warning: rule name '$name' is over 50 chars ".
-	     "(recommended maximum length is 22 characters)\n";
-      $conf->{errors}++;
+      $self->lint_warn("config: warning: rule name '$name' is over 50 chars ".
+	     "(recommended maximum length is 22 characters)\n", $name);
     }
   }
 
@@ -865,8 +850,7 @@
     my $err = $@;
     $err =~ s/\s+(?:at|near)\b.*//s;
     $err =~ s/Illegal division by zero/division by zero possible/i;
-    warn "config: invalid expression for rule $name: \"$rule\": $err\n";
-    $self->{conf}->{errors}++;
+    $self->lint_warn("config: invalid expression for rule $name: \"$rule\": $err\n", $name);
     return 0;
   }
 }
@@ -875,8 +859,7 @@
   my ($self, $name, $re) = @_;
 
   unless ($re =~ /^\s*m?(\W).*(?:\1|>|}|\)|\])[a-z]*\s*$/) {
-    warn "config: invalid regexp for rule $name: $re: missing or invalid delimiters\n";
-    $self->{conf}->{errors}++;
+    $self->lint_warn("config: invalid regexp for rule $name: $re: missing or invalid delimiters\n", $name);
     return 0;
   }
   return $self->is_regexp_valid($name, $re);
@@ -925,8 +908,7 @@
 
   my $err = $@;
   $err =~ s/ at .*? line \d.*$//;
-  warn "config: invalid regexp for rule $name: $origre: $err\n";
-  $self->{conf}->{errors}++;
+  $self->lint_warn("config: invalid regexp for rule $name: $origre: $err\n", $name);
   return 0;
 }
 
@@ -995,6 +977,32 @@
     dbg("plugin: fixed relative path: $path");
   }
   return $path;
+}
+
+###########################################################################
+
+sub lint_warn {
+  my ($self, $msg, $rule, $iserror) = @_;
+
+  if (!defined $iserror) { $iserror = 1; }
+
+  if ($self->{main}->{lint_callback}) {
+    $self->{main}->{lint_callback}->(
+          msg => $msg,
+          rule => $rule,
+          iserror => $iserror
+        );
+  }
+  elsif ($self->{conf}->{lint_rules}) {
+    warn $msg."\n";
+  }
+  else {
+    info($msg);
+  }
+
+  if ($iserror) {
+    $self->{conf}->{errors}++;
+  }
 }
 
 ###########################################################################