Bug 7480 - Unescaped left brace in regex
Summary: Unescaped left brace in regex
Status: RESOLVED DUPLICATE of bug 7404
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Libraries (show other bugs)
Version: 3.4.1
Hardware: PC Linux
: P2 normal
Target Milestone: Undefined
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-24 00:29 UTC by waltman
Modified: 2018-01-11 17:52 UTC (History)
6 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status

Note You need to log in before you can comment on or make changes to this bug.
Description waltman 2017-10-24 00:29:28 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;
Comment 1 waltman 2017-10-24 00:52:54 UTC
In fact, does that even need to be a regex? Maybe it would be simpler to just say

$str = substr($str, 0, 200);
Comment 2 Kevin A. McGrail 2017-10-24 01:21:40 UTC
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.
Comment 3 waltman 2017-10-24 11:27:47 UTC
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?
Comment 4 Todd Rinaldo 2017-10-24 16:38:55 UTC
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?
Comment 5 RW 2017-10-24 17:35:06 UTC

In 3.4.1 I have 

   $str =~ s/^(.{200}).+$/$1 [...]/gm;

Unless you are looking at a development version this has been fixed.
Comment 7 RW 2017-10-24 19:30:15 UTC
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.
Comment 8 Todd Rinaldo 2017-10-24 20:01:35 UTC
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.
Comment 9 RW 2017-10-24 20:24:09 UTC
(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.
Comment 10 waltman 2017-10-25 15:54:00 UTC
The regex in comment 5 appears to trim every line to 200 characters, not just the last line.
Comment 11 RW 2017-10-25 16:04:42 UTC
(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.
Comment 12 Todd Rinaldo 2017-10-27 16:25:51 UTC
(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
Comment 13 RW 2017-10-27 19:51:20 UTC
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.
Comment 14 waltman 2017-10-27 20:39:22 UTC
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.
Comment 15 RW 2017-10-27 21:58:01 UTC
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.
Comment 16 Todd Rinaldo 2017-10-27 23:01:11 UTC
(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
Comment 17 Giovanni Bechis 2018-01-11 08:05:57 UTC
It has been fixed on trunk, should we close this bz ?
Comment 18 Bill Cole 2018-01-11 14:53:51 UTC
(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 ***
Comment 19 waltman 2018-01-11 17:52:22 UTC
Fine with me.