From a1d08106c52d3deabb26f9ff2f859d38d68a1c2d Mon Sep 17 00:00:00 2001
From: Maxwell Bernstein <emacs@fb.com>
Date: Fri, 8 Mar 2019 15:59:24 -0800
Subject: [PATCH] Check callback is callable in PyWeakref_NewRef

The documentation says that `PyWeakref_NewRef` should raise a
`TypeError` if the callback is not callable, None, or NULL, but the code
does not reflect the documentation. This PR fixes that.
---
 Lib/test/test_weakref.py                               |  8 ++++++++
 .../C API/2019-03-09-00-10-25.bpo-36203.yKKWd3.rst     |  1 +
 Modules/_testcapimodule.c                              | 10 ++++++++++
 Objects/weakrefobject.c                                |  7 +++++++
 4 files changed, 26 insertions(+)
 create mode 100644 Misc/NEWS.d/next/C API/2019-03-09-00-10-25.bpo-36203.yKKWd3.rst

diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index 56a42f055d0b54..31cecc1b0ec42a 100644
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -130,6 +130,14 @@ def test_cfunction(self):
         self.check_basic_ref(create_cfunction)
         self.check_basic_callback(create_cfunction)
 
+    def test_PyWeakref_NewRef_with_non_callable_raises_type_error(self):
+        import _testcapi
+        o = C()
+        with self.assertRaises(TypeError) as ctx:
+            _testcapi.PyWeakref_NewRef(o, 3)
+        self.assertEqual(str(ctx.exception),
+                         "callback must be None, NULL, or callable object")
+
     def test_multiple_callbacks(self):
         o = C()
         ref1 = weakref.ref(o, self.callback)
diff --git a/Misc/NEWS.d/next/C API/2019-03-09-00-10-25.bpo-36203.yKKWd3.rst b/Misc/NEWS.d/next/C API/2019-03-09-00-10-25.bpo-36203.yKKWd3.rst
new file mode 100644
index 00000000000000..09183477993b0d
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2019-03-09-00-10-25.bpo-36203.yKKWd3.rst	
@@ -0,0 +1 @@
+Amend PyWeakref_New to raise a TypeError if the callback given is not NULL, None, or a callable object. The documentation previously falsely claimed this check occurred.
\ No newline at end of file
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 0c48accf2803a8..59b1a4d8d0d063 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -5547,6 +5547,15 @@ test_fatal_error(PyObject *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
+static PyObject *
+test_PyWeakref_NewRef(PyObject *self, PyObject *args) {
+    PyObject *ob = NULL;
+    PyObject *callback = NULL;
+    if (!PyArg_ParseTuple(args, "OO", &ob, &callback))
+        return NULL;
+    return PyWeakref_NewRef(ob, callback);
+}
+
 
 static PyObject *test_buildvalue_issue38913(PyObject *, PyObject *);
 static PyObject *getargs_s_hash_int(PyObject *, PyObject *, PyObject*);
@@ -5825,6 +5834,7 @@ static PyMethodDef TestMethods[] = {
     {"test_py_is_funcs", test_py_is_funcs, METH_NOARGS},
     {"fatal_error", test_fatal_error, METH_VARARGS,
      PyDoc_STR("fatal_error(message, release_gil=False): call Py_FatalError(message)")},
+    {"PyWeakref_NewRef", test_PyWeakref_NewRef, METH_VARARGS},
     {NULL, NULL} /* sentinel */
 };
 
diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
index 313e8abab5a25f..314f0daeeacbaf 100644
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -835,6 +835,13 @@ PyWeakref_NewRef(PyObject *ob, PyObject *callback)
     if (callback == NULL)
         /* return existing weak reference if it exists */
         result = ref;
+    else {
+        if (!PyCallable_Check(callback)) {
+            PyErr_SetString(PyExc_TypeError,
+                            "callback must be None, NULL, or callable object");
+            return NULL;
+        }
+    }
     if (result != NULL)
         Py_INCREF(result);
     else {