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

diff --git a/Misc/NEWS b/Misc/NEWS
index 9ebe6bc6eb9594f790a27201100a05857c1be8c9..07aff2b89f20e007cc90d5ec38a0e839739789a1 100644 (file)
--- a/Misc/NEWS
+++ b/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?
 ===================================

diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py
index ced3e675245a0d9c1142257ec3fcbec5325ec573..cb4d6ab1e094b412c8e4e6767f83b39e6c1d75e3 100755 (executable)
--- a/Tools/clinic/clinic.py
+++ b/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 @@ return self short signed sizeof static struct switch typedef
 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 @@ class CConverter(metaclass=CConverterAutoRegister):
         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 @@ class CConverter(metaclass=CConverterAutoRegister):
         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 @@ class CConverter(metaclass=CConverterAutoRegister):
             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 @@ class Py_buffer_converter(CConverter):
             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 @@ class DSLParser:
         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:

diff --git a/Tools/clinic/clinic_test.py b/Tools/clinic/clinic_test.py
index 81a0050d7ebc7e3be8bf8d26bc6d547f357fc189..7baf3801bdac4d6ffc6af343a19c36ed6564c51b 100644 (file)
--- a/Tools/clinic/clinic_test.py
+++ b/Tools/clinic/clinic_test.py
@@ -560,6 +560,20 @@ Docstring
         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