Repository: python/cpython
Pull request: gh-91760: More strict rules for numerical group references and group names in RE
Only sequence of ASCII digits ~not starting with 0 (except group 0)~ is
now accepted as a numerical reference.
The group name in bytes patterns and replacement strings can now only
contain ASCII letters and digits and underscore.

Closes #91760.


Problem statement:
More strict rules for group numbers and names in RE
There were unintentional changes in parsing regular expressions between Python 2 and Python 3.

1. Group references.

   In patterns and replacement strings you can refer a group by its number using syntax `\N` where N is a 1-2 digit decimal number. The number should not start by 0, because it will be in an octal escape sequence. The group number can also be used in the conditional expression `(?(N)...)` in patterns and in references `\g<N>` in replacement strings. And it is interesting, that in Python 3 it can be not only a sequence of decimal digits. The following things are allowed in the group number:

   * Initial zero: `\g<01>`.
   * Spaces around the number: `\g< 1 >`.
   * Underscores: `\g<1_2>`.
   * Non-decimal digits: `\g<¹>`.
   * Non-ASCII decimal digits: `\g<१>`.

   All this is purely an implementation artifact. After `\g<` we search the nearest `>` and pass a substring between `<` and `>` to `int()`. In other implementation we could search the longest sequence of decimal digits and all above examples (except may be the first one) would be filtered out automatically.

2. Group names.

   In `(?P<name>...)`, `(?P=name)`, `(?(name)...)` and `\g<name>` we can refer groups by name. To avoid ambiguity there is a limitation: the name should follow the rules for identifier. In Python 2 it means that it should contain only letters, digits and underscores and start with a non-digit. Letters and digits are ASCII-only: [A-Za-z] and [0-9].

   In Python 3 identifiers can contain non-ASCII letters and digits. It is good. But in bytes patterns and replacement strings the codes `\xaa`, `\xb2`, `\xb3`, `\xb5`, `\xb9`, `\xba`, `\xc0`-`\xd6`, `\xd8`-`\xf6`, `\xf8`-`\xff` are allowed in the group name. They correspond characters `ª²³µ¹ºÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõöøùúûüýþÿ` after decoding.

   It is an implementation artifact too. Bytes patterns and replacement strings are decoded with the Latin1 encoding for parsing. It simplifies and speeds up the code. There is no other reason why letters and digits in the range U-0080--U-00FF are allowed.

   Note that In Python 3 the bytes literal can only contain printable literal characters in the ASCII range. Codes outside of this range should be represented as octal or hexadecimal escape sequences. So supporting non-ASCII letters and digits does not add to readability.

Since the above "features" are not intentional, not supported by most other RE engines (except `regex`, which is also written in Python), are not tested, and can be changed in result of refactoring the parser, I suggest to introduce more strict rules on group number and name.

1. Group number should only contain ASCII decimal digits in range [0-9]. Initial 0 is not allowed except for group number 0.
2. Group name in the bytes pattern or replacement string should only contain ASCII letters and digits.

The question: do we need a deprecation period for this? I have wrote a code for both options (with deprecation and with error), will create PRs tomorrow.

Hints from issue discussion:
> The question: do we need a deprecation period for this?

A deprecation period of at least one release would be good.

Commit message: Merge branch 'main' into re-group-name

Patch to review:
diff --git a/Doc/library/re.rst b/Doc/library/re.rst
index 89de9286ace79c..c65aaf58990b73 100644
--- a/Doc/library/re.rst
+++ b/Doc/library/re.rst
@@ -395,7 +395,8 @@ The special characters are:
 ``(?P<name>...)``
    Similar to regular parentheses, but the substring matched by the group is
    accessible via the symbolic group name *name*.  Group names must be valid
-   Python identifiers, and each group name must be defined only once within a
+   Python identifiers, and in bytes patterns they must contain only characters
+   in the ASCII range.  Each group name must be defined only once within a
    regular expression.  A symbolic group is also a numbered group, just as if
    the group were not named.
 
@@ -417,6 +418,10 @@ The special characters are:
    |                                       | * ``\1``                         |
    +---------------------------------------+----------------------------------+
 
