https://issues.apache.org/SpamAssassin/show_bug.cgi?id=5931
Daryl C. W. O'Shea <[EMAIL PROTECTED]> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |[EMAIL PROTECTED]
--- Comment #4 from Daryl C. W. O'Shea <[EMAIL PROTECTED]> 2008-10-16 15:56:00
PST ---
(In reply to comment #1)
> agreed, this is a waste of time. fixed in trunk:
>
> : jm 237...; svn commit -m "bug 5931: remove needless O(N^2) de-duplication
> step from trusted_networks" lib/Mail/SpamAssassin/NetSet.pm
FWIW, there are two reasons why I wrote it like this (fully aware of the O(N^2)
step, but not expecting someone to load 10,000 ranges):
- The major one: It provides debugging for whether or not an excluded net is
really going to be excluded... without this an apparently valid config will not
produce the expected results (ie, "trusted_networks 0/0 !131.107/16" will lint
fine *and* trust mail from Microsoft"). Knowing how confused people are with
the whole Apache ACL mess I wanted to make this config option perfectly clear
as to whether or not you were getting what you expected.
- A minor one: I'd rather save time per message (albeit probably just a little)
than on start-up. Again I never expected 10,000 ranges to be used.
I guess I didn't do a very good job at the regression tests for this... it
still passes... it only warns you that it's expecting an error (that now never
happens), it doesn't fail when it doesn't see the error.
Also... the same 0(N^2) slowdown exists as soon as you start loading both a
trusted_networks and internal_networks config (internal networks are checked to
make sure they're in trusted_networks). This is another lint thing that I feel
is important since so many people are confused about the whole trusted/internal
networks thing. internal_networks must be in trusted_networks.
Anyway, as such, I'm -0.9 on the change. I would much prefer that the existing
check be performed until the amount of ranges inputted makes it impractical to
do so (say when 200 ranges are added it skips the O(N^2) bit). Note that I am
not for running this only with --lint enabled... it's useful when a message is
run through with -D. I can't emphasize how important I think the lint checks
are for this part of the config.
(In reply to comment #3)
> Henrik Krohns wrote:
> Just in case this pops up some time in the future: having separate tables
> for includes and excludes is a really bad idea. We do not want to repeat
> an apache http access-list mistake (a need for Order directive, inability to
> express subranges within subranges)
This is specifically the reason I wrote it like this. The Apache ACL is a mess
that, although really is straight-forward (just not very flexible), is
confusing as hell to your typical user.
> The patch is fine, an occasional duplicate does no harm.
Obviously I disagree. :) I believe that the resulting degradation of the lint
capability is detrimental.
--
Configure bugmail:
https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
