classification
Title: os.urandom(1.1): infinite loop
Type: behavior Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: ajaksu2, amaury.forgeotdarc, benjamin.peterson, gregory.p.smith, pitrou, rpetrov
Priority: normal Keywords: easy, patch

Created on 2008-08-27 23:30 by ajaksu2, last changed 2008-09-18 22:53 by rpetrov. This issue is now closed.

Files
File name Uploaded Description Edit
urandom.diff ajaksu2, 2008-08-27 23:30 Check len(bytes) against int(n)
urandom-float-and-close.diff gregory.p.smith, 2008-08-28 06:00 fix os.urandom infinite loop when passed a non integral float
Messages (12)
msg72050 - (view) Author: Daniel Diniz (ajaksu2) Date: 2008-08-27 23:30
Calling os.urandom(1 + float(x)) ends in a infinite loop due to a naive
condition check:

while len(bytes) < n:
    bytes += read(_urandomfd, n - len(bytes))

Trivial patch attached.
msg72064 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-08-28 05:42
i'll fix this and add a unit test.
msg72065 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-08-28 06:00
better patch with tests attached, no explicit int conversion is done.

i also wrapped the use of the fd returned by open with a try: finally:
to avoid any chance of a leak and renamed the bytes variable to bs to
match whats in py3k and avoid overriding the builtin type.
msg72074 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-08-28 09:40
The explicit int() conversion looked saner to me, rather than passing a
float argument to read().
By the way, is there a reason this code still uses os.open rather than
the builtin io.open?
msg72107 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-08-28 19:30
if i did 

 n = int(n)

that would change the API to allow bytes/unicode to be passed in which
is not something i want.  i don't even like that it allows floats.

by not doing the int conversion at all, a DeprecationWarning is raised
by the read() about the argument being a float which I figure it not a
bad thing given that the API really should only accept ints...

fwiw, daniel's patch would still cause this deprecation warning from
read so I guess using while len(bs) < int(n): isn't that bad either. 
but it would allow a string such as '0' to be passed in as an argument
without an exception...
msg72109 - (view) Author: Daniel Diniz (ajaksu2) Date: 2008-08-28 19:50
Gregory,
IMHO your patch is better in all aspects.

Regarding my patch, the API wouldn't change at all, as the source reads:
        while len(bytes) < int(n):
            bytes += read(_urandomfd, n - len(bytes))

So "n - len(bytes)" restricts the API to what it was before. But it
would call int() for each loop iteration :/

@Pitrou: My patch still passed the float to read (to keep the current
behavior, warning included), but if doing that can be considered a bug,
let's get rid of the DeprecationWarning by passing an int.
msg72117 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-08-29 00:25
Le jeudi 28 août 2008 à 19:30 +0000, Gregory P. Smith a écrit :
> if i did 
> 
>  n = int(n)
> 
> that would change the API to allow bytes/unicode to be passed in which
> is not something i want.

Ok, I hadn't thought about that. You convinced me.
msg72291 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-01 20:08
The patch looks fine to me as well.
msg72314 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-09-02 05:36
committed to trunk r66142
msg72791 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-09-08 21:46
Gregory, could you merge this into py3k, please?
msg72794 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-09-08 22:06
Apparently this isn't an issue in py3k, so no worries! :)
msg73407 - (view) Author: Roumen Petrov (rpetrov) * Date: 2008-09-18 22:53
It seems to me that test case will fail on windows and vms platforms.
The case contain os.urandom(1.1) but in posixmodule.c for urandon
functions (windows and vms) exits:
  PyArg_ParseTuple(args, "i:urandom", &howMany))

./python.exe -c 'import os; print "%s" %(os.urandom(1.9))' =>
-c:1: DeprecationWarning: integer argument expected, got float
History
Date User Action Args
2008-09-18 22:53:36rpetrovsetnosy: + rpetrov
messages: + msg73407
2008-09-08 22:06:28benjamin.petersonsetmessages: + msg72794
2008-09-08 21:46:38benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg72791
2008-09-02 05:36:35gregory.p.smithsetstatus: open -> closed
messages: + msg72314
2008-09-01 20:08:59amaury.forgeotdarcsetkeywords: - needs review
nosy: + amaury.forgeotdarc
resolution: accepted
messages: + msg72291
2008-08-29 00:25:53pitrousetmessages: + msg72117
2008-08-28 19:50:55ajaksu2setmessages: + msg72109
2008-08-28 19:30:30gregory.p.smithsetmessages: + msg72107
2008-08-28 09:40:31pitrousetnosy: + pitrou
messages: + msg72074
2008-08-28 06:00:35gregory.p.smithsetpriority: low -> normal
keywords: + needs review
messages: + msg72065
files: + urandom-float-and-close.diff
2008-08-28 05:42:07gregory.p.smithsetpriority: low
assignee: gregory.p.smith
messages: + msg72064
keywords: + easy
nosy: + gregory.p.smith
2008-08-27 23:30:11ajaksu2create