From edc202c0254747780f615d992784f32f7c5e2fc3 Mon Sep 17 00:00:00 2001 From: INADA Naoki Date: Thu, 23 Feb 2017 15:51:44 +0900 Subject: [PATCH 1/3] bpo-29622: loosen positional argument check of AST constructor --- Parser/asdl_c.py | 5 ++--- Python/Python-ast.c | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index 444cdea341d78b..e14f2ed01bc6d1 100644 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -666,11 +666,10 @@ def visitModule(self, mod): } res = 0; /* if no error occurs, this stays 0 to the end */ if (PyTuple_GET_SIZE(args) > 0) { - if (numfields != PyTuple_GET_SIZE(args)) { - PyErr_Format(PyExc_TypeError, "%.400s constructor takes %s" + if (numfields < PyTuple_GET_SIZE(args)) { + PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most " "%zd positional argument%s", Py_TYPE(self)->tp_name, - numfields == 0 ? "" : "either 0 or ", numfields, numfields == 1 ? "" : "s"); res = -1; goto cleanup; diff --git a/Python/Python-ast.c b/Python/Python-ast.c index a6a49f78b1e249..f3e7afe89689f5 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -553,11 +553,10 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } res = 0; /* if no error occurs, this stays 0 to the end */ if (PyTuple_GET_SIZE(args) > 0) { - if (numfields != PyTuple_GET_SIZE(args)) { - PyErr_Format(PyExc_TypeError, "%.400s constructor takes %s" + if (numfields < PyTuple_GET_SIZE(args)) { + PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most " "%zd positional argument%s", Py_TYPE(self)->tp_name, - numfields == 0 ? "" : "either 0 or ", numfields, numfields == 1 ? "" : "s"); res = -1; goto cleanup; From d29ca6725a4e4a4f1b95d0caf963d73f10a2cfa9 Mon Sep 17 00:00:00 2001 From: INADA Naoki Date: Thu, 23 Feb 2017 18:40:02 +0900 Subject: [PATCH 2/3] remove test about argument is not enough --- Lib/test/test_ast.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py index ac79989796d6a4..366269fe405aa9 100644 --- a/Lib/test/test_ast.py +++ b/Lib/test/test_ast.py @@ -373,12 +373,8 @@ def test_nodeclasses(self): self.assertEqual(x.right, 3) self.assertEqual(x.lineno, 0) - # node raises exception when not given enough arguments - self.assertRaises(TypeError, ast.BinOp, 1, 2) # node raises exception when given too many arguments self.assertRaises(TypeError, ast.BinOp, 1, 2, 3, 4) - # node raises exception when not given enough arguments - self.assertRaises(TypeError, ast.BinOp, 1, 2, lineno=0) # node raises exception when given too many arguments self.assertRaises(TypeError, ast.BinOp, 1, 2, 3, 4, lineno=0) From 03940b1d6fe5420605ff4c7b544b05fa8ad70946 Mon Sep 17 00:00:00 2001 From: INADA Naoki Date: Fri, 24 Feb 2017 01:49:51 +0900 Subject: [PATCH 3/3] remove redundant check --- Parser/asdl_c.py | 35 +++++++++++++++++------------------ Python/Python-ast.c | 35 +++++++++++++++++------------------ 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index e14f2ed01bc6d1..096f5f8c7494f7 100644 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -664,28 +664,27 @@ def visitModule(self, mod): if (numfields == -1) goto cleanup; } + res = 0; /* if no error occurs, this stays 0 to the end */ - if (PyTuple_GET_SIZE(args) > 0) { - if (numfields < PyTuple_GET_SIZE(args)) { - PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most " - "%zd positional argument%s", - Py_TYPE(self)->tp_name, - numfields, numfields == 1 ? "" : "s"); + if (numfields < PyTuple_GET_SIZE(args)) { + PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most " + "%zd positional argument%s", + Py_TYPE(self)->tp_name, + numfields, numfields == 1 ? "" : "s"); + res = -1; + goto cleanup; + } + for (i = 0; i < PyTuple_GET_SIZE(args); i++) { + /* cannot be reached when fields is NULL */ + PyObject *name = PySequence_GetItem(fields, i); + if (!name) { res = -1; goto cleanup; } - for (i = 0; i < PyTuple_GET_SIZE(args); i++) { - /* cannot be reached when fields is NULL */ - PyObject *name = PySequence_GetItem(fields, i); - if (!name) { - res = -1; - goto cleanup; - } - res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i)); - Py_DECREF(name); - if (res < 0) - goto cleanup; - } + res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i)); + Py_DECREF(name); + if (res < 0) + goto cleanup; } if (kw) { i = 0; /* needed by PyDict_Next */ diff --git a/Python/Python-ast.c b/Python/Python-ast.c index f3e7afe89689f5..2759b2fe9c4b33 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -551,28 +551,27 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) if (numfields == -1) goto cleanup; } + res = 0; /* if no error occurs, this stays 0 to the end */ - if (PyTuple_GET_SIZE(args) > 0) { - if (numfields < PyTuple_GET_SIZE(args)) { - PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most " - "%zd positional argument%s", - Py_TYPE(self)->tp_name, - numfields, numfields == 1 ? "" : "s"); + if (numfields < PyTuple_GET_SIZE(args)) { + PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most " + "%zd positional argument%s", + Py_TYPE(self)->tp_name, + numfields, numfields == 1 ? "" : "s"); + res = -1; + goto cleanup; + } + for (i = 0; i < PyTuple_GET_SIZE(args); i++) { + /* cannot be reached when fields is NULL */ + PyObject *name = PySequence_GetItem(fields, i); + if (!name) { res = -1; goto cleanup; } - for (i = 0; i < PyTuple_GET_SIZE(args); i++) { - /* cannot be reached when fields is NULL */ - PyObject *name = PySequence_GetItem(fields, i); - if (!name) { - res = -1; - goto cleanup; - } - res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i)); - Py_DECREF(name); - if (res < 0) - goto cleanup; - } + res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i)); + Py_DECREF(name); + if (res < 0) + goto cleanup; } if (kw) { i = 0; /* needed by PyDict_Next */