[Bug 2787] [review] Type of received line misunderstood

[bu...@bugzilla.spamassassin.org]

http://bugzilla.spamassassin.org/show_bug.cgi?id=2787

niels@fabel.dk changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |
            Version|2.60                        |2.61



------- Additional Comments From niels@fabel.dk  2004-01-01 19:23 -------
I am sorry to report that I have problems with SA 2.61 and received lines from 
the same mail server:

Received: from dyn-81-166-39-132.ppp.tiscali.fr (81.166.39.132) by cpmail.dk.
tiscali.com (6.7.018)
        id 3FE6899B004FE7A4; Thu, 1 Jan 2004 05:28:49 +0100

The line is not caught by the replacement patch's regex:
  /^from (\S+) \((${IP_ADDRESS})\) by (\S+) with /
because there is no "with" in the line.

Also my own patch would not have caught it either because there is no "for 
user@example.com" in this instance of the header.

So I will propose a path similar to my first one, that will make a special rule 
for that kind of mail server.


Comments on Received.pm in general:

The Content Technologies rule is very broad and doesn't even assign $ip. When 
control reaches the enough label the parser will just return because there is no 
$ip defined - without any debugging information.

Also the tests for defined $ip above the call to extract_ipv4_addr_from_string 
are bogus, the $ip variable is initialized with
 my $ip = '';
So it is always defined below that, the tests should be
 $ip eq ''
And you might want to add tests just after the enough label, maybe something 
like:

  if ($ip eq '') {
    dbg ("received-header: known format, unknown IP address: $_");
  }

At least that would have made it easier for me to track down the bug.

Another nice thing would have been easy identification of the rule that matched 
the header, that might be done simply by a dbg call for every rule.

Also some regression testing for header lines would be nice, it's pretty hard to 
tell if you are ruining some other rule when you add or modify a rule.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.