Bug 7164 - TxRep undef warnings
Summary: TxRep undef warnings
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Plugins (show other bugs)
Version: SVN Trunk (Latest Devel Version)
Hardware: All All
: P2 normal
Target Milestone: 3.4.2
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
: 7241 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-04-06 13:05 UTC by Joe Quinn
Modified: 2018-08-24 01:46 UTC (History)
12 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status

Note You need to log in before you can comment on or make changes to this bug.
Description Joe Quinn 2015-04-06 13:05:41 UTC
New ticket for TxRep undef warnings from bug 7021.

Wed Aug  6 11:24:44 2014 [54404] warn: Use of uninitialized value $msgscore in addition (+) at /usr/local/lib/perl5/site_perl/5.12.4/Mail/SpamAssassin/Plugin/TxRep.pm line 1414.
Wed Aug  6 11:24:44 2014 [54404] warn: Use of uninitialized value $msgscore in subtraction (-) at /usr/local/lib/perl5/site_perl/5.12.4/Mail/SpamAssassin/Plugin/TxRep.pm line 1414.

Per Benny Pedersen, perhaps when msgscore is undef, it could be set to 0 before continuing? I've been unable to reproduce these warnings, so it's possible there's some deeper problem.
Comment 1 Simon Matter 2015-05-06 09:42:13 UTC
I see the same behaviour on RHEL6 with spampd and current SA-3.4.1:

May  6 11:39:38 ns1 spampd[13321]: Use of uninitialized value $msgscore in addition (+) at /usr/share/perl5/vendor_perl/Mail/SpamAssassin/Plugin/TxRep.pm line 1415.#012
May  6 11:39:38 ns1 spampd[13321]: Use of uninitialized value $msgscore in subtraction (-) at /usr/share/perl5/vendor_perl/Mail/SpamAssassin/Plugin/TxRep.pm line 1415.#012

