Issue #14203: Remove obsolete support for view==NULL in bytesiobuf_getbuffer()

Stefan Krah · Stefan Krah · commit 650c1e818d7b · 2015-02-03T21:43:23.000+01:00
and array_buffer_getbuf().
diff --git a/Lib/test/test_array.py b/Lib/test/test_array.py
@@ -1041,6 +1041,11 @@ def test_initialize_with_unicode(self):
             a = array.array(self.typecode, "foo")
             a = array.array(self.typecode, array.array('u', 'foo'))
 
+    @support.cpython_only
+    def test_obsolete_write_lock(self):
+        from _testcapi import getbuffer_with_null_view
+        a = array.array('B', b"")
+        self.assertRaises(BufferError, getbuffer_with_null_view, a)
 
 class StringTest(BaseTest):
 
diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py
@@ -1224,6 +1224,10 @@ def delslice():
         self.assertRaises(BufferError, delslice)
         self.assertEqual(b, orig)
 
+    @test.support.cpython_only
+    def test_obsolete_write_lock(self):
+        from _testcapi import getbuffer_with_null_view
+        self.assertRaises(BufferError, getbuffer_with_null_view, bytearray())
 
 class AssortedBytesTest(unittest.TestCase):
     #
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -1560,9 +1560,9 @@ Build
 C API
 -----
 
-- Issue #14203: Remove obsolete support for view==NULL in PyBuffer_FillInfo()
-  and bytearray_getbuffer().  Both functions now raise BufferError in that
-  case.
+- Issue #14203: Remove obsolete support for view==NULL in PyBuffer_FillInfo(),
+  bytearray_getbuffer(), bytesiobuf_getbuffer() and array_buffer_getbuf().
+  All functions now raise BufferError in that case.
 
 - Issue #22445: PyBuffer_IsContiguous() now implements precise contiguity
   tests, compatible with NumPy's NPY_RELAXED_STRIDES_CHECKING compilation
diff --git a/Modules/_io/bytesio.c b/Modules/_io/bytesio.c
@@ -1028,23 +1028,24 @@ PyTypeObject PyBytesIO_Type = {
 static int
 bytesiobuf_getbuffer(bytesiobuf *obj, Py_buffer *view, int flags)
 {
-    int ret;
     bytesio *b = (bytesio *) obj->source;
+
+    if (view == NULL) {
+        PyErr_SetString(PyExc_BufferError,
+            "bytesiobuf_getbuffer: view==NULL argument is obsolete");
+        return -1;
+    }
     if (SHARED_BUF(b)) {
         if (unshare_buffer(b, b->string_size) < 0)
             return -1;
     }
-    if (view == NULL) {
-        b->exports++;
-        return 0;
-    }
-    ret = PyBuffer_FillInfo(view, (PyObject*)obj,
+
+    /* cannot fail if view != NULL and readonly == 0 */
+    (void)PyBuffer_FillInfo(view, (PyObject*)obj,
                             PyBytes_AS_STRING(b->buf), b->string_size,
                             0, flags);
-    if (ret >= 0) {
-        b->exports++;
-    }
-    return ret;
+    b->exports++;
+    return 0;
 }
 
 static void
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -2518,21 +2518,26 @@ test_from_contiguous(PyObject* self, PyObject *noargs)
 
     Py_RETURN_NONE;
 }
- 
+
+extern PyTypeObject _PyBytesIOBuffer_Type;
+
 static PyObject *
 test_pep3118_obsolete_write_locks(PyObject* self, PyObject *noargs)
 {
+    PyTypeObject *type = &_PyBytesIOBuffer_Type;
     PyObject *b;
     char *dummy[1];
     int ret, match;
 
+    /* PyBuffer_FillInfo() */
     ret = PyBuffer_FillInfo(NULL, NULL, dummy, 1, 0, PyBUF_SIMPLE);
     match = PyErr_Occurred() && PyErr_ExceptionMatches(PyExc_BufferError);
     PyErr_Clear();
     if (ret != -1 || match == 0)
         goto error;
 
-    b = PyByteArray_FromStringAndSize("", 0);
+    /* bytesiobuf_getbuffer() */
+    b = type->tp_alloc(type, 0);
     if (b == NULL) {
         return NULL;
     }
@@ -2552,6 +2557,18 @@ test_pep3118_obsolete_write_locks(PyObject* self, PyObject *noargs)
     return NULL;
 }
 
+/* This tests functions that historically supported write locks.  It is
+   wrong to call getbuffer() with view==NULL and a compliant getbufferproc
+   is entitled to segfault in that case. */
+static PyObject *
+getbuffer_with_null_view(PyObject* self, PyObject *obj)
+{
+    if (PyObject_GetBuffer(obj, NULL, PyBUF_SIMPLE) < 0)
+        return NULL;
+
+    Py_RETURN_NONE;
+}
+
 /* Test that the fatal error from not having a current thread doesn't
    cause an infinite loop.  Run via Lib/test/test_capi.py */
 static PyObject *
@@ -3213,6 +3230,7 @@ static PyMethodDef TestMethods[] = {
     {"test_capsule", (PyCFunction)test_capsule, METH_NOARGS},
     {"test_from_contiguous", (PyCFunction)test_from_contiguous, METH_NOARGS},
     {"test_pep3118_obsolete_write_locks", (PyCFunction)test_pep3118_obsolete_write_locks, METH_NOARGS},
+    {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O},
     {"getargs_tuple",           getargs_tuple,                   METH_VARARGS},
     {"getargs_keywords", (PyCFunction)getargs_keywords,
       METH_VARARGS|METH_KEYWORDS},
diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c
@@ -2530,7 +2530,11 @@ static const void *emptybuf = "";
 static int
 array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags)
 {
-    if (view==NULL) goto finish;
+    if (view == NULL) {
+        PyErr_SetString(PyExc_BufferError,
+            "array_buffer_getbuf: view==NULL argument is obsolete");
+        return -1;
+    }
 
     view->buf = (void *)self->ob_item;
     view->obj = (PyObject*)self;
@@ -2560,7 +2564,6 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags)
 #endif
     }
 
