[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 13da1a60f13e173f65bb0da5ab325641d5bb99ec by Serhiy Storchaka 
(Oren Milman) in branch '2.7':
[2.7] bpo-31478: Prevent unwanted behavior in _random.Random.seed() in case the 
arg has a bad __abs__() method (GH-3596) (#3845)
https://github.com/python/cpython/commit/13da1a60f13e173f65bb0da5ab325641d5bb99ec


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Oren Milman]

Change by Oren Milman :


--
pull_requests: +3826

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I agree that a backport is still desirable.

Your plan LGTM. Implement __abs__ raising an exception (test both int ant long 
subclasses). Make sure that no error is raised. Make sure that random() returns 
the same value as when seeding with an exact int and long.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Oren Milman]

Oren Milman  added the comment:

With regard to backporting to 2.7:

In 2.7 also, PyNumber_Absolute() is called, and its return value is stored in
the variable n.
However, there is no _PyLong_NumBits(n), so there is no assertion failure.
If n isn't an integer:
- if !PyObject_IsTrue(n), then the seed is zero (e.g. if n is None, [], () or 
{})
- otherwise, PyNumber_And() and PyNumber_Rshift() are used in a loop on n, so
  probably a TypeError would be raised.


So I think a backport is still desirable, but i am not sure about the test.

Maybe we should use @cpython_only, and make sure that no error is raised?
We can also make sure that random() returns a different value than when the
seed is zero.

What do you think?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset befc956acf8ddeb94f000ed081ddec51315429e5 by Serhiy Storchaka in 
branch '3.6':
[3.6] bpo-31478: Fix an assertion failure in random.seed() in case a seed has a 
bad __abs__() method. (GH-3596) (#3794)
https://github.com/python/cpython/commit/befc956acf8ddeb94f000ed081ddec51315429e5


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +3778

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset d780b2d588e68bd7047ef5d1f04e36da38b7a350 by Serhiy Storchaka 
(Oren Milman) in branch 'master':
bpo-31478: Fix an assertion failure in random.seed() in case a seed has a bad 
__abs__() method. (#3596)
https://github.com/python/cpython/commit/d780b2d588e68bd7047ef5d1f04e36da38b7a350


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Raymond Hettinger]

Raymond Hettinger added the comment:

>  I don't see reasons why it should obey overriding the __abs__() method.

I concur.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The more I think about this the more I like the idea of using int.__abs__() 
directly (PyLong_Type.tp_as_number->nb_absolute() in C). The C code doesn't 
call potentially overridable methods bit_length() and to_bytes(), it uses 
concrete implementations directly. I don't see reasons why it should obey 
overriding the __abs__() method.

This will save us from the problem with the wording of the error message.

I mentioned a drawback, but the current implementation has the same drawback. 
We can avoid copying positive integer subtypes by using more complex code, but 
I think it isn't worth.

--
versions: +Python 2.7, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Vedran Čačić]

Vedran Čačić added the comment:

So floats (and complexes) cannot be seeds anymore? :-o Or this pertains only to 
ints? In this case, I think the easiest doc fix is to change

If a is an int, it is used directly.

to

If a is an int, its absolute value is used.

--
nosy: +veky

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Oren Milman]

Oren Milman added the comment:

i opened a PR that implements the first option, but of course I wouldn't
mind if you decide another option is better.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Oren Milman]

Changes by Oren Milman :


--
keywords: +patch
pull_requests: +3587
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

There are several ways of solving this issue:

1. Verify that an actual int was returned and raise a TypeError if it wasn't.

2. Verify that an actual int was returned and fall back to the hash if it 
wasn't.

3. Use int.__abs__() instead of abs(), the result will be guaranteed int. The 
drawback is that this makes a copy of positive integers for int subclasses.

4. Check the sign of the seed and call int.__neg__() for negative values.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Oren Milman]

Oren Milman added the comment:

sure.
but what about the TypeError message? should it complain about
the return value of abs(seed)? (the docs of random.seed don't mention abs().)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Raymond Hettinger]

Raymond Hettinger added the comment:

It would make sense to verify that an actual int was returned and to raise a 
TypeError if it wasn't.  

Do you want to submit a PR (with a test case)?

--
nosy: +rhettinger, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31478] assertion failure in random.seed() in case the seed argument has a bad __abs__() method

[Oren Milman]

New submission from Oren Milman:

The following code causes an assertion failure:
class BadInt(int):
def __abs__(self):
return None

import random

random.seed(BadInt())


this is because random_seed() (in Modules/_randommodule.c) assumes that
PyNumber_Absolute() returned an int, and so it passes it to _PyLong_NumBits(),
which asserts it received an int.


what should we do in such a case?
should we raise an exception? (the docs don't mention abs() in case the seed is
an int - https://docs.python.org/3.7/library/random.html#random.seed)

--
components: Extension Modules
messages: 302208
nosy: Oren Milman
priority: normal
severity: normal
status: open
title: assertion failure in random.seed() in case the seed argument has a bad 
__abs__() method
type: crash
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com