process-quarantine-sub.pl errors with Botnet.pm

[jnorell]

Hello,

I put in the SA botnet plugin the other day, and find

process-quarantine-sub.pl giving errors of redefined subroutines (for every sub that is defined within Botnet.pm). I found a solution (or workaround?) with a small change to process-quarantine-sub.pl, but I don't know if it's the right solution (not familiar with SA plugins). I don't know if it would be a problem with how Botnet.pm is written (I didn't see anything obvious comparing it to the sa plugin wiki), or maybe how the environment is setup on this server. In any case, I'll present the change for consideration.

The errors seen are all like:

Subroutine new redefined at /etc/mail/spamassassin/Botnet.pm line 41.

Running process-quarantine-sub.pl --debug I found they were printed after process_spam was run, and it would be from a subsequent call to process_ham. Both of process_spam and _ham create a new (and local) Mail::SpamAssassin instance; I simply changed those functions to accept $sa in the calling arguments, and instantiate it in the main script body prior to calling them.

Also the $sa->init(1) seems unnecessary there, as every function in

SpamAssassin.pm looks to call that itsself (perhaps it's in support of previous SA versions?).

Attached is a patch against process-quarantine-sub.pl from 1.0.0 that

resolves the issue. Perhaps the $sa->init(1) call should be wrapped in a check for older SA versions, I don't know, but it works fine without it for me (SA 3.1.3 on gentoo).

[rjl]

The patch appears to have been lost during our server crash late last year, but I agree with what's being proposed here. In fact, I could have sworn I did exactly that in the last revision of process-quarantine-sub.pl, but apparently I did not.

This is the same issue being reported in #452, incidentally--any third-party plug-in for SpamAssassin that isn't properly designed with concurrency in mind will cause these nuisance "redefinition" warnings to appear.

[mortonda@…]

I think I got it recreated in [1422].

I agree Robert, I sure thought you did this, and lots of other calls have a $sa parameter. Odd.