svn commit: r1933760 - in spamassassin/trunk: . lib/Mail/SpamAssassin lib/Mail/SpamAssassin/Message lib/Mail/SpamAssassin/Plugin

[email protected] Sun, 03 May 2026 07:44:36 -0000
Newsgroups gmane.mail.spam.spamassassin.cvs
Message-ID <177779427652.1965302.4531963225454231802@svn03-he-fi>
Author: fkento
Date: Sun May  3 07:44:36 2026
New Revision: 1933760

Log:
Add eval:vertical_whitespace(<threshold>)

Triggers when the number of consecutive blanks lines exceeds <threshold>

This offers several improvements over eval:check_blank_line_ratio():

check_blank_line_ratio
- Calls get_decoded_body_text_array()
- Includes all parts
- Includes raw html markup
- Includes invisible text
- Counts all blank lines

vertical_whitespace
- Calls visible_rendered()
- Includes message body only (no attachments)
- Renders HTML
- Excludes invisible text
- Counts consecutive blank lines

The changes to HTML.pm are to
- Render whitespace closer to how an MUA would render it
- Prevent false positives because **too much** whitespace was in the rendered output

Modified:
   spamassassin/trunk/MANIFEST
   spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm
   spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm
   spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyEval.pm

Modified: spamassassin/trunk/MANIFEST
==============================================================================
--- spamassassin/trunk/MANIFEST	Sun May  3 06:12:58 2026	(r1933759)
+++ spamassassin/trunk/MANIFEST	Sun May  3 07:44:36 2026	(r1933760)
@@ -586,6 +586,7 @@ t/html_nested_anchors.t
 t/html_obfu.t
 t/html_utf8.t
 t/html_visibility.t
+t/html_whitespace.t
 t/idn_dots.t
 t/if_can.t
 t/if_else.t

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm	Sun May  3 06:12:58 2026	(r1933759)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm	Sun May  3 07:44:36 2026	(r1933760)
@@ -59,7 +59,7 @@ my %elements_text_style = map {; $_ => 1
 # elements that insert whitespace
 my %elements_whitespace = map {; $_ => 1 }
   qw( br div li th td dt dd p hr blockquote pre embed listing plaintext xmp title 
-    h1 h2 h3 h4 h5 h6 ),
+    h1 h2 h3 h4 h5 h6 tr),
 ;
 
 # elements that push URIs
