From e15c28c176cb7d66ca2176d3f69dd50eab9a82bb Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 1 Oct 2017 15:56:09 +0300 Subject: [PATCH 1/5] init commit --- Lib/test/test_random.py | 21 +++++++++++++++++++++ Modules/_randommodule.c | 11 +++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 90d334a835a2a7f..45edbd30ab77717 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -307,6 +307,27 @@ def test_randbelow_logic(self, _log=log, int=int): class MersenneTwister_TestBasicOps(TestBasicOps): gen = random.Random() + @test_support.cpython_only + def test_bug_31478(self): + # _random.Random.seed() should ignore the __abs__() method of a + # long/int subclass argument. + class BadInt(int): + def __abs__(self): + 1/0 + class BadLong(long): + def __abs__(self): + 1/0 + + for seed_arg in [-42, 0, 42]: + self.gen.seed(seed_arg) + expected_rand = self.gen.random() + self.gen.seed(long(seed_arg)) + self.assertEqual(expected_rand, self.gen.random()) + self.gen.seed(BadInt(seed_arg)) + self.assertEqual(expected_rand, self.gen.random()) + self.gen.seed(BadLong(seed_arg)) + self.assertEqual(expected_rand, self.gen.random()) + def test_setstate_first_arg(self): self.assertRaises(ValueError, self.gen.setstate, (1, None, None)) diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index 852b810ae219557..bd368cc867770c7 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -231,8 +231,15 @@ random_seed(RandomObject *self, PyObject *args) /* If the arg is an int or long, use its absolute value; else use * the absolute value of its hash code. */ - if (PyInt_Check(arg) || PyLong_Check(arg)) - n = PyNumber_Absolute(arg); + if (PyInt_Check(arg)) { + /* Calling int.__abs__() (or long.__abs__()) prevents calling + arg.__abs__(), which might return an invalid value. + See issue #31478. */ + n = PyInt_Type.tp_as_number->nb_absolute(arg); + } + else if (PyLong_Check(arg)) { + n = PyLong_Type.tp_as_number->nb_absolute(arg); + } else { long hash = PyObject_Hash(arg); if (hash == -1) From f5300dbe0e7e6b143846f7560fd0b8d728de13bf Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 1 Oct 2017 16:04:20 +0300 Subject: [PATCH 2/5] move the comment --- Modules/_randommodule.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index bd368cc867770c7..2e49db6ef7379a1 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -230,11 +230,10 @@ random_seed(RandomObject *self, PyObject *args) } /* If the arg is an int or long, use its absolute value; else use * the absolute value of its hash code. + * Calling int.__abs__() or long.__abs__() prevents calling arg.__abs__(), + * which might return an invalid value. See issue #31478. */ if (PyInt_Check(arg)) { - /* Calling int.__abs__() (or long.__abs__()) prevents calling - arg.__abs__(), which might return an invalid value. - See issue #31478. */ n = PyInt_Type.tp_as_number->nb_absolute(arg); } else if (PyLong_Check(arg)) { From eb8ea90b4bcd3870307808fbd66a99e882ee7319 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 1 Oct 2017 16:40:18 +0300 Subject: [PATCH 3/5] simplify the test --- Lib/test/test_random.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 45edbd30ab77717..aa62780303a3628 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -318,15 +318,11 @@ class BadLong(long): def __abs__(self): 1/0 - for seed_arg in [-42, 0, 42]: + self.gen.seed(42) + expected_value = self.gen.random() + for seed_arg in [42L, BadInt(42), BadLong(42)]: self.gen.seed(seed_arg) - expected_rand = self.gen.random() - self.gen.seed(long(seed_arg)) - self.assertEqual(expected_rand, self.gen.random()) - self.gen.seed(BadInt(seed_arg)) - self.assertEqual(expected_rand, self.gen.random()) - self.gen.seed(BadLong(seed_arg)) - self.assertEqual(expected_rand, self.gen.random()) + self.assertEqual(expected_value, self.gen.random()) def test_setstate_first_arg(self): self.assertRaises(ValueError, self.gen.setstate, (1, None, None)) From e9d75c6511b3bdca0868635a7eb9499c3a901e65 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 1 Oct 2017 16:41:46 +0300 Subject: [PATCH 4/5] remove blank line --- Lib/test/test_random.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index aa62780303a3628..396c6deb08896d8 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -317,7 +317,6 @@ def __abs__(self): class BadLong(long): def __abs__(self): 1/0 - self.gen.seed(42) expected_value = self.gen.random() for seed_arg in [42L, BadInt(42), BadLong(42)]: From 27f926045befff98df6b6c32616ab33bfb80927c Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 1 Oct 2017 19:01:19 +0300 Subject: [PATCH 5/5] add a NEWS.d item, and make a little fix to the test --- Lib/test/test_random.py | 2 +- .../Core and Builtins/2017-10-01-18-59-40.bpo-31478.owtqoO.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-10-01-18-59-40.bpo-31478.owtqoO.rst diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 396c6deb08896d8..d2a4f213f13e169 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -321,7 +321,7 @@ def __abs__(self): expected_value = self.gen.random() for seed_arg in [42L, BadInt(42), BadLong(42)]: self.gen.seed(seed_arg) - self.assertEqual(expected_value, self.gen.random()) + self.assertEqual(self.gen.random(), expected_value) 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-10-01-18-59-40.bpo-31478.owtqoO.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-01-18-59-40.bpo-31478.owtqoO.rst new file mode 100644 index 000000000000000..b5b32d6989f23ab --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-01-18-59-40.bpo-31478.owtqoO.rst @@ -0,0 +1,2 @@ +Prevent unwanted behavior in `_random.Random.seed()` in case the argument +has a bad ``__abs__()`` method. Patch by Oren Milman.