This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Warn on unescaped /[]}]/ under re strict
authorKarl Williamson <khw@cpan.org>
Fri, 6 Jan 2017 05:03:35 +0000 (22:03 -0700)
committerKarl Williamson <khw@cpan.org>
Fri, 13 Jan 2017 19:20:04 +0000 (12:20 -0700)
This commit generates a warning when the experimental 're strict'
feature is in effect for unescaped '}' and ']' characters (in a regular
expression pattern) that are interpreted literally.

This brings the behavior of these more in line with ')' which croaks
when it is taken literally.

The problem with the existing behavior is that these characters may be
metacharacters or they may be literals, depending on action at a
distance.  Not so with ')', which is always a metacharacter unless
escaped.

Ideally, all three of these characters should behave similarly, but it
really is too late for that, except we can warn if the user has
requested extra checking of their patterns with this experimental
're strict' feature.

pod/perldelta.pod
pod/perldiag.pod
regcomp.c
t/re/reg_mesg.t

index 86b7e9a..a375b74 100644 (file)
@@ -138,6 +138,22 @@ L<XXX> has been upgraded from version A.xx to B.yy.
 
 =item *
 
+L<re> has been upgraded from version 0.33 to 0.34;
+
+This adds support for the new L<C</xx>|perlre/E<sol>x and E<sol>xx>
+regular expression pattern modifier, and a change to the L<S<C<use re
+'strict'>>|re/'strict' mode> experimental feature.  When S<C<re
+'strict'>> is enabled, a warning now will be generated for all
+unescaped uses of the two characters C<}> and C<]> in regular
+expression patterns (outside bracketed character classes) that are taken
+literally.  This brings them more in line with the C<)> character which
+is always a metacharacter unless escaped.  Being a metacharacter only
+sometimes, depending on action at a distance, can lead to silently
+having the pattern mean something quite different than was intended,
+which the S<C<re 'strict'>> mode is intended to minimize.
+
+=item *
+
 L<B::Xref> has been upgraded from version 1.05 to 1.06.
 
 =item *
index 7d6675c..4de4574 100644 (file)
@@ -6327,6 +6327,26 @@ as the first character following a quantifier
 
 =back
 
+=item Unescaped literal '%c' in regex; marked by <-- HERE in m/%s/
+
+(W regexp) (only under C<S<use re 'strict'>>)
+
+Within the scope of C<S<use re 'strict'>> in a regular expression
+pattern, you included an unescaped C<}> or C<]> which was interpreted
+literally.  These two characters are sometimes metacharacters, and
+sometimes literals, depending on what precedes them in the
+pattern.  This is unlike the similar C<)> which is always a
+metacharacter unless escaped.
+
+This action at a distance, perhaps a large distance, can lead to Perl
+silently misinterpreting what you meant, so when you specify that you
+want extra checking by C<S<use re 'strict'>>, this warning is generated.
+If you meant the character as a literal, simply confirm that to Perl by
+preceding the character with a backslash, or make it into a bracketed
+character class (like C<[}]>).  If you meant it as closing a
+corresponding C<[> or C<{>, you'll need to look back through the pattern
+to find out why that isn't happening.
+
 =item unexec of %s into %s failed!
 
 (F) The unexec() routine failed for some reason.  See your local FSF
index fe094b1..2cbc94d 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -13363,6 +13363,12 @@ S_regatom(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth)
                         RExC_parse = p + 1;
                        vFAIL("Unescaped left brace in regex is illegal here");
                    }
+                   goto normal_default;
+                case '}':
+                case ']':
+                    if (PASS2 && p > RExC_parse && RExC_strict) {
+                        ckWARN2reg(p + 1, "Unescaped literal '%c'", *p);
+                    }
                    /*FALLTHROUGH*/
                default:    /* A literal character */
                  normal_default:
index 435f1ee..b2ed98a 100644 (file)
@@ -568,6 +568,7 @@ my @warning = (
                                                'Assuming NOT a POSIX class since no blanks are allowed in one {#} m/[[   ^   {#}:   x d i g i t   :   ]   ]\x{100}/',
                                                'Assuming NOT a POSIX class since no blanks are allowed in one {#} m/[[   ^   :   {#}x d i g i t   :   ]   ]\x{100}/',
                                                'Assuming NOT a POSIX class since no blanks are allowed in one {#} m/[[   ^   :   x d i g i t   :   ]{#}   ]\x{100}/',
+                                               $only_strict_marker . 'Unescaped literal \']\' {#} m/[[   ^   :   x d i g i t   :   ]   ]{#}\x{100}/',
                             ],
     '/[foo:lower:]]\x{100}/' => 'Assuming NOT a POSIX class since it doesn\'t start with a \'[\' {#} m/[foo{#}:lower:]]\x{100}/',
     '/[[;upper;]]\x{100}/' => [ 'Assuming NOT a POSIX class since a semi-colon was found instead of a colon {#} m/[[;{#}upper;]]\x{100}/',
@@ -618,6 +619,8 @@ my @warning_only_under_strict = (
     "/[A-$B_hex]/" => "Ranges of ASCII printables should be some subset of \"0-9\", \"A-Z\", or \"a-z\" {#} m/[A-$B_hex\{#}]/",
     "/[$low_mixed_alpha-$high_mixed_alpha]/" => "Ranges of ASCII printables should be some subset of \"0-9\", \"A-Z\", or \"a-z\" {#} m/[$low_mixed_alpha-$high_mixed_alpha\{#}]/",
     "/[$low_mixed_digit-$high_mixed_digit]/" => "Ranges of ASCII printables should be some subset of \"0-9\", \"A-Z\", or \"a-z\" {#} m/[$low_mixed_digit-$high_mixed_digit\{#}]/",
+    '/\b<GCB}/' => 'Unescaped literal \'}\' {#} m/\b<GCB}{#}/',
+    '/[ ]def]/' => 'Unescaped literal \']\' {#} m/[ ]def]{#}/',
 );
 
 my @warning_utf8_only_under_strict = mark_as_utf8(
@@ -690,6 +693,8 @@ for my $strict ("",  "no warnings 'experimental::re_strict'; use re 'strict';")
     }
     else {
         for (my $i = 0; $i < @warning_only_under_strict; $i += 2) {
+
+            # (?[ ]) are always under strict
             if ($warning_only_under_strict[$i] =~ /\Q(?[/) {
                 push @warning_tests, $warning_only_under_strict[$i],  # The regex
                                     $warning_only_under_strict[$i+1];