gh-85260: Extend the AST Validator to validate all identifiers (GH-21069)

Author: isidentical

Lib/test/test_ast/test_ast.py

@@ -983,6 +983,34 @@ def test_constant_as_name(self):
             with self.assertRaisesRegex(ValueError, f"identifier field can't represent '{constant}' constant"):
                 compile(expr, "<test>", "eval")
 
+    def test_constant_in_identifier_fields(self):
+        # gh-85260: an identifier field holding a constant name used to
+        # crash the compiler
+        for statement in [
+            "def x(): pass",
+            "async def x(): pass",
+            "class x: pass",
+            "from a import x",
+            "from a import b as x",
+            "from a import b, c, d as x",
+            "import x",
+            "import a, b, x",
+            "try: pass\nexcept A as x: pass",
+            "try: pass\nexcept A as b: pass\nexcept B as x: pass\n",
+        ]:
+            for constant in "True", "False", "None":
+                with self.subTest(statement=statement, constant=constant):
+                    tree = ast.parse(statement)
+                    for node in ast.walk(tree):
+                        for field, value in ast.iter_fields(node):
+                            if value == "x":
+                                setattr(node, field, constant)
+                    with self.assertRaisesRegex(
+                            ValueError,
+                            f"identifier field can't represent "
+                            f"'{constant}' constant"):
+                        compile(tree, "<test>", "exec")
+
     def test_constant_as_unicode_name(self):
         constants = [
             ("True", b"Tru\xe1\xb5\x89"),

Misc/NEWS.d/next/Core_and_Builtins/2020-06-23-13-59-37.gh-issue-85260.o_LJ76.rst

@@ -0,0 +1,4 @@
+:func:`compile` now raises :exc:`ValueError` instead of crashing on a debug
+build if an identifier field of an AST node (such as the name of a function,
+a class, an imported module or a caught exception) is ``"None"``, ``"True"``
+or ``"False"``.

Python/ast.c

@@ -710,6 +710,23 @@ _validate_nonempty_seq(asdl_seq *seq, const char *what, const char *owner)
 }
 #define validate_nonempty_seq(seq, what, owner) _validate_nonempty_seq((asdl_seq*)seq, what, owner)
 
+static int
+validate_import_names(asdl_alias_seq *seq, const char *what, const char *owner)
+{
+    if (!validate_nonempty_seq(seq, what, owner)) {
+        return 0;
+    }
+    Py_ssize_t n = asdl_seq_LEN(seq);
+    for (Py_ssize_t i = 0; i < n; i++) {
+        alias_ty alias = asdl_seq_GET(seq, i);
+        if (!validate_name(alias->name) ||
+            (alias->asname && !validate_name(alias->asname))) {
+            return 0;
+        }
+    }
+    return 1;
+}
+
 static int
 validate_assignlist(asdl_expr_seq *targets, expr_context_ty ctx)
 {
@@ -735,6 +752,7 @@ validate_stmt(stmt_ty stmt)
     switch (stmt->kind) {
     case FunctionDef_kind:
         ret = validate_body(stmt->v.FunctionDef.body, "FunctionDef") &&
+            validate_name(stmt->v.FunctionDef.name) &&
             validate_type_params(stmt->v.FunctionDef.type_params) &&
             validate_arguments(stmt->v.FunctionDef.args) &&
             validate_exprs(stmt->v.FunctionDef.decorator_list, Load, 0) &&
@@ -743,6 +761,7 @@ validate_stmt(stmt_ty stmt)
         break;
     case ClassDef_kind:
         ret = validate_body(stmt->v.ClassDef.body, "ClassDef") &&
+            validate_name(stmt->v.ClassDef.name) &&
             validate_type_params(stmt->v.ClassDef.type_params) &&
             validate_exprs(stmt->v.ClassDef.bases, Load, 0) &&
             validate_keywords(stmt->v.ClassDef.keywords) &&
@@ -873,6 +892,8 @@ validate_stmt(stmt_ty stmt)
             VALIDATE_POSITIONS(handler);
             if ((handler->v.ExceptHandler.type &&
                  !validate_expr(handler->v.ExceptHandler.type, Load)) ||
+                (handler->v.ExceptHandler.name &&
+                 !validate_name(handler->v.ExceptHandler.name)) ||
                 !validate_body(handler->v.ExceptHandler.body, "ExceptHandler"))
                 return 0;
         }
@@ -911,14 +932,14 @@ validate_stmt(stmt_ty stmt)
             (!stmt->v.Assert.msg || validate_expr(stmt->v.Assert.msg, Load));
         break;
     case Import_kind:
-        ret = validate_nonempty_seq(stmt->v.Import.names, "names", "Import");
+        ret = validate_import_names(stmt->v.Import.names, "names", "Import");
         break;
     case ImportFrom_kind:
         if (stmt->v.ImportFrom.level < 0) {
             PyErr_SetString(PyExc_ValueError, "Negative ImportFrom level");
             return 0;
         }
-        ret = validate_nonempty_seq(stmt->v.ImportFrom.names, "names", "ImportFrom");
+        ret = validate_import_names(stmt->v.ImportFrom.names, "names", "ImportFrom");
         break;
     case Global_kind:
         ret = validate_nonempty_seq(stmt->v.Global.names, "names", "Global");
@@ -931,6 +952,7 @@ validate_stmt(stmt_ty stmt)
         break;
     case AsyncFunctionDef_kind:
         ret = validate_body(stmt->v.AsyncFunctionDef.body, "AsyncFunctionDef") &&
+            validate_name(stmt->v.AsyncFunctionDef.name) &&
             validate_type_params(stmt->v.AsyncFunctionDef.type_params) &&
             validate_arguments(stmt->v.AsyncFunctionDef.args) &&
             validate_exprs(stmt->v.AsyncFunctionDef.decorator_list, Load, 0) &&