@@ -218,22 +218,39 @@ sub get_rendered_text {
   my $self = shift;
   my %options = @_;
 
-  return join('', @{ $self->{text} }) unless %options;
-
-  my $mask;
-  while (my ($k, $v) = each %options) {
-    next if !defined $self->{"text_$k"};
-    if (!defined $mask) {
-      $mask |= $v ? $self->{"text_$k"} : ~ $self->{"text_$k"};
-    }
-    else {
-      $mask &= $v ? $self->{"text_$k"} : ~ $self->{"text_$k"};
+  my $text = '';
+  if (%options) {
+    my $mask;
+    while (my ($k, $v) = each %options) {
+      next if !defined $self->{"text_$k"};
+      if (!defined $mask) {
+        $mask |= $v ? $self->{"text_$k"} : ~ $self->{"text_$k"};
+      }
+      else {
+        $mask &= $v ? $self->{"text_$k"} : ~ $self->{"text_$k"};
+      }
     }
+
+    my $i = 0;
+    for (@{ $self->{text} }) { $text .= $_ if vec($mask, $i++, 1); }
+  } else {
+    $text = join('', @{$self->{text}});
   }
 
-  my $text = '';
-  my $i = 0;
-  for (@{ $self->{text} }) { $text .= $_ if vec($mask, $i++, 1); }
+  $text =~ s/^[ \x{00}]+|[ \x{00}]+$//g;       # Remove spaces and nulls from beginning and end of string
+  $text =~ s/ +/ /g;                           # Collapse spaces
+  $text =~ s/ ?(\x{00} ?)+/\n\n/g;             # Collapse nulls (include neighboring spaces)
+
+  # Convert non-breaking spaces to spaces
+  if ( $self->{SA_character_semantics_input} ) {
+    # Unicode NBSP
+    $text =~ s/\xa0/ /g;
+  } else {
+    # UTF-8 NBSP
+    $text =~ s/\xc2\xa0/ /g;
+  }
+  $text =~ s/ ?\n ?/\n/g;                      # Remove spaces around newlines
+  $text =~ s/^ +| +$//g;                       # Remove spaces from beginning and end of string
   return $text;
 }
 
@@ -257,11 +274,14 @@ sub parse {
   # NOTE: HTML::Parser can cope with: <?xml pis>, <? with space>, so we
   # don't need to fix them here.
 
-  # # (outdated claim) HTML::Parser converts &nbsp; into a question mark ("?")
-  # # for some reason, so convert them to spaces.  Confirmed in 3.31, at least.
-  # ... Actually it doesn't, it is correctly converted into Unicode NBSP,
-  # nevertheless it does not hurt to treat it as a space.
-  $text =~ s/&nbsp;/ /g;
+  # # # (outdated claim) HTML::Parser converts &nbsp; into a question mark ("?")
+  # # # for some reason, so convert them to spaces.  Confirmed in 3.31, at least.
+  # # ... Actually it doesn't, it is correctly converted into Unicode NBSP,
+  # # nevertheless it does not hurt to treat it as a space.
+  # Actually it does hurt. Because <p> </p> is not the same as <p>&nbsp;</p>
+  # when calculating vertical whitespace. So don't do this here. We'll do it
+  # at the very end in get_rendered_text.
+  # $text =~ s/&nbsp;/ /g;
 
   # bug 4695: we want "<br/>" to be treated the same as "<br>", and
   # the HTML::Parser API won't do it for us
@@ -347,6 +367,11 @@ sub html_tag {
       pop(@{ $self->{anchor_refs} }) if $tag eq "a";
       $self->{closed_html} = 1 if $tag eq "html";
       $self->{closed_body} = 1 if $tag eq "body";
+      # If we're closing a block element && previous tag was a <br>, convert it to a non-breaking space
+      # This is a hackish way to prevent adding vertical whitespace while making sure the element is non-empty
+      if (exists $elements_whitespace{$tag} && defined(my $br = $self->{br})) {
+        $self->{text}->[$br] = $self->{SA_character_semantics_input} ? "\x{a0}" : "\xc2\xa0";
+      }
     }
   }
 }
@@ -355,14 +380,15 @@ sub html_whitespace {
   my ($self, $tag) = @_;
 
   # ordered by frequency of tag groups, note: whitespace is always "visible"
-  if ($tag eq "br" || $tag eq "div") {
+  if ($tag eq "br") {
     $self->display_text("\n", whitespace => 1);
+    $self->{br} = $#{ $self->{text} };
   }
   elsif ($tag =~ /^(?:li|t[hd]|d[td]|embed|h\d)$/) {
     $self->display_text(" ", whitespace => 1);
   }
-  elsif ($tag =~ /^(?:p|hr|blockquote|pre|listing|plaintext|xmp|title)$/) {
-    $self->display_text("\n\n", whitespace => 1);
+  elsif ($tag =~ /^(?:div|p|hr|blockquote|pre|listing|plaintext|xmp|title|tr)$/) {
+    $self->display_text("\x{00}", whitespace => 1);
   }
 }
 
@@ -954,30 +980,11 @@ sub display_text {
     $display{invisible} = 0;
   }
 
-  if ($display{whitespace}) {
-    # trim trailing whitespace from previous element if it was not whitespace
-    # and it was not invisible
-    if (@{ $self->{text} } &&
-	(!defined $self->{text_whitespace} ||
-	 !vec($self->{text_whitespace}, $#{$self->{text}}, 1)) &&
-	(!defined $self->{text_invisible} ||
-	 !vec($self->{text_invisible}, $#{$self->{text}}, 1)))
-    {
-      $self->{text}->[-1] =~ s/ $//;
-    }
-  }
-  else {
-    # NBSP:  UTF-8: C2 A0, ISO-8859-*: A0
-    $text =~ s/[ \t\n\r\f\x0b]+|\xc2\xa0/ /gs;
-    # trim leading whitespace if previous element was whitespace 
-    # and current element is not invisible
-    if (@{ $self->{text} } && !$display{invisible} &&
-	defined $self->{text_whitespace} &&
-	vec($self->{text_whitespace}, $#{$self->{text}}, 1))
-    {
-      $text =~ s/^ //;
-    }
+  unless ($display{whitespace}) {
+    $text =~ s/[ \t\n\r\f\x0b]+/ /gs;
+    $self->{br} = undef unless $text eq ' ';
   }
+
   push @{ $self->{text} }, $text;
   while (my ($k, $v) = each %display) {
     my $textvar = "text_".$k;

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm	Sun May  3 06:12:58 2026	(r1933759)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm	Sun May  3 07:44:36 2026	(r1933760)
@@ -828,7 +828,8 @@ sub rendered {
           $space, length $self->{rendered},
           $character_semantics ? '' : ', octets!?');
     } else {
-      $space = $self->{rendered} =~ tr/ \t\n\r\x0b//;
+      my $str = $self->{rendered};
+      $space = $str =~ tr/ \t\n\r\x0b//;
       dbg("message: spaces (octets) in HTML: %d out of %d%s",
           $space, length $self->{rendered},
           $character_semantics ? ', chars!?' : '');

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyEval.pm
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyEval.pm	Sun May  3 06:12:58 2026	(r1933759)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyEval.pm	Sun May  3 07:44:36 2026	(r1933760)
@@ -50,6 +50,8 @@ sub new {
   $self->register_eval_rule("plaintext_sig_length", $Mail::SpamAssassin::Conf::TYPE_BODY_EVALS);
   $self->register_eval_rule("plaintext_body_sig_ratio", $Mail::SpamAssassin::Conf::TYPE_BODY_EVALS);
 
+  $self->register_eval_rule("vertical_whitespace", $Mail::SpamAssassin::Conf::TYPE_BODY_EVALS);
+
   return $self;
 }
 
@@ -381,6 +383,76 @@ sub _plaintext_body_sig_ratio {
   return 1;
 }
 
+# Count the maximum run of consecutive blank lines in the message's primary
+# visible text part.
+#
+# Argument:
+#   $min - minimum run length (inclusive) required for the rule to fire
+#
+# Returns 1 if the longest run of blank lines is >= $min, else 0.
+#
+# Example rule:
+#   body  __VERTICAL_WHITESPACE_10  eval:vertical_whitespace('10')
+#
+sub vertical_whitespace {
+  my ($self, $pms, undef, $min) = @_;
+
+  unless (exists($pms->{vertical_whitespace})) {
+    my $vertical_whitespace = 0;
+
+    # Walk the top-level body to find the first eligible text part, mimicking
+    # what an MUA would display. Skip named parts (attachments) and do not
+    # descend into non-multipart containers like message/rfc822.
+    my $preferred_alt = $pms->{conf}->{multipart_alternative_preferred_part};
+    my $part;
+    my @queue = ($pms->{msg});
+    while (my $p = shift @queue) {
+      # Named parts are attachments regardless of type
+      next if defined $p->{name};
+
+      my $type = $p->{'type'} || '';
+      if ($type eq 'text/plain' || $type eq 'text/html') {
+        $part = $p;
+        last;
+      }
+      if ($type eq 'multipart/alternative') {
+        if ($preferred_alt) {
+          my $pref = Mail::SpamAssassin::Message::_find_part_by_type(
+            $p, $preferred_alt);
+          if ($pref) { $part = $pref; last; }
+        }
+        unshift @queue, @{$p->{'body_parts'} || []};
+        next;
+      }
+      if (index($type, 'multipart/') == 0) {
+        unshift @queue, @{$p->{'body_parts'} || []};
+        next;
+      }
+      # any other type: skip, do not descend
+    }
+
+    if ($part) {
+      # Get the rendered text of the part (visible only)
+      my (undef, $text) = $part->visible_rendered();
+      # Count the max number of consecutive blank lines
+      my $counter = 0;
+      for (split(/^/m, $text)) {
+        if (/^\s*$/) {
+          $counter++;
+        } else {
+          $vertical_whitespace = $counter if $counter > $vertical_whitespace;
+          $counter = 0;
+        }
+        # print STDERR $counter.": \"$_\"\n";
+      }
+      $vertical_whitespace = $counter if $counter > $vertical_whitespace;
+    }
+    $pms->{vertical_whitespace} = $vertical_whitespace;
+    dbg("eval: vertical_whitespace: $vertical_whitespace");
+  }
+
+  return $pms->{vertical_whitespace} >= $min;
+}
 
 # ---------------------------------------------------------------------------
 
@@ -390,4 +462,6 @@ sub has_check_body_length { 1 }
 
 sub has_plaintext_body_sig_ratio { 1 }
 
+sub has_vertical_whitespace { 1 }
+
 1;