From 2447773573e74819e163f8963ab107bc5db123e5 Mon Sep 17 00:00:00 2001
From: Tal Einat <taleinat+github@gmail.com>
Date: Mon, 22 Oct 2018 18:33:10 +0300
Subject: [PATCH] bpo-29843: raise AttributeError if given negative _length_
 (GH-10029)

Raise ValueError OverflowError in case of a negative
_length_ in a ctypes.Array subclass.  Also raise TypeError
instead of AttributeError for non-integer _length_.


Co-authored-by: Oren Milman <orenmn@gmail.com>
---
 Lib/ctypes/test/test_arrays.py                | 26 ++++++++++++++-----
 .../2018-10-21-17-43-48.bpo-29743.aeCcKR.rst  |  4 +++
 Modules/_ctypes/_ctypes.c                     | 25 ++++++++++++++----
 3 files changed, 44 insertions(+), 11 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-10-21-17-43-48.bpo-29743.aeCcKR.rst

diff --git a/Lib/ctypes/test/test_arrays.py b/Lib/ctypes/test/test_arrays.py
index 6e562cfd24e..6cfda8b7d2e 100644
--- a/Lib/ctypes/test/test_arrays.py
+++ b/Lib/ctypes/test/test_arrays.py
@@ -163,8 +163,6 @@ class ArrayTestCase(unittest.TestCase):
         self.assertEqual(Y()._length_, 187)
 
     def test_bad_subclass(self):
-        import sys
-
         with self.assertRaises(AttributeError):
             class T(Array):
                 pass
@@ -174,14 +172,30 @@ class ArrayTestCase(unittest.TestCase):
         with self.assertRaises(AttributeError):
             class T(Array):
                 _length_ = 13
+
+    def test_bad_length(self):
+        with self.assertRaises(ValueError):
+            class T(Array):
+                _type_ = c_int
+                _length_ = - sys.maxsize * 2
+        with self.assertRaises(ValueError):
+            class T(Array):
+                _type_ = c_int
+                _length_ = -1
+        with self.assertRaises(TypeError):
+            class T(Array):
+                _type_ = c_int
+                _length_ = 1.87
         with self.assertRaises(OverflowError):
             class T(Array):
                 _type_ = c_int
                 _length_ = sys.maxsize * 2
-        with self.assertRaises(AttributeError):
-            class T(Array):
-                _type_ = c_int
-                _length_ = 1.87
+
+    def test_zero_length(self):
+        # _length_ can be zero.
+        class T(Array):
+            _type_ = c_int
+            _length_ = 0
 
     @unittest.skipUnless(sys.maxsize > 2**32, 'requires 64bit platform')
     @bigmemtest(size=_2G, memuse=1, dry_run=False)
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-10-21-17-43-48.bpo-29743.aeCcKR.rst b/Misc/NEWS.d/next/Core and Builtins/2018-10-21-17-43-48.bpo-29743.aeCcKR.rst
new file mode 100644
index 00000000000..9e79bb388e8
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-10-21-17-43-48.bpo-29743.aeCcKR.rst	
@@ -0,0 +1,4 @@
+Raise :exc:`ValueError` instead of :exc:`OverflowError` in case of a negative
+``_length_`` in a :class:`ctypes.Array` subclass.  Also raise :exc:`TypeError`
+instead of :exc:`AttributeError` for non-integer ``_length_``.
+Original patch by Oren Milman.
diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c
index 3ae6348fef4..60f6985a664 100644
--- a/Modules/_ctypes/_ctypes.c
+++ b/Modules/_ctypes/_ctypes.c
@@ -1405,13 +1405,28 @@ PyCArrayType_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
     type_attr = NULL;
 
     length_attr = PyObject_GetAttrString((PyObject *)result, "_length_");
-    if (!length_attr || !PyLong_Check(length_attr)) {
-        PyErr_SetString(PyExc_AttributeError,
-                        "class must define a '_length_' attribute, "
-                        "which must be a positive integer");
-        Py_XDECREF(length_attr);
+    if (!length_attr) {
+        if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
+            PyErr_SetString(PyExc_AttributeError,
+                            "class must define a '_length_' attribute");
+        }
         goto error;
     }
+
+    if (!PyLong_Check(length_attr)) {
+        Py_DECREF(length_attr);
+        PyErr_SetString(PyExc_TypeError,
+                        "The '_length_' attribute must be an integer");
+        goto error;
+    }
+
+    if (_PyLong_Sign(length_attr) == -1) {
+        Py_DECREF(length_attr);
+        PyErr_SetString(PyExc_ValueError,
+                        "The '_length_' attribute must not be negative");
+        goto error;
+    }
+
     length = PyLong_AsSsize_t(length_attr);
     Py_DECREF(length_attr);
     if (length == -1 && PyErr_Occurred()) {