+   .. versionchanged:: 3.11
+      In bytes patterns group names must contain only characters in
+      the ASCII range.
+
 .. index:: single: (?P=; in regular expressions
 
 ``(?P=name)``
@@ -486,6 +491,9 @@ The special characters are:
    will match with ``'<user@host.com>'`` as well as ``'user@host.com'``, but
    not with ``'<user@host.com'`` nor ``'user@host.com>'``.
 
+   .. versionchanged:: 3.11
+      Group *id* can only contain ASCII digits and cannot start with ``0``.
+
 
 The special sequences consist of ``'\'`` and a character from the list below.
 If the ordinary character is not an ASCII digit or an ASCII letter, then the
@@ -995,6 +1003,12 @@ form.
       Empty matches for the pattern are replaced when adjacent to a previous
       non-empty match.
 
+   .. versionchanged:: 3.11
+      Group *id* can only contain ASCII digits and cannot start with ``0``
+      (except group 0).
+      In bytes replacement strings group names must contain only characters
+      in the ASCII range.
+
 
 .. function:: subn(pattern, repl, string, count=0, flags=0)
 
diff --git a/Doc/whatsnew/3.11.rst b/Doc/whatsnew/3.11.rst
index 5b53fbe0dda6ff..cd34c813fe897b 100644
--- a/Doc/whatsnew/3.11.rst
+++ b/Doc/whatsnew/3.11.rst
@@ -1070,6 +1070,14 @@ Changes in the Python API
   before.
   (Contributed by Ma Lin in :issue:`35859`.)
 
+* More strict rules are now applied for numerical group references and
+  group names in regular expressions.
+  Only sequence of ASCII digits not starting with ``0`` (except group 0) is
+  now accepted as a numerical reference.
+  The group name in bytes patterns and replacement strings can now only
+  contain ASCII letters and digits and underscore.
+  (Contributed by Serhiy Storchaka in :issue:`91760`.)
+
 * The *population* parameter of :func:`random.sample` must be a sequence.
   Automatic conversion of sets to lists is no longer supported. If the sample size
   is larger than the population size, a :exc:`ValueError` is raised.
diff --git a/Lib/re/_parser.py b/Lib/re/_parser.py
index b864bf2b108a6f..8db7f451a8cf18 100644
--- a/Lib/re/_parser.py
+++ b/Lib/re/_parser.py
@@ -293,6 +293,14 @@ def seek(self, index):
     def error(self, msg, offset=0):
         return error(msg, self.string, self.tell() - offset)
 
+    def checkgroupname(self, name, offset):
+        if not (self.istext or name.isascii()):
+            msg = "bad character in group name %a" % name
+            raise self.error(msg, len(name) + offset)
+        if not name.isidentifier():
+            msg = "bad character in group name %r" % name
+            raise self.error(msg, len(name) + offset)
+
 def _class_escape(source, escape):
     # handle escape code inside character class
     code = ESCAPES.get(escape)
@@ -707,15 +715,11 @@ def _parse(source, state, verbose, nested, first=False):
                     if sourcematch("<"):
                         # named group: skip forward to end of name
                         name = source.getuntil(">", "group name")
-                        if not name.isidentifier():
-                            msg = "bad character in group name %r" % name
-                            raise source.error(msg, len(name) + 1)
+                        source.checkgroupname(name, 1)
                     elif sourcematch("="):
                         # named backreference
                         name = source.getuntil(")", "group name")
-                        if not name.isidentifier():
-                            msg = "bad character in group name %r" % name
-                            raise source.error(msg, len(name) + 1)
+                        source.checkgroupname(name, 1)
                         gid = state.groupdict.get(name)
                         if gid is None:
                             msg = "unknown group name %r" % name
@@ -776,19 +780,9 @@ def _parse(source, state, verbose, nested, first=False):
                 elif char == "(":
                     # conditional backreference group
                     condname = source.getuntil(")", "group name")
-                    if condname.isidentifier():
-                        condgroup = state.groupdict.get(condname)
-                        if condgroup is None:
-                            msg = "unknown group name %r" % condname
-                            raise source.error(msg, len(condname) + 1)
-                    else:
-                        try:
-                            condgroup = int(condname)
-                            if condgroup < 0:
-                                raise ValueError
-                        except ValueError:
-                            msg = "bad character in group name %r" % condname
-                            raise source.error(msg, len(condname) + 1) from None
+                    if (condname.isdecimal() and condname.isascii() and
+                            (condname[0] != "0" or condname == "0")):
+                        condgroup = int(condname)
                         if not condgroup:
                             raise source.error("bad group number",
                                                len(condname) + 1)
@@ -799,6 +793,12 @@ def _parse(source, state, verbose, nested, first=False):
                             state.grouprefpos[condgroup] = (
                                 source.tell() - len(condname) - 1
                             )
+                    else:
+                        source.checkgroupname(condname, 1)
+                        condgroup = state.groupdict.get(condname)
+                        if condgroup is None:
+                            msg = "unknown group name %r" % condname
+                            raise source.error(msg, len(condname) + 1)
                     state.checklookbehindgroup(condgroup, source)
                     item_yes = _parse(source, state, verbose, nested + 1)
                     if source.match("|"):
@@ -1004,26 +1004,21 @@ def addgroup(index, pos):
             # group
             c = this[1]
             if c == "g":
-                name = ""
                 if not s.match("<"):
                     raise s.error("missing <")
                 name = s.getuntil(">", "group name")
-                if name.isidentifier():
+                if (name.isdecimal() and name.isascii() and
+                        (name[0] != "0" or name == "0")):
+                    index = int(name)
+                    if index >= MAXGROUPS:
+                        raise s.error("invalid group reference %d" % index,
+                                      len(name) + 1)
+                else:
+                    s.checkgroupname(name, 1)
                     try:
                         index = groupindex[name]
                     except KeyError:
                         raise IndexError("unknown group name %r" % name) from None
-                else:
-                    try:
-                        index = int(name)
-                        if index < 0:
-                            raise ValueError
-                    except ValueError:
-                        raise s.error("bad character in group name %r" % name,
-                                      len(name) + 1) from None
-                    if index >= MAXGROUPS:
-                        raise s.error("invalid group reference %d" % index,
-                                      len(name) + 1)
                 addgroup(index, len(name) + 1)
             elif c == "0":
                 if s.next in OCTDIGITS:
diff --git a/Lib/test/test_re.py b/Lib/test/test_re.py
index 91971d8126c98a..74e1f4e87be64a 100644
--- a/Lib/test/test_re.py
+++ b/Lib/test/test_re.py
@@ -135,6 +135,7 @@ def test_basic_re_sub(self):
         self.assertEqual(re.sub('(?P<a>x)', r'\g<a>\g<1>', 'xx'), 'xxxx')
         self.assertEqual(re.sub('(?P<unk>x)', r'\g<unk>\g<unk>', 'xx'), 'xxxx')
         self.assertEqual(re.sub('(?P<unk>x)', r'\g<1>\g<1>', 'xx'), 'xxxx')
+        self.assertEqual(re.sub('()x', r'\g<0>\g<0>', 'xx'), 'xxxx')
 
         self.assertEqual(re.sub('a', r'\t\n\v\r\f\a\b', 'a'), '\t\n\v\r\f\a\b')
         self.assertEqual(re.sub('a', '\t\n\v\r\f\a\b', 'a'), '\t\n\v\r\f\a\b')
@@ -274,6 +275,12 @@ def test_symbolic_groups_errors(self):
         self.checkPatternError('(?P<©>x)', "bad character in group name '©'", 4)
         self.checkPatternError('(?P=©)', "bad character in group name '©'", 4)
         self.checkPatternError('(?(©)y)', "bad character in group name '©'", 3)
+        self.checkPatternError(b'(?P<\xc2\xb5>x)',
+                               r"bad character in group name '\xc2\xb5'", 4)
+        self.checkPatternError(b'(?P=\xc2\xb5)',
+                               r"bad character in group name '\xc2\xb5'", 4)
+        self.checkPatternError(b'(?(\xc2\xb5)y)',
+                               r"bad character in group name '\xc2\xb5'", 3)
 
     def test_symbolic_refs(self):
         self.assertEqual(re.sub('(?P<a>x)|(?P<b>y)', r'\g<b>', 'xx'), '')
@@ -306,12 +313,24 @@ def test_symbolic_refs_errors(self):
             re.sub('(?P<a>x)', r'\g<ab>', 'xx')
         self.checkTemplateError('(?P<a>x)', r'\g<-1>', 'xx',
                                 "bad character in group name '-1'", 3)
+        self.checkTemplateError('(?P<a>x)', r'\g<+1>', 'xx',
+                                "bad character in group name '+1'", 3)
+        self.checkTemplateError('(?P<a>x)', r'\g<01>', 'xx',
+                                "bad character in group name '01'", 3)
+        self.checkTemplateError('()'*10, r'\g<1_0>', 'xx',
+                                "bad character in group name '1_0'", 3)
+        self.checkTemplateError('(?P<a>x)', r'\g< 1 >', 'xx',
+                                "bad character in group name ' 1 '", 3)
         self.checkTemplateError('(?P<a>x)', r'\g<©>', 'xx',
                                 "bad character in group name '©'", 3)
+        self.checkTemplateError(b'(?P<a>x)', b'\\g<\xc2\xb5>', b'xx',
+                                r"bad character in group name '\xc2\xb5'", 3)
         self.checkTemplateError('(?P<a>x)', r'\g<㊀>', 'xx',
                                 "bad character in group name '㊀'", 3)
         self.checkTemplateError('(?P<a>x)', r'\g<¹>', 'xx',
                                 "bad character in group name '¹'", 3)
+        self.checkTemplateError('(?P<a>x)', r'\g<१>', 'xx',
+                                "bad character in group name '१'", 3)
 
     def test_re_subn(self):
         self.assertEqual(re.subn("(?i)b+", "x", "bbbb BBBB"), ('x x', 2))
@@ -577,10 +596,20 @@ def test_re_groupref_exists_errors(self):
         self.checkPatternError(r'(?P<a>)(?(0)a|b)', 'bad group number', 10)
         self.checkPatternError(r'()(?(-1)a|b)',
                                "bad character in group name '-1'", 5)
+        self.checkPatternError(r'()(?(+1)a|b)',
+                               "bad character in group name '+1'", 5)
+        self.checkPatternError(r'()(?(01)a|b)',
+                               "bad character in group name '01'", 5)
+        self.checkPatternError(r'()'*10 + r'(?(1_0)a|b)',
+                               "bad character in group name '1_0'", 23)
+        self.checkPatternError(r'()(?( 1 )a|b)',
+                               "bad character in group name ' 1 '", 5)
         self.checkPatternError(r'()(?(㊀)a|b)',
                                "bad character in group name '㊀'", 5)
         self.checkPatternError(r'()(?(¹)a|b)',
                                "bad character in group name '¹'", 5)
+        self.checkPatternError(r'()(?(१)a|b)',
+                               "bad character in group name '१'", 5)
         self.checkPatternError(r'()(?(1',
                                "missing ), unterminated name", 5)
         self.checkPatternError(r'()(?(1)a',
diff --git a/Misc/NEWS.d/next/Library/2022-04-21-19-14-29.gh-issue-91760.54AR-m.rst b/Misc/NEWS.d/next/Library/2022-04-21-19-14-29.gh-issue-91760.54AR-m.rst
new file mode 100644
index 00000000000000..439313c433bf36
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-04-21-19-14-29.gh-issue-91760.54AR-m.rst
@@ -0,0 +1,5 @@
+Apply more strict rules for numerical group references and group names in
+regular expressions. Only sequence of ASCII digits not starting with 0
+(except group 0) is now accepted as a numerical reference. The group name in
+bytes patterns and replacement strings can now only contain ASCII letters
+and digits and underscore.


Code review comments:
diff_hunk: @@ -776,19 +780,9 @@ def _parse(source, state, verbose, nested, first=False):
                 elif char == "(":
                     # conditional backreference group
                     condname = source.getuntil(")", "group name")
-                    if condname.isidentifier():
-                        condgroup = state.groupdict.get(condname)
-                        if condgroup is None:
-                            msg = "unknown group name %r" % condname
-                            raise source.error(msg, len(condname) + 1)
-                    else:
-                        try:
-                            condgroup = int(condname)
-                            if condgroup < 0:
-                                raise ValueError
-                        except ValueError:
-                            msg = "bad character in group name %r" % condname
-                            raise source.error(msg, len(condname) + 1) from None
+                    if (condname.isdecimal() and condname.isascii() and
+                            (condname[0] != "0" or condname == "0")):
+                        condgroup = int(condname)
                         if not condgroup:
                             raise source.error("bad group number",
path: Lib/re/_parser.py
text: @user1:
Wouldn't this fail if condname is `"0"`?  If this is intentional I would use a more explicit `condgroup == 0`, otherwise `.isdecimal` already expects at least 1 digit and doesn't allow negative numbers, so condgroup should always be an `int` `>=0`, making the condition always `True`.

@author:
Yes, it is intentional.

@user1:
Is this because `0` is an invalid group with `(?(...)...|...)`?
The documentation doesn't mention this.

@author:
It just does not make any sense. Group 0 does not exist until the matching finished.