Confirmed, moved to 2.60 milestone.

I was able to work around the problem by lowering the maximum stars from 100
to 71 or lower.  Changing both 100s in the STARS code to 71 fixes the problem
for very high scoring spam.  72 does not.  My test message has a score of 97.

I believe the source of the bug is due to the wrapping code so the problem
could affect other headers.

For a workaround, I would suggest a maximum value of 65 since that would
result in a maximum line length of 79 which is the advisable maximum.
(I don't know why a value of 71 works, though, since that does result in a line
length longer than 79, I'm just suggesting 65 for aesthetic reasons if we can't
use 100.)

For a real fix, someone will have to do more work.

Subject: Re:  X-Spam-Level does not show *** on very high scores

I would be happy if it showed 65 stars for any score over 65.

Patch to fix this will be attaches, for review.

Now a large number of stars will appear as follows (lets see how bugzilla
mangles this....)

X-Spam-Level:(space)***********************************************************************(newline)
(tab)*****************************

There's also the possibility of forcing it to overflow, but I dont think this is
desired given that the user has asked for them to be folded.

Created attachment 1050 [details]
Patch to properly fold long headers

Subject: Re: [SAdev]  X-Spam-Level does not show *** on very high scores

On Tue, Jun 17, 2003 at 06:10:11PM -0700, bugzilla-daemon@hughes-family.org wrote:
> Now a large number of stars will appear as follows (lets see how bugzilla
> mangles this....)
> 
> X-Spam-Level:(space)***********************************************************************(newline)
> (tab)*****************************
> 
> There's also the possibility of forcing it to overflow, but I dont think this is
> desired given that the user has asked for them to be folded.

Perhaps instead of limiting the # of stars to 100, we should choose
whatever number makes it fit on a single line?

-0: instead of splitting on '!!', use something that can't appear in
headers like a null.  "\000" (must use double quotes), and maybe have
it complain (not particularly important unless you're paranoid) if for
some reason a null was already present in the header data.

Also, let's limit the stars to 65 characters.  If it's broken after 65, it's
not going to match anything usefully anyway.

Subject: Re: [SAdev]  X-Spam-Level does not show *** on very high scores

> -0: instead of splitting on '!!', use something that can't appear in
> headers like a null.  "\000" (must use double quotes), and maybe have
> it complain (not particularly important unless you're paranoid) if for
> some reason a null was already present in the header data.

It only splits on the first one, so "!!" is just as valid as any
other, providing "!!" doesn't occur in the header name, which it
can't. I also haven't investigated how Text::wrap would treat a \000.

> Also, let's limit the stars to 65 characters.  If it's broken after 65, it's
> not going to match anything usefully anyway.

65 is somewhat arbitrary, and we still run into trouble, if people
want stars in a header called
X-Spam-Number-Of-Stars-Representing-Score:

So maybe we should pick 50 or 60 or something.

Subject: Re: [SAdev]  X-Spam-Level does not show *** on very high scores

bugzilla-daemon@hughes-family.org writes:

> It only splits on the first one, so "!!" is just as valid as any
> other, providing "!!" doesn't occur in the header name, which it
> can't. I also haven't investigated how Text::wrap would treat a \000.

Okay.
 
> 65 is somewhat arbitrary, and we still run into trouble, if people
> want stars in a header called

Ah, okay.  We could figure it out to come out to 78, but let's just use
50 and keep it simple.

Subject: Re:  X-Spam-Level does not show *** on very high scores

The stars are useful to test the message in Exim for a specific number 
of stars (or greater) to do routing. I use two level spam tagging - 
spam-high and spam-low at 15 and 5 points. Since Exim isn't great for 
processing numbers I need to count stars to see if something is over 15 
or 20 or something like that.

I doubt that I will ever need to do a conditional on over 50 stars. So I 
think that if you capped the number of stars at anything over 50 that it 
would do the job for most people who use stars.

-0: I'll ditto quinlan's comments.

Created attachment 1053 [details]
new proposed patch, same as Duncan's, but adds limit of 50

-0: !! is still in there, and there should be docs about the 50 char limit.

-0: ok, I misread your statement about !!, which does make sense.  so now I
would just like to have some docs about 50 being the limit.

+ _STARS(*)_        one * for each score point (use any character)

I would probably just change this to specify the max length.  Something like:

+ _STARS(*)_        one * (use any character) for each score point (up to 50)


with that change, +1 from me. :)

Created attachment 1056 [details]
new patch, burning my time for something not even listed in the bug.  :-(

just because the doc isn't in the initial bug, it's the right thing to do.

+1 for 1056, btw.

I'm going to consider this 3 +1s now since the patch is essentially duncf's (+1
duncf) with the doc modification (+1 quinlan), and I've +1ed it.

committed.

> just because the doc isn't in the initial bug, it's the right thing to do.

I was accused in the past by a user of holding a bug hostage because I wouldn't
accept his patch unless he fixed an additional orthogonal problem.  In that
instance, he was right.  I've tried to avoid doing that since then.  I don't
want to see code review (which I believe is leading to better stability and
helping us be closer than otherwise to our schedule) head down that path.

Subject: Re: [SAdev]  X-Spam-Level does not show *** on very high scores

On Thu, Jun 19, 2003 at 12:25:58PM -0700, bugzilla-daemon@hughes-family.org wrote:
> I was accused in the past by a user of holding a bug hostage because I wouldn't
> accept his patch unless he fixed an additional orthogonal problem.  In that
> instance, he was right.  I've tried to avoid doing that since then.  I don't
> want to see code review (which I believe is leading to better stability and
> helping us be closer than otherwise to our schedule) head down that path.

Sure.  But documenting the code we're about to put in place isn't exactly
orthogonal, even if the previous code didn't have similar documentation.

and it was a -0 instead of a -1, so I wasn't holding anything hostage.