TxRep seems to work but I'd like to see those messages go away.
Comment 2 Simon Matter 2015-05-06 11:19:28 UTC
###########################################################################
sub check_reputation {
###########################################################################
  my ($self, $storage, $pms, $key, $id, $ip, $signedby, $msgscore) = @_;

  my $delta  = 0;
  my $weight = ($key eq 'MSG_ID')? 1 : eval('$pms->{main}->{conf}->{txrep_weight_'.lc($key).'}');

  if (defined $weight && $weight) {
    my $meanrep;
    my $timer = $self->{main}->time_method('check_txrep_'.lc($key));

    if (defined $storage) {
        $self->{checker} = $self->{$storage};
    }
    my $found  = $self->get_sender($id, $ip, $signedby);
    my $tag_id = (defined $storage)? uc($key.'_'.substr($storage,0,1)) : uc($key);
    if (defined $found && $self->count()) {
        $meanrep = $self->total() / $self->count();
    }
    if ($self->{learning} && defined $msgscore) {
        if (defined $meanrep) {
            # $msgscore<=>0 gives the sign of $msgscore
            $msgscore += ($msgscore<=>0) * abs($meanrep);
        }
        dbg("TxRep: reputation: %s, count: %d, learning: %s, $tag_id: %s",
            defined $meanrep? sprintf("%.3f",$meanrep) : 'none',
            $self->count()      || 0,
            $self->{learning}   || '',
            $id                 || 'none'
        );
    } else {
        $self->{totalweight} += $weight;
        if ($key eq 'MSG_ID' && $self->count() > 0) {
            $delta = $self->total() / $self->count();
            $pms->set_tag('TXREP'.$tag_id,              sprintf("%2.1f",$delta));
        } elsif (defined $self->total()) {
            $delta = ($self->total() + $msgscore) / (1 + $self->count()) - $msgscore;

Here the error happens. Sorry for not really knowing perl but, from what I see, if check_reputation is called with undefined msgscore, it's possible to end up in the line above without defining it before. Isn't that what produces the error?
Comment 3 Kevin A. McGrail 2015-05-06 15:00:35 UTC
More testing shows that the issue only occurs with txrep_track_messages turned on for txrep.

And it has to be a message that generates a new msg_id from the bayes.pm get_msgid call.  

I'm tracking the issue through the code and it appears to be from line 1269 (line #'s are likely off from my additional code for debug) which is this:  my $msg_rep = $self->check_reputations($pms, 'MSG_ID', $msg_id, undef, $date, undef);

which triggers $delta = $self->check_reputation(undef,@_); in check_reputations.

Setting msgscore in line 1269 or setting it to 0 at the beginning of the block doesn't seem correct as it appears msgscore is supposed to be undef but the logic is missing for this case so I've added this block:

         } elsif (defined $self->total()) {
-            $delta = ($self->total() + $msgscore) / (1 + $self->count()) - $msgscore;
+            #Bug 7164 - $msgscore undefined
+            if (defined $msgscore) {
+              $delta = ($self->total() + $msgscore) / (1 + $self->count()) - $msgscore;
+            } else {
+              $delta = ($self->total()) / (1 + $self->count());
+            }

Will also ask Ivo to weigh in on the logic change but it's committed to 3.4 and trunk and considered resolved.

svn commit -m 'bug 7164 - 3.4 commit - small clean up on whitespace/logic for clarity and added logic for msgscore undefined in Txrep.pm'
Sending        lib/Mail/SpamAssassin/Plugin/TxRep.pm
Transmitting file data .
Committed revision 1678016.


svn commit -m 'bug 7164 - 3.4 commit - small clean up on whitespace/logic for clarity and added logic for msgscore undefined in TxRep.pm'
Sending        lib/Mail/SpamAssassin/Plugin/TxRep.pm
Transmitting file data .
Committed revision 1678017.

NOTE: my comment on this commit was wrong.  Should have said trunk commit.

Regards,
KAM
Comment 4 Charles Sprickman 2015-06-27 05:22:11 UTC
I know this was recently closed, but I was seeing the same error and applied the diff attached to this bug and while I no longer see a warning, the data in the txrep db table is looking a little suspect.

Example:

mysql> select * from txrep;
+--------------------------+-------------------------------------------------------+---------+-------+----------+-----------------------+---------------------+
| username                 | email                                                 | ip      | count | totscore | signedby              | last_hit            |
+--------------------------+-------------------------------------------------------+---------+-------+----------+-----------------------+---------------------+
| foo@bway.net          | 219.95.24.109                                         | none    |     1 |   23.354 |                       | 2015-06-27 01:10:19 |
| foo@bway.net          | caylajammalamadaka@comstock.com                       | 219.95  |     1 |   23.354 |                       | 2015-06-27 01:10:19 |
| foo@bway.net          | caylajammalamadaka@comstock.com                       | none    |     1 |   23.354 |                       | 2015-06-27 01:10:19 |
| foo@bway.net          | comstock.com                                          | 219.95  |     1 |   23.354 |                       | 2015-06-27 01:10:19 |
| foo@bway.net          | d8545c1cef79c20fb26a58bcb2cdeb0eaf101081@sa_generated | none    |     1 |   23.354 | 1435381816            | 2015-06-27 01:10:19 |
| foo@bway.net          | spamexperts.com                                       | none    |     1 |   23.354 | helo                  | 2015-06-27 01:10:19 |

"username", "count", "totscore" seem legit, the rest look a bit odd.  My schema came directly from the 3.4.1 distribution.
Comment 5 Kevin A. McGrail 2015-06-29 19:37:41 UTC
What do you think the data should look like if this is suspect?  Can you reverse the patch and capture data as well?
Comment 6 Joe Quinn 2015-09-04 16:45:43 UTC
*** Bug 7241 has been marked as a duplicate of this bug. ***
Comment 7 Joe Quinn 2015-09-04 16:46:22 UTC
Some further info moved from bug 7241

baba 2015-09-03 23:08:48 UTC

When learning a message i got these two warnings:

Use of uninitialized value in numeric gt (>) at /usr/local/share/perl/5.10.1/Mail/SpamAssassin/Plugin/TxRep.pm line 1411.
Use of uninitialized value in string eq at /usr/local/share/perl/5.10.1/Mail/SpamAssassin/Plugin/TxRep.pm line 1443.

SpamAssassin Server version 3.4.1
  running on Perl 5.10.1
  with SSL support (IO::Socket::SSL 1.33)
  with zlib support (Compress::Zlib 2.064)


Also i got this warning (only the warning on line 1411) on every incoming mail:

Sep  4 17:14:28.858 [22310] info: spamd: processing message <55E9B525.7010600@test.de> for qmaild:2020
Sep  4 17:14:30.014 [22310] warn: Use of uninitialized value in numeric gt (>) at /usr/local/share/perl/5.10.1/Mail/SpamAssassin/Plugin/TxRep.pm line 1411.
Sep  4 17:14:30.016 [22310] warn: Use of uninitialized value in numeric gt (>) at /usr/local/share/perl/5.10.1/Mail/SpamAssassin/Plugin/TxRep.pm line 1411.
Sep  4 17:14:30.023 [22310] info: spamd: clean message (-1.0/5.0) for qmaild:2020 in 1.2 seconds, 545 bytes.
Sep  4 17:14:30.024 [22310] info: spamd: result: . -1 - ALL_TRUSTED scantime=1.2,size=545,user=qmaild,uid=2020,required_score=5.0,rhost=127.0.0.1,raddr=127.0.0.1,rport=36469,mid=<55E9B525.7010600@test.de>,autolearn=no autolearn_force=no
Comment 8 David Birnbaum 2016-05-17 17:41:28 UTC
Howdy.  Not sure if this bug is fixed or not, but the problem is actually around line 1897 in TxRep.pm, I think:

        my $msg_rep = $self->check_reputations($pms, 'MSG_ID', $msg_id, undef, $date, undef);

Note that the last field is passed into check_reputation() as $msgscore, so it will show up as undef.  I'm assuming this should be passed in as 0 instead, and the error goes away (perl should probably treat it as 0).
Comment 9 David Birnbaum 2016-05-22 01:40:57 UTC
Actually, that fix screws things up because it's expecting to return undef.  My ugly patch would instead be to just check to remove the warning:

1419,1423c1415,1416
< 	    if ( defined( $msgscore )) {
< 		$delta = ($self->total() + $msgscore) / (1 + $self->count()) - $msgscore;
< 	    } else {
< 		$delta = ($self->total()) / (1 + $self->count());
< 	    }
---
>             $delta = ($self->total() + $msgscore) / (1 + $self->count()) - $msgscore;
>
Comment 10 steven_nikkel 2016-06-03 19:07:52 UTC
I get this for every message I run through sa-learn v3.4.1



Use of uninitialized value $msgscore in addition (+) at /usr/local/lib/perl5/site_perl/Mail/SpamAssassin/Plugin/TxRep.pm line 1415.
Use of uninitialized value $msgscore in subtraction (-) at /usr/local/lib/perl5/site_perl/Mail/SpamAssassin/Plugin/TxRep.pm line 1415.
Comment 11 JFARJONA 2016-07-28 18:53:29 UTC
Hello:

Please find here:

http://www.filetolink.com/b03b8e652a

a set of files that will create the warnings. 

On my installation:

# sa-learn -V
SpamAssassin version 3.4.1
# sa-learn --spam [0-9]*.

Use of uninitialized value $msgscore in addition (+) at /usr/local/share/perl5/Mail/SpamAssassin/Plugin/TxRep.pm line 1416.
Use of uninitialized value $msgscore in subtraction (-) at /usr/local/share/perl5/Mail/SpamAssassin/Plugin/TxRep.pm line 1416.
Learned tokens from 0 message(s) (809 message(s) examined)

Thanks.

Juan
Comment 12 Max Kostikov 2016-08-28 21:35:06 UTC
Seems this bug remains in latest Spamassassin distribution.
> oot@beta:~ # spamassassin -V
> SpamAssassin version 3.4.1
>  running on Perl version 5.24.1
But commit 1678017 fixes this. After applying annoying message gone.
Comment 13 Charles Sprickman 2016-11-23 06:25:12 UTC
Anyone have a patch until a new release comes out?
Comment 15 gessel@blackrosetech.com 2017-12-19 16:56:15 UTC
I tried 1720440 (https://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/TxRep.pm?revision=1720440&view=markup) which solves the undef value error in this bug, as noted in line 1420: 

#    #Bug 7164, trying to find out reason for these: _WARN: Use of uninitialized value $msgscore in addition (+) at /usr/share/perl5/vendor_perl/Mail/SpamAssassin/Plugin/TxRep.pm line 1415.

However, another mod, at line 1633 causes a 61x slowdown.  

  # disabled per bug 7191
  # return 1 unless (!defined $self->{default_storage});

Which is the fix for this bug: https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7191

I found this slowdown made sa-learn unusable.  reverting this patch resulted in a net 2.1x slowdown over the default version in FreeBSD but without the errors.  I am not seeing any misattribution errors as described in #7191 and there are other fixes relative to the version that ships with FreeBSD.  

I've attached a diff file to 7191  It is trivial:

--- TxRep.pm.1720440    2017-12-19 07:53:56.837268934 -0800
+++ TxRep.pm.1720440.mod        2017-12-19 08:38:09.737087765 -0800
@@ -1630,7 +1630,8 @@
   my $self = shift;
 
   # disabled per bug 7191
-  # return 1 unless (!defined $self->{default_storage});
+  # Enabled per bug 7191 comment 18
+  return 1 unless (!defined $self->{default_storage});
 
   my $factory;
   if ($self->{main}->{pers_addr_list_factory}) {
Comment 16 Kevin A. McGrail 2018-08-24 01:46:40 UTC
revert the return commented per comment 18

trunk: Committed revision 1838776.
3.4: Committed revision 1838777.