SA Bugzilla – Bug 7480
Unescaped left brace in regex
Last modified: 2018-01-11 17:52:22 UTC
In perl 5.26 I'm getting the following warning when I run sa-learn: Unescaped left brace in regex is deprecated here (and will be fatal in Perl 5.30), passed through in regex; marked by <-- HERE in m/^(.{ <-- HERE ,200}).*$/ at /usr/share/perl5/Mail/SpamAssassin/PerMsgStatus.pm line 921. This is the line that's producing the warning: $str =~ s/^(.{,200}).*$/$1/gs; That is invalid syntax; there needs to be number before the comma inside curly braces. I believe this should work: $str =~ s/^(.{1,200}).*$/$1/gs;
In fact, does that even need to be a regex? Maybe it would be simpler to just say $str = substr($str, 0, 200);
I think this is a dupe of bug 7369 which is fixed in can . But I will double check tomorrow.and look at the substr idea.
It's hard to tell what that regex is actually supposed to be doing. If the idea is that it trims each line to 200 characters, it needs a /m modifier. If it's only supposed to trim the entire string, then substr would be much clearer. On the other hand, since the code hasn't been working at all for several versions of perl, perhaps it could just be deleted?
The code would be more clear to me and be potentially faster and smaller (at run time) if we don't use a regex: $str = substr($str, 0, 200) if length($str) > 200; It's also unclear to me if a new line needs to be on the end of it?
In 3.4.1 I have $str =~ s/^(.{200}).+$/$1 [...]/gm; Unless you are looking at a development version this has been fixed.
I'm looking at line 921 here: https://metacpan.org/source/KMCGRAIL/Mail-SpamAssassin-3.4.1/lib/Mail/SpamAssassin/PerMsgStatus.pm
It turns out that my version has a FreeBSD port patch for Bug 7404, it's fixed in trunk. Someone should mark this as a duplicate of 7404.
I suggest the code be converted to substr as an additional enhancement. There's much less risk for surprise down the road if you do this.
(In reply to Todd Rinaldo from comment #8) > I suggest the code be converted to substr as an additional enhancement. > There's much less risk for surprise down the road if you do this. It's trimming the last line to 200 characters, not the entire string.
The regex in comment 5 appears to trim every line to 200 characters, not just the last line.
(In reply to waltman from comment #10) > The regex in comment 5 appears to trim every line to 200 characters, not > just the last line. I know, but if you read the comments and a few lines of context, it's clear that only the last line can be over 200 character.
(In reply to RW from comment #9) > (In reply to Todd Rinaldo from comment #8) > > I suggest the code be converted to substr as an additional enhancement. > > There's much less risk for surprise down the road if you do this. > > It's trimming the last line to 200 characters, not the entire string. I don't follow. My comment is not about what the regex is doing. My comment is that using substr would make it more clear what the intention is and also keep you away from {0,200} regex wierdness. Todd
Your version doesn't do the same thing; it trims to 200 characters from the beginning of the string rather than from the beginning of the last line and it doesn't add the ellipsis.
Is this the code you're referring to? $str =~ s/^(.{200}).+$/$1 [...]/gm; If so, that trims every line to 200 characters, not just the last one.
We've already been through this, it does apply to every line but only the last line can be over 200. The rest of the string is guaranteed to be under 200 in total.
(In reply to waltman from comment #14) > Is this the code you're referring to? > > $str =~ s/^(.{200}).+$/$1 [...]/gm; > > If so, that trims every line to 200 characters, not just the last one. Walt, I think we're communicating past each other. I think what RW is suggesting is that $str has multiple new lines and he's trying to shorten the last line in $str. In which case, he's correct that substr wouldn't work. Also it would mean that it truncates the last line of every $str but not every line in $str. Todd
It has been fixed on trunk, should we close this bz ?
(In reply to RW from comment #7) > Someone should mark this as a duplicate of 7404. Yup. *** This bug has been marked as a duplicate of bug 7404 ***
Fine with me.