From 499a26d838961f59053df1f95689fed831934429 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Fri, 15 Sep 2017 09:21:47 +0300 Subject: [PATCH 1/4] init commit --- Lib/test/test_random.py | 10 ++++++++++ .../2017-09-15-09-13-07.bpo-31478.o06iKD.rst | 2 ++ Modules/_randommodule.c | 17 ++++++++++++++--- 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index fbb1cf6696e531..a45ece7e3c7d2f 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -430,6 +430,16 @@ def test_bug_27706(self): ['0x1.b0580f98a7dbep-1', '0x1.84129978f9c1ap-1', '0x1.aeaa51052e978p-2', '0x1.092178fb945a6p-2']) + @support.cpython_only + def test_issue31478(self): + # There shouldn't be an assertion failure in seed() in case the 'a' + # argument has a bad __abs__() method. + class BadInt(int): + def __abs__(self): + return None + with self.assertRaises(TypeError): + self.gen.seed(BadInt()) + def test_setstate_first_arg(self): self.assertRaises(ValueError, self.gen.setstate, (1, None, None)) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst new file mode 100644 index 00000000000000..2cf5ee0a1f6dfe --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst @@ -0,0 +1,2 @@ +Fix an assertion failure in `random.seed()` in case the `a` argument has a +bad ``__abs__()`` method. Patch by Oren Milman. diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index 747a5473420872..981d434ebe60aa 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -259,16 +259,27 @@ random_seed(RandomObject *self, PyObject *args) * So: if the arg is a PyLong, use its absolute value. * Otherwise use its hash value, cast to unsigned. */ - if (PyLong_Check(arg)) + if (PyLong_Check(arg)) { n = PyNumber_Absolute(arg); + if (n == NULL) { + goto Done; + } + if (!PyLong_Check(n)) { + PyErr_Format(PyExc_TypeError, + "abs(a) must return an integer, not '%.200s'", + Py_TYPE(n)->tp_name); + goto Done; + } + } else { Py_hash_t hash = PyObject_Hash(arg); if (hash == -1) goto Done; n = PyLong_FromSize_t((size_t)hash); + if (n == NULL) { + goto Done; + } } - if (n == NULL) - goto Done; /* Now split n into 32-bit chunks, from the right. */ bits = _PyLong_NumBits(n); From 448297cbd885167c17edfa4ba6e48117cf569a55 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sat, 23 Sep 2017 18:43:02 +0300 Subject: [PATCH 2/4] use PyLong_Type.tp_as_number->nb_absolute(arg) --- Lib/test/test_random.py | 7 +++---- .../2017-09-15-09-13-07.bpo-31478.o06iKD.rst | 2 +- Modules/_randommodule.c | 19 ++++--------------- 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index a45ece7e3c7d2f..f5c4c7b900ea38 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -432,13 +432,12 @@ def test_bug_27706(self): @support.cpython_only def test_issue31478(self): - # There shouldn't be an assertion failure in seed() in case the 'a' - # argument has a bad __abs__() method. + # There shouldn't be an assertion failure in _random.Random.seed() in + # case the argument has a bad __abs__() method. class BadInt(int): def __abs__(self): return None - with self.assertRaises(TypeError): - self.gen.seed(BadInt()) + self.gen.seed(BadInt()) def test_setstate_first_arg(self): self.assertRaises(ValueError, self.gen.setstate, (1, None, None)) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst index 2cf5ee0a1f6dfe..bbeb810ebaa248 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-15-09-13-07.bpo-31478.o06iKD.rst @@ -1,2 +1,2 @@ -Fix an assertion failure in `random.seed()` in case the `a` argument has a +Fix an assertion failure in `_random.Random.seed()` in case the argument has a bad ``__abs__()`` method. Patch by Oren Milman. diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index 981d434ebe60aa..d114fabd148539 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -259,27 +259,16 @@ random_seed(RandomObject *self, PyObject *args) * So: if the arg is a PyLong, use its absolute value. * Otherwise use its hash value, cast to unsigned. */ - if (PyLong_Check(arg)) { - n = PyNumber_Absolute(arg); - if (n == NULL) { - goto Done; - } - if (!PyLong_Check(n)) { - PyErr_Format(PyExc_TypeError, - "abs(a) must return an integer, not '%.200s'", - Py_TYPE(n)->tp_name); - goto Done; - } - } + if (PyLong_Check(arg)) + n = PyLong_Type.tp_as_number->nb_absolute(arg); else { Py_hash_t hash = PyObject_Hash(arg); if (hash == -1) goto Done; n = PyLong_FromSize_t((size_t)hash); - if (n == NULL) { - goto Done; - } } + if (n == NULL) + goto Done; /* Now split n into 32-bit chunks, from the right. */ bits = _PyLong_NumBits(n); From aaca5452932a0871d972e8be0b9a7faf961af671 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sat, 23 Sep 2017 19:03:30 +0300 Subject: [PATCH 3/4] add braces --- Modules/_randommodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index d114fabd148539..2fe47fabbb7389 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -259,8 +259,9 @@ random_seed(RandomObject *self, PyObject *args) * So: if the arg is a PyLong, use its absolute value. * Otherwise use its hash value, cast to unsigned. */ - if (PyLong_Check(arg)) + if (PyLong_Check(arg)) { n = PyLong_Type.tp_as_number->nb_absolute(arg); + } else { Py_hash_t hash = PyObject_Hash(arg); if (hash == -1) From 852b643e7e39ceb1208925701fe661e5f9b83a06 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sat, 23 Sep 2017 22:21:45 +0300 Subject: [PATCH 4/4] remove cpython_only decorator, and add a comment. --- Lib/test/test_random.py | 6 ++++-- Modules/_randommodule.c | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index f3ef9646e8e29c..e2db72330c5524 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -430,14 +430,16 @@ def test_bug_27706(self): ['0x1.b0580f98a7dbep-1', '0x1.84129978f9c1ap-1', '0x1.aeaa51052e978p-2', '0x1.092178fb945a6p-2']) - @support.cpython_only def test_bug_31478(self): # There shouldn't be an assertion failure in _random.Random.seed() in # case the argument has a bad __abs__() method. class BadInt(int): def __abs__(self): return None - self.gen.seed(BadInt()) + try: + self.gen.seed(BadInt()) + except TypeError: + pass def test_bug_31482(self): # Verify that version 1 seeds are unaffected by hash randomization diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index 2fe47fabbb7389..51677f8b00a492 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -260,6 +260,8 @@ random_seed(RandomObject *self, PyObject *args) * Otherwise use its hash value, cast to unsigned. */ if (PyLong_Check(arg)) { + /* Calling int.__abs__() prevents calling arg.__abs__(), which might + return an invalid value. See issue #31478. */ n = PyLong_Type.tp_as_number->nb_absolute(arg); } else {