- finish:
     self->ob_exports++;
     return 0;
 }

Original file line number	Diff line number	Diff line change
`@@ -1224,6 +1224,10 @@ def delslice():`
`1224`	`1224`	`self.assertRaises(BufferError, delslice)`
`1225`	`1225`	`self.assertEqual(b, orig)`
`1226`	`1226`
	`1227`	`+ @test.support.cpython_only`
	`1228`	`+ def test_obsolete_write_lock(self):`
	`1229`	`+ from _testcapi import getbuffer_with_null_view`
	`1230`	`+ self.assertRaises(BufferError, getbuffer_with_null_view, bytearray())`
`1227`	`1231`
`1228`	`1232`	`class AssortedBytesTest(unittest.TestCase):`
`1229`	`1233`	`#`
Original file line number	Diff line number	Diff line change
`@@ -2530,7 +2530,11 @@ static const void *emptybuf = "";`
`2530`	`2530`	`static int`
`2531`	`2531`	`array_buffer_getbuf(arrayobject self, Py_buffer view, int flags)`
`2532`	`2532`	`{`
`2533`		`- if (view==NULL) goto finish;`
	`2533`	`+ if (view == NULL) {`
	`2534`	`+ PyErr_SetString(PyExc_BufferError,`
	`2535`	`+ "array_buffer_getbuf: view==NULL argument is obsolete");`
	`2536`	`+ return -1;`
	`2537`	`+ }`
`2534`	`2538`
`2535`	`2539`	`view->buf = (void *)self->ob_item;`
`2536`	`2540`	`view->obj = (PyObject*)self;`
`@@ -2560,7 +2564,6 @@ array_buffer_getbuf(arrayobject self, Py_buffer view, int flags)`
`2560`	`2564`	`#endif`
`2561`	`2565`	`}`
`2562`	`2566`
`2563`		`- finish:`
`2564`	`2567`	`self->ob_exports++;`
`2565`	`2568`	`return 0;`
`2566`	`2569`	`}`