BUG: Error checks for the dtype shape.

Also creates a clean tuple for the shape tuple, instead of
passing potentially mutable objects through.

Closes gh-4009

2 files changed, 95 insertions, 11 deletions

diff --git a/numpy/core/src/multiarray/descriptor.c b/numpy/core/src/multiarray/descriptor.c
index 10d4023d5..1d295a1cf 100644
--- a/numpy/core/src/multiarray/descriptor.c
+++ b/numpy/core/src/multiarray/descriptor.c
@@ -286,6 +286,8 @@ _convert_from_tuple(PyObject *obj)
          */
         PyArray_Dims shape = {NULL, -1};
         PyArray_Descr *newdescr;
+        npy_intp items;
+        int i;
 
         if (!(PyArray_IntpConverter(val, &shape)) || (shape.len > NPY_MAXDIMS)) {
             PyDimMem_FREE(shape.ptr);
@@ -310,9 +312,38 @@ _convert_from_tuple(PyObject *obj)
             PyDimMem_FREE(shape.ptr);
             goto fail;
         }
-        newdescr->elsize = type->elsize;
-        newdescr->elsize *= PyArray_MultiplyList(shape.ptr, shape.len);
-        PyDimMem_FREE(shape.ptr);
+
+        /* validate and set shape */
+        for (i=0; i < shape.len; i++) {
+            if (shape.ptr[i] < 0) {
+                PyErr_SetString(PyExc_ValueError,
+                                "invalid shape in fixed-type tuple: "
+                                "dimension smaller then zero.");
+                PyDimMem_FREE(shape.ptr);
+                goto fail;
+            }
+            if (shape.ptr[i] > NPY_MAX_INT) {
+                PyErr_SetString(PyExc_ValueError,
+                                "invalid shape in fixed-type tuple: "
+                                "dimension does not fit into a C int.");
+                PyDimMem_FREE(shape.ptr);
+                goto fail;
+            }
+        }
+        items = PyArray_OverflowMultiplyList(shape.ptr, shape.len);
+        if ((items < 0) || (items > (NPY_MAX_INT / type->elsize))) {
+            PyErr_SetString(PyExc_ValueError,
+                            "invalid shape in fixed-type tuple: dtype size in "
+                            "bytes must fit into a C int.");
+            PyDimMem_FREE(shape.ptr);
+            goto fail;
+        }
+        newdescr->elsize = type->elsize * items;
+        if (newdescr->elsize == -1) {
+            PyDimMem_FREE(shape.ptr);
+            goto fail;
+        }
+
         newdescr->subarray = PyArray_malloc(sizeof(PyArray_ArrayDescr));
         newdescr->flags = type->flags;
         newdescr->subarray->base = type;
@@ -321,17 +352,29 @@ _convert_from_tuple(PyObject *obj)
         Py_XDECREF(newdescr->names);
         newdescr->fields = NULL;
         newdescr->names = NULL;
-        /* Force subarray->shape to always be a tuple */
-        if (PyTuple_Check(val)) {
-            Py_INCREF(val);
-            newdescr->subarray->shape = val;
-        } else {
-            newdescr->subarray->shape = Py_BuildValue("(O)", val);
-            if (newdescr->subarray->shape == NULL) {
-                Py_DECREF(newdescr);
+
+        /*
+         * Create a new subarray->shape tuple (it can be an arbitrary
+         * sequence of integer like objects, neither of which is safe.
+         */
+        newdescr->subarray->shape = PyTuple_New(shape.len);
+        if (newdescr->subarray->shape == NULL) {
+            PyDimMem_FREE(shape.ptr);
+            goto fail;
+        }
+        for (i=0; i < shape.len; i++) {
+            PyTuple_SET_ITEM(newdescr->subarray->shape, i,
+                             PyInt_FromLong((long)shape.ptr[i]));
+
+            if (PyTuple_GET_ITEM(newdescr->subarray->shape, i) == NULL) {
+                Py_DECREF(newdescr->subarray->shape);
+                newdescr->subarray->shape = NULL;
+                PyDimMem_FREE(shape.ptr);
                 goto fail;
             }
         }
+
+        PyDimMem_FREE(shape.ptr);
         type = newdescr;
     }
     return type;
diff --git a/numpy/core/tests/test_dtype.py b/numpy/core/tests/test_dtype.py
index 83017ea6a..bd0761181 100644
--- a/numpy/core/tests/test_dtype.py
+++ b/numpy/core/tests/test_dtype.py
@@ -291,6 +291,47 @@ class TestSubarray(TestCase):
             np.dtype(([('a', [('a', 'i4', 6)], (2, 1)), ('b', 'f8', (1, 3))], (2, 2))),
             np.dtype(([('a', [('a', 'u4', 6)], (2, 1)), ('b', 'f8', (1, 3))], (2, 2))))
 
+    def test_shape_sequence(self):
+        # Any sequence of integers should work as shape, but the result
+        # should be a tuple (immutable) of base type integers.
+        a = np.array([1, 2, 3], dtype=np.int16)
+        l = [1, 2, 3]
+        # Array gets converted
+        dt = np.dtype([('a', 'f4', a)])
+        assert_(isinstance(dt['a'].shape, tuple))
+        assert_(isinstance(dt['a'].shape[0], int))
+        # List gets converted
+        dt = np.dtype([('a', 'f4', l)])
+        assert_(isinstance(dt['a'].shape, tuple))
+        # 
+        class IntLike(object):
+            def __index__(self):
+                return 3
+            def __int__(self):
+                # (a PyNumber_Check fails without __int__)
+                return 3
+        dt = np.dtype([('a', 'f4', IntLike())])
+        assert_(isinstance(dt['a'].shape, tuple))
+        assert_(isinstance(dt['a'].shape[0], int))
+        dt = np.dtype([('a', 'f4', (IntLike(),))])
+        assert_(isinstance(dt['a'].shape, tuple))
+        assert_(isinstance(dt['a'].shape[0], int))
+
+    def test_shape_invalid(self):
+        # Check that the shape is valid.
+        max_int = np.iinfo(np.intc).max
+        max_intp = np.iinfo(np.intp).max
+        # Too large values (the datatype is part of this)
+        assert_raises(ValueError, np.dtype, [('a', 'f4', max_int // 4 + 1)])
+        assert_raises(ValueError, np.dtype, [('a', 'f4', max_int + 1)])
+        assert_raises(ValueError, np.dtype, [('a', 'f4', (max_int, 2))])
+        # Takes a different code path (fails earlier:
+        assert_raises(ValueError, np.dtype, [('a', 'f4', max_intp + 1)])
+        # Negative values
+        assert_raises(ValueError, np.dtype, [('a', 'f4', -1)])
+        assert_raises(ValueError, np.dtype, [('a', 'f4', (-1, -1))])
+
+
 class TestMonsterType(TestCase):
     """Test deeply nested subtypes."""
     def test1(self):