Issue #19390: Argument Clinic no longer accepts malformed Python and C ids.

Author: larryhastings

Misc/NEWS

@@ -129,6 +129,11 @@ Build
 - Issue #19356: Avoid using a C variabled named "_self", it's a reserved
   word in some C compilers.
 
+Tools/Demos
+-----------
+
+- Issue #19390: Argument Clinic no longer accepts malformed Python
+  and C ids.
 
 What's New in Python 3.4.0 Alpha 4?
 ===================================

Tools/clinic/clinic.py

@@ -122,6 +122,11 @@ def quoted_for_c_string(s):
         s = s.replace(old, new)
     return s
 
+is_legal_c_identifier = re.compile('^[A-Za-z_][A-Za-z0-9_]*$').match
+
+def is_legal_py_identifier(s):
+    return all(is_legal_c_identifier(field) for field in s.split('.'))
+
 # added "self", "cls", and "null" just to be safe
 # (clinic will generate variables with these names)
 c_keywords = set("""
@@ -131,8 +136,11 @@ def quoted_for_c_string(s):
 typeof union unsigned void volatile while
 """.strip().split())
 
-def legal_c_identifier(s):
-    # if we picked a C keyword, pick something else
+def ensure_legal_c_identifier(s):
+    # for now, just complain if what we're given isn't legal
+    if not is_legal_c_identifier(s):
+        fail("Illegal C identifier: {}".format(s))
+    # but if we picked a C keyword, pick something else
     if s in c_keywords:
         return s + "_value"
     return s
@@ -1311,7 +1319,7 @@ def render(self, parameter, data):
         parameter is a clinic.Parameter instance.
         data is a CRenderData instance.
         """
-        name = legal_c_identifier(self.name)
+        name = ensure_legal_c_identifier(self.name)
 
         # declarations
         d = self.declaration()
@@ -1359,7 +1367,7 @@ def parse_argument(self, list):
         if self.encoding:
             list.append(self.encoding)
 
-        s = ("&" if self.parse_by_reference else "") + legal_c_identifier(self.name)
+        s = ("&" if self.parse_by_reference else "") + ensure_legal_c_identifier(self.name)
         list.append(s)
 
     #
@@ -1377,7 +1385,7 @@ def simple_declaration(self, by_reference=False):
             prototype.append(" ")
         if by_reference:
             prototype.append('*')
-        prototype.append(legal_c_identifier(self.name))
+        prototype.append(ensure_legal_c_identifier(self.name))
         return "".join(prototype)
 
     def declaration(self):
@@ -1575,7 +1583,7 @@ def converter_init(self, *, str=False, zeroes=False, nullable=False, read_write=
             self.format_unit = 'z*' if nullable else 's*'
 
     def cleanup(self):
-        return "PyBuffer_Release(&" + legal_c_identifier(self.name) + ");\n"
+        return "PyBuffer_Release(&" + ensure_legal_c_identifier(self.name) + ");\n"
 
 
 def add_c_return_converter(f, name=None):
@@ -1895,6 +1903,11 @@ def state_modulename_name(self, line):
         full_name = full_name.strip()
         c_basename = c_basename.strip() or None
 
+        if not is_legal_py_identifier(full_name):
+            fail("Illegal function name: {}".format(full_name))
+        if c_basename and not is_legal_c_identifier(c_basename):
+            fail("Illegal C basename: {}".format(c_basename))
+
         if not returns:
             return_converter = CReturnConverter()
         else:

Tools/clinic/clinic_test.py

@@ -560,6 +560,20 @@ def test_no_parameters(self):
         self.assertEqual("Docstring\n\nfoo.bar()", function.docstring)
         self.assertEqual(0, len(function.parameters))
 
+    def test_illegal_module_line(self):
+        self.parse_function_should_fail("""
+module foo
+foo.bar => int
+    /
+""")
+
+    def test_illegal_c_basename(self):
+        self.parse_function_should_fail("""
+module foo
+foo.bar as 935
+    /
+""")
+
     def test_single_star(self):
         self.parse_function_should_fail("""
 module foo