Bug 5312 - Razor plugin uses forbidden $' operator
Summary: Razor plugin uses forbidden $' operator
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Plugins (show other bugs)
Version: SVN Trunk (Latest Devel Version)
Hardware: Other other
: P5 normal
Target Milestone: 3.2.1
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-26 05:31 UTC by Justin Mason
Modified: 2007-06-07 00:56 UTC (History)
1 user (show)


Attachment Type Modified Status Actions Submitter/CLA Status
patch against razor-agents 2.82 patch None Justin Mason [HasCLA]
Updated patch including Whiplash.pm fix patch None Alex Kiernan [NoCLA]
Updated patch with Whiplash.pm fix changed to be consistent with Preproc fixes patch None Richard Soderberg [NoCLA]
update our docs patch None Justin Mason [HasCLA]
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 Justin Mason 2007-01-26 05:31:33 UTC 
Using the magic of the Devel::SawAmpersand module, I've written a new regression
test which detects the use of $& during scanning.

as 'perldoc perlvar' states:

               The use of this variable anywhere in a program imposes a consider-
               able performance penalty on all regular expression matches.  See
               "BUGS".

so in other words, once one module uses it, every single RE match afterwards
becomes much slower... very bad news for SA, naturally.

And wouldn't you know it, one of our dependency modules does: Razor.  at least,
once the Mail::SA::Plugin::Razor2 plugin is loaded, with my installed version of
razor, Devel::SawAmpersand::sawampersand() returns 1...

unfortunately we can't scope use of $&.
Comment 1 Justin Mason 2007-01-26 05:43:35 UTC 
just confirmed; still an issue with razor 2.82, the current release... I think
it's from the use of $' in these files:

lib/Razor2/Preproc/deBase64.pm
lib/Razor2/Preproc/deQP.pm
lib/Razor2/Signature/Whiplash.pm

Patch to follow, which fixes the bug for me.
Comment 2 Justin Mason 2007-01-26 05:44:06 UTC 
Created attachment 3844 [details]
patch against razor-agents 2.82
Comment 3 Justin Mason 2007-01-26 05:49:22 UTC 
reported to razor guys:

http://sourceforge.net/tracker/index.php?func=detail&aid=1645290&group_id=3978&atid=103978

