bpo-31478: Fix an assertion failure in random.seed() in case the 'a' arg has a bad abs() method #3596

orenmn · 2017-09-15T09:34:18Z

in _randommodule.c: add a check whether PyNumber_Absolute() hasn't returned an integer.
in test_random.py: add a test to verify that the assertion failure is no more.

https://bugs.python.org/issue31478

serhiy-storchaka · 2017-09-15T09:57:54Z

Modules/_randommodule.c

+        }
+        if (!PyLong_Check(n)) {
+            PyErr_Format(PyExc_TypeError,
+                         "abs(a) must return an integer, not '%.200s'",


abs(seed)?

i agree this seems weird, but that's what the docs say (https://docs.python.org/3.7/library/random.html#random.seed).
should i change to abs(seed) anyway? (or should i also update the docs?)

The doc is about random.Random.seed(). This is _random.Random.seed(). It doesn't have named arguments. I don't know what is the best wording, but mentioning a whose name is arbitrary and not related to this function looks incorrect to me.

I leave up this to @rhettinger.

serhiy-storchaka · 2017-09-15T10:02:05Z

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


What is `a`?

serhiy-storchaka · 2017-09-23T17:09:45Z

Lib/test/test_random.py

                return None
-        with self.assertRaises(TypeError):
-            self.gen.seed(BadInt())
+        self.gen.seed(BadInt())


I think that if wrap this with try: ... except TypeError: pass, the cpython_only decorator can be removed.

serhiy-storchaka · 2017-09-23T17:12:18Z

Modules/_randommodule.c

-                         Py_TYPE(n)->tp_name);
-            goto Done;
-        }
+        n = PyLong_Type.tp_as_number->nb_absolute(arg);


Please add a short comment explaining why this is written in such way.

serhiy-storchaka · 2017-09-24T11:10:08Z

@rhettinger, if you don't have comments I'll merge this PR.

miss-islington · 2017-09-28T07:50:03Z

Thanks @orenmn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6.
🐍🍒⛏🤖

miss-islington · 2017-09-28T07:50:09Z

Sorry @orenmn and @serhiy-storchaka, I had trouble checking out the 3.6 backport branch.
Please backport using cherry_picker on command line.
cherry_picker d780b2d588e68bd7047ef5d1f04e36da38b7a350 3.6

miss-islington · 2017-09-28T07:50:13Z

Sorry, @orenmn and @serhiy-storchaka, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker d780b2d588e68bd7047ef5d1f04e36da38b7a350 2.7

…seed has a bad __abs__() method. (pythonGH-3596) (cherry picked from commit d780b2d)

bedevere-bot · 2017-09-28T07:52:10Z

GH-3794 is a backport of this pull request to the 3.6 branch.

serhiy-storchaka · 2017-09-28T07:54:54Z

Would you mind to create a backport to the 2.7 branch @orenmn?

orenmn · 2017-09-28T08:20:26Z

sure

…seed has a bad __abs__() method. (GH-3596) (#3794) (cherry picked from commit d780b2d)

orenmn · 2017-10-01T06:16:05Z

The behavior is different in 2.7, so i wasn't sure how to proceed, and asked for advice in https://bugs.python.org/issue31478#msg303207..

bedevere-bot · 2017-10-01T13:51:07Z

GH-3845 is a backport of this pull request to the 2.7 branch.

…n case the arg has a bad __abs__() method (GH-3596) (#3845)

init commit

499a26d

orenmn requested a review from rhettinger as a code owner September 15, 2017 09:34

bedevere-bot added the awaiting review label Sep 15, 2017

the-knights-who-say-ni added the CLA signed label Sep 15, 2017

serhiy-storchaka reviewed Sep 15, 2017

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Sep 15, 2017

orenmn added 3 commits September 23, 2017 18:43

use PyLong_Type.tp_as_number->nb_absolute(arg)

448297c

merge master

0226085

add braces

aaca545

serhiy-storchaka reviewed Sep 23, 2017

View reviewed changes

remove cpython_only decorator, and add a comment.

852b643

serhiy-storchaka approved these changes Sep 23, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Sep 23, 2017

serhiy-storchaka self-assigned this Sep 26, 2017

serhiy-storchaka added needs backport to 2.7 type-bug An unexpected behavior, bug, or error labels Sep 26, 2017

serhiy-storchaka merged commit d780b2d into python:master Sep 28, 2017

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Sep 28, 2017

[3.6] bpo-31478: Fix an assertion failure in random.seed() in case a …

04e9676

…seed has a bad __abs__() method. (pythonGH-3596) (cherry picked from commit d780b2d)

serhiy-storchaka mentioned this pull request Sep 28, 2017

[3.6] bpo-31478: Fix an assertion failure in random.seed() in case a … #3794

Merged

bedevere-bot removed the needs backport to 3.6 label Sep 28, 2017

serhiy-storchaka added a commit that referenced this pull request Sep 28, 2017

[3.6] bpo-31478: Fix an assertion failure in random.seed() in case a …

befc956

…seed has a bad __abs__() method. (GH-3596) (#3794) (cherry picked from commit d780b2d)

bedevere-bot removed the needs backport to 2.7 label Oct 1, 2017

serhiy-storchaka pushed a commit that referenced this pull request Oct 2, 2017

[2.7] bpo-31478: Prevent unwanted behavior in _random.Random.seed() i…

13da1a6

…n case the arg has a bad __abs__() method (GH-3596) (#3845)

Mariatta removed the awaiting merge label Oct 8, 2017

serhiy-storchaka removed their assignment Dec 6, 2018

		@@ -0,0 +1,2 @@
		Fix an assertion failure in `random.seed()` in case the `a` argument has a

Uh oh!

bpo-31478: Fix an assertion failure in random.seed() in case the 'a' arg has a bad __abs__() method #3596

bpo-31478: Fix an assertion failure in random.seed() in case the 'a' arg has a bad __abs__() method #3596

Uh oh!

Conversation

orenmn commented Sep 15, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka Sep 15, 2017

Choose a reason for hiding this comment

Uh oh!

orenmn Sep 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 22, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 15, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 28, 2017

Uh oh!

miss-islington commented Sep 28, 2017

Uh oh!

miss-islington commented Sep 28, 2017

Uh oh!

bedevere-bot commented Sep 28, 2017

Uh oh!

serhiy-storchaka commented Sep 28, 2017

Uh oh!

orenmn commented Sep 28, 2017

Uh oh!

orenmn commented Oct 1, 2017

Uh oh!

bedevere-bot commented Oct 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bpo-31478: Fix an assertion failure in random.seed() in case the 'a' arg has a bad abs() method #3596

bpo-31478: Fix an assertion failure in random.seed() in case the 'a' arg has a bad abs() method #3596

orenmn commented Sep 15, 2017 •

edited by bedevere-bot

Loading

orenmn Sep 15, 2017 •

edited

Loading