Bug 3670 - 2.64: Error in PerMsgStatus.pm causes warnings
Summary: 2.64: Error in PerMsgStatus.pm causes warnings
Status: RESOLVED WONTFIX
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamc/spamd (show other bugs)
Version: 2.63
Hardware: All All
: P5 normal
Target Milestone: 3.1.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-08-06 00:04 UTC by Sebastian Hagedorn
Modified: 2004-08-06 07:31 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
Simple patch text/plain None Sebastian Hagedorn [NoCLA]
Simple patch (now in proper Perl) patch None Sebastian Hagedorn [NoCLA]
Show Obsolete (1) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Hagedorn 2004-08-06 00:04:17 UTC 
sub get() is changed in 2.64. I noticed messages like the following:

Use of uninitialized value in length at /usr/local/perl-5.6.1/lib/site_perl/5.6.1/Mail/SpamAssassin/
PerMsgStatus.pm line 1339, <GEN133> line 49

The cause seems pretty obvious. This code is new:

      foreach $hdr ($self->{msg}->get_all_headers()) {
        last if ($length + length($hdr) > MAX_HEADER_LENGTH);

        my($key, $value) = split(/:/, $hdr, 2);
        # limit the length of the pairs we store
        if (length($key) > MAX_HEADER_KEY_LENGTH) {
          $key = substr($key, 0, MAX_HEADER_KEY_LENGTH);
        }
        if (length($value) > MAX_HEADER_VALUE_LENGTH) {
          $value = substr($value, 0, MAX_HEADER_VALUE_LENGTH);
        }
        push(@hdrs, "$key:$value");
        $length += length "$key:$value";
      }

For headers without a right-hand part, $value is undefined ...
Comment 1 Sebastian Hagedorn 2004-08-06 00:28:34 UTC 
Created attachment 2222 [details]
Simple patch

This is the simplest solution to the problem I could think of. There may be
better ways ...
Comment 2 Sebastian Hagedorn 2004-08-06 00:44:10 UTC 
Comment on attachment 2222 [details]
Simple patch

Ouch, too early in the morning ;-)
Comment 3 Sebastian Hagedorn 2004-08-06 00:45:39 UTC 
Created attachment 2223 [details]
Simple patch (now in proper Perl)

This is a simple patch to fix the problem. There may be better ways to do this
...
Comment 4 Sidney Markowitz 2004-08-06 01:44:37 UTC 
The same code is in Bayes.pm

In the 3.0 trunk there is only one copy of this code, in Message.pm. There this
bug does not exist because the code that uses $value is wrapped in

 # If it's not a valid header (aka: not in the form "foo: bar"), skip it.
 if (defined $value) {
   [ ... ]
 }

which seems like a cleaner solution.

The question is whether this is serious enough to require a patch to 2.64 :-(
Comment 5 Sidney Markowitz 2004-08-06 13:04:01 UTC 
The regression tests in 3.0 catch this bug. The ones in 2.64 do not, which is
how this slipped by.

Would some other devs please comment on the serousness of this being in 2.64? If
we are going to patch this it should be as soon as possible before more people
have downloaded the security upgrade. Is this just a matter of having to ignore
a warning whenever a message has a header with no body? Is it worse?
Comment 6 Justin Mason 2004-08-06 13:13:49 UTC 
IMO, if it's just "undefined value" warnings, I don't see an urgent need to put
out a fix.  putting out 3.0.0 would be better.
Comment 7 Theo Van Dinter 2004-08-06 13:23:31 UTC 
Subject: Re:  2.64: Error in PerMsgStatus.pm causes warnings

On Fri, Aug 06, 2004 at 01:04:02PM -0700, bugzilla-daemon@bugzilla.spamassassin.org wrote:
> Would some other devs please comment on the serousness of this being in 2.64? If
> we are going to patch this it should be as soon as possible before more people
> have downloaded the security upgrade. Is this just a matter of having to ignore
> a warning whenever a message has a header with no body? Is it worse?

First: d'oh!

As for the issue, I think that's about the extent of it.  As an FYI,
I've had 2.64 running for several days now, and have 0 of those warnings
in my logfile.

If a number of people are having a problem, we can put out a 2.65,
but I'd really rather just get 3.0 done and released.
Comment 8 Sidney Markowitz 2004-08-06 15:31:44 UTC 
This even more benign than I had thought.

It only happens when a line in the header section has no ':', not when it is a
valid header that is empty.

It shows up in the 3.0 regression test because there is a BSMTP formatted file
in the test spam directory, which means that SpamAssassin is fed an incorrectly
formatted file.

When there is a header line without a ':' the only result is a warning message
in the log file, but processing continues ok.

I'm going to close this as WONTFIX, treating this as a 2.6x bug. If another dev
thinks it should be resolved as fixed because it is fixed in 3.0, feel free to
make the change.