I don't think there's much we can do about this in our own code for 3.2.0; maybe
warn in the INSTALL notes?
Comment 4 Theo Van Dinter 2007-01-26 08:42:59 UTC 
(In reply to comment #3)
> I don't think there's much we can do about this in our own code for 3.2.0; maybe
> warn in the INSTALL notes?

Yeah, definitely nothing for 3.2.  IMO, there's two options overall:

- change the Razor2 plugin to drop as much of the razor-agents code as possible.
 this would, for instance, mean the message doesn't need to be reparsed, and we
could potentially drop all the commandline script code, etc.

- no offense to them, but the razor2 perl code could use a bunch of cleanup and
refactoring.  for example, if you look at razor-admin, there's pretty much
nothing in there -- everything, including commandline parsing, etc, is all in
the modules.  so we could work on fixing up their stuff, thereby making it work
better with SA.


I'd rather focus on the first option if possible.
Comment 5 Justin Mason 2007-01-29 14:16:51 UTC 
ok, added some doc:

: jm 453...; svn commit -m "bug 5312: document Razor slowdown bug" INSTALL
Sending        INSTALL
Transmitting file data .
Committed revision 501198.


hopefully the Razor guys can fix this soon... in the meantime, let's leave this
bug open and if anyone wants to hack up an alternate razor-agents, that can be
done for 3.3.0.
Comment 6 Justin Mason 2007-01-29 14:22:14 UTC 
(btw, it's worth noting that simply setting scores to 0 doesn't help; even if
the scores are == 0, if the plugin is loaded and -L is not specified, the
bug will take effect and all regexps will be slowed down.)
Comment 7 Alex Kiernan 2007-04-19 04:22:12 UTC 
Looks like the patch wasn't quite what was intended - the patch to Whiplash.pm 
doesn't apply cleanly; it looks like the .OLD tree wasn't a completely clean 
razor-agents-2.82.
Comment 8 Alex Kiernan 2007-04-19 04:32:48 UTC 
Created attachment 3915 [details]
Updated patch including Whiplash.pm fix
Comment 9 Yizhar Hurwitz 2007-04-23 07:08:58 UTC 
HI.

Sorry for my ignorance, but
How exactly should I apply the patch?
I mean, what is the exact command line(s)?

My setup is:
OS = Fedora Core 6
razor agents 2.8.2
spamassassin 3.1.8
MimeDefang 2.61

Path to Razor agents:
/usr/lib/perl5/vendor_perl/5.8.8/Razor2/

Thanks.
Yizhar Hurwitz
Comment 10 Richard Soderberg 2007-05-08 15:28:06 UTC 
Created attachment 3937 [details]
Updated patch with Whiplash.pm fix changed to be consistent with Preproc fixes

(In reply to comment #8)
> Created an attachment (id=3915) [edit]
> Updated patch including Whiplash.pm fix

Added one character to the Whiplash.pm patch to match the Preproc patch.

-	 last unless $text =~ m"http://(.*)";
+	 last unless $text =~ m"http://(.*)$";
Comment 11 Richard Soderberg 2007-05-08 15:32:59 UTC 
(In reply to comment #3)
> reported to razor guys:
> 
>
http://sourceforge.net/tracker/index.php?func=detail&aid=1645290&group_id=3978&atid=103978

SF bug updated with links back to this bug and closed as fixed in 2.83 (upcoming).
Comment 12 Richard Soderberg 2007-05-08 16:39:42 UTC 
(In reply to comment #11)
> SF bug updated with links back to this bug and closed asfixed in 2.83 (upcoming).

2.83 published to SourceForge.

http://sourceforge.net/project/showfiles.php?group_id=3978&package_id=30227&release_id=507010
Comment 13 Justin Mason 2007-05-09 03:28:12 UTC 
Created attachment 3938 [details]
update our docs

thanks Richard.  this patch updates our docs to note the fix...
devs, please review for 3.2.1...
Comment 14 Justin Mason 2007-05-09 03:29:28 UTC 
actually, doc fixes count as trivial ;)  applied to trunk:

: jm 183...; svn commit -m "bug 5312: doc fix: note that Razor-agents 2.83
includes the fix for the perl-interpreter slowdown bug" INSTALL
Sending        INSTALL
Transmitting file data .
Committed revision 536481.

and 3.2.0:

: jm 159...; svn commit -m "bug 5312: doc fix: note that Razor-agents 2.83
includes the fix for the perl-interpreter slowdown bug" INSTALL
Sending        INSTALL
Transmitting file data .
Committed revision 536482.
Comment 15 Mark Martinec 2007-06-06 08:29:30 UTC 
Note that NetAddr::IP is infested with $& and $` thoroughly,
including its autoloaded modules.

Unfortunately Mail::SPF uses NetAddr::IP,
and SPF plugin for SA is loaded through init.pre by default.

$ perl -MNetAddr::IP -MDevel::SawAmpersand \
  -e 'printf("SawA: %d\n",Devel::SawAmpersand::sawampersand)'
SawA: 1

$ perl -MMail::SPF -MDevel::SawAmpersand \
  -e 'printf("SawA: %d\n",Devel::SawAmpersand::sawampersand)'
SawA: 1
Comment 16 Daryl C. W. O'Shea 2007-06-06 15:00:53 UTC 
The NetAddr::IP guys are in the process of resolving this.
Comment 17 Daryl C. W. O'Shea 2007-06-06 20:16:32 UTC 
NetAddr::IP v4.007 removes everything that would cause PL_sawampersand to be set
to true.

We may want to note this somewhere in the POD or on the wiki.
Comment 18 Daryl C. W. O'Shea 2007-06-07 00:56:06 UTC 
[dos@cyan trunk]$ svn ci -m "bug 5312: add note to INSTALL doc that NetAddr::IP
versions before 4.007 trigger PL_sawampersand slow down"
Sending        INSTALL
Transmitting file data .
Committed revision 545093.
[dos@cyan trunk]$ cd ../3.2
[dos@cyan 3.2]$ svn ci -m "bug 5312: add note to INSTALL doc that NetAddr::IP
versions before 4.007 trigger PL_sawampersand slow down"
Sending        INSTALL
Transmitting file data .
Committed revision 545094.
[dos@cyan 